Skip to content

Commit 8dd5470

Browse files
committed
Resolve review comments
1 parent 9225edf commit 8dd5470

1 file changed

Lines changed: 15 additions & 7 deletions

File tree

ggml/src/ggml-cann/aclnn_ops.cpp

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2207,8 +2207,8 @@ static void aclnn_index_fill_tensor(ggml_backend_cann_context & ctx,
22072207

22082208
/**
22092209
* @brief Initializes and caches all intermediate tensors required for RoPE
2210-
* (Rotary Position Embedding), including support for YarnmRoPE
2211-
* i-mRoPENeox repeat strategyindependent sectorsfrequency factors,
2210+
* (Rotary Position Embedding), including support for Yarn, mRoPE,
2211+
* i-mRoPE, Neox repeat strategy, independent sectors, frequency factors,
22122212
* and multi-section rotary groups.
22132213
*
22142214
* This function computes and caches the per-dimension θ coefficients used for
@@ -2286,10 +2286,10 @@ static void aclnn_rope_cache_init(ggml_backend_cann_context & ctx,
22862286
// (1) multiplying by the position,
22872287
// (2) dividing by freq_factors,
22882288
// (3) computing the sine and cosine,
2289-
// the other parameters used in thecomputation generally do not change in most scenarios.
2289+
// the other parameters used in the computation generally do not change in most scenarios.
22902290
// Therefore, we can first compute this part of the result and then cache it.
22912291

2292-
// Step1.1: prepare theta_scale exponent. if this this exponent updated, should update theta_scale_tensor.
2292+
// Step1.1: prepare theta_scale exponent. if this exponent updated, should update theta_scale_tensor.
22932293
acl_tensor_ptr acl_theta_scale_tensor;
22942294
bool theta_scale_updated = false;
22952295
if (ctx.rope_cache.theta_scale_length != theta_scale_length || ctx.rope_cache.theta_scale != theta_scale ||
@@ -2299,7 +2299,7 @@ static void aclnn_rope_cache_init(ggml_backend_cann_context & ctx,
22992299
free(ctx.rope_cache.theta_scale_exp_host);
23002300
}
23012301
ctx.rope_cache.theta_scale_exp_host = (float *) malloc(theta_scale_length * sizeof(float));
2302-
2302+
GGML_ASSERT(ctx.rope_cache.theta_scale_exp_host != nullptr);
23032303
if (!indep_sects) {
23042304
ctx.rope_cache.theta_scale_exp_host[0] = 1;
23052305
for (int i = 1; i < theta_scale_length; i++) {
@@ -2382,7 +2382,7 @@ static void aclnn_rope_cache_init(ggml_backend_cann_context & ctx,
23822382
GGML_CANN_CALL_ACLNN_OP(ctx, InplaceAdds, acl_yarn_ramp_tensor.get(), freq_scale_sc.get(), one.get());
23832383
}
23842384

2385-
// Step 1.3: update thea_scale_tensor according to ext_factor or freq_scale.
2385+
// Step 1.3: update theta_scale_tensor according to ext_factor or freq_scale.
23862386
if (ext_factor != 0) {
23872387
if (theta_scale_updated || yarn_ramp_tensor_updated) {
23882388
theta_scale_updated = true;
@@ -2411,7 +2411,7 @@ static void aclnn_rope_cache_init(ggml_backend_cann_context & ctx,
24112411
free(ctx.rope_cache.position_select_index_host);
24122412
}
24132413
ctx.rope_cache.position_select_index_host = (int *) malloc(theta_scale_length * sizeof(int));
2414-
2414+
GGML_ASSERT(ctx.rope_cache.position_select_index_host != nullptr);
24152415
int sect_dims = sections[0] + sections[1] + sections[2] + sections[3];
24162416
int sec_w = sections[1] + sections[0];
24172417
int sec_e = sections[2] + sec_w;
@@ -2675,6 +2675,14 @@ void ggml_cann_rope(ggml_backend_cann_context & ctx, ggml_tensor * dst) {
26752675
const bool mrope_used = mode & GGML_ROPE_TYPE_MROPE; // ggml_rope_multi, note: also true for vision (24 & 8 == true) and for imrope
26762676
const bool is_vision = mode == GGML_ROPE_TYPE_VISION;
26772677

2678+
if (mrope_used) {
2679+
GGML_ASSERT(sections[0] > 0 || sections[1] > 0 || sections[2] > 0);
2680+
}
2681+
2682+
if (is_vision) {
2683+
GGML_ASSERT(n_dims == ne0/2);
2684+
}
2685+
26782686
if (is_imrope || mrope_used) {
26792687
is_neox = true;
26802688
}

0 commit comments

Comments
 (0)