deps: RoPE correctness fix in llama.cpp (point at GeometricAGI fork)#9
Open
DeanoC wants to merge 1 commit into
Open
deps: RoPE correctness fix in llama.cpp (point at GeometricAGI fork)#9DeanoC wants to merge 1 commit into
DeanoC wants to merge 1 commit into
Conversation
Bumps server/deps/llama.cpp from ac06e543 (Luce-Org perf/luce-verify-kernels) to GeometricAGI/lucebox-ggml @ 0fe65d93 = ac06e543 + one commit: 'ggml-cuda: defer RoPE mod-2pi reduction until after angle scaling'. rope_theta_fp64() reduced the rotation angle mod-2pi before rope_yarn's freq_scale multiply and the callers' /freq_factor; the reduction does not commute with scaling, so every RoPE config with freq_scale!=1 or freq_factor!=1 was numerically wrong (freq_factors, YaRN, mrope/vision/imrope, ROPE_BACK, ROPE_SET_ROWS). rope.cu is byte-identical between ac06e543 and 30c9d7d, where this exact change was verified via test-backend-ops on gfx1201 + gfx1100: ROPE 268/268, ROPE_BACK 228/228, ROPE_SET_ROWS 48/48 (was 356 failures), SOFT_MAX unchanged 212/212. Upstream to Luce-Org when ready.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Repoints
server/deps/llama.cppfromac06e543(Luce-Orgperf/luce-verify-kernels) toGeometricAGI/lucebox-ggml @ 0fe65d93=ac06e543+ one commit:Also switches the submodule
urlto our fork and its trackingbranchtofix/rope-fp64-deferred-reduction. No source changes in lucebox-hub itself — only.gitmodules+ the gitlink.Why (the bug)
rope_theta_fp64()reduced the rotation angle mod-2π before the downstream scaling inrope_yarn(freq_scale *) and the callers'/freq_factor. mod-2π reduction does not commute with scaling, so results were wrong for every RoPE config withfreq_scale != 1.0orfreq_factor != 1.0— freq_factors, YaRN, mrope/vision/imrope, and the backward + set_rows paths that share the helper. (The code comment even documented the caveat: only valid for the qwen35 dflash graph wherefs==ff==1.)The fix returns the unreduced angle in double, carries double through
rope_yarn's scaling, and does the mod-2π reduction in double right beforecosf/sinf— preserving the large-freq_base(Qwen3.5, 1e7) precision benefit and a no-op for thefs=1,ff=1fast path.Verification
rope.cuis byte-identical betweenac06e543and30c9d7d, where this exact change was verified withtest-backend-opson gfx1201 (R9700) and gfx1100 (7900 XTX):Full suite went 11232 → 11588 / 11606 (the remaining 18 are
GATED_DELTA_NET, a separate unrelated op bug — not addressed here).Recommend a CI
test-backend-opsrun on the fork to confirm on the exactac06e543tree.Follow-ups
0fe65d93fromGeometricAGI/lucebox-ggml→Luce-Org/lucebox-ggml(baseperf/luce-verify-kernels) when ready.GATED_DELTA_NET(18 failing ops) still needs separate diagnosis.🤖 Generated with Claude Code