Skip to content

unsloth : repin PR #70 after its force-push - #104

Closed
danielhanchen wants to merge 1 commit into
masterfrom
fix-pr70-pin
Closed

unsloth : repin PR #70 after its force-push#104
danielhanchen wants to merge 1 commit into
masterfrom
fix-pr70-pin

Conversation

@danielhanchen

Copy link
Copy Markdown
Member

The nightly Unsloth prebuilt (full release) run has been failing at Resolve tag since PR #70 was force-pushed:

refusing unslothai/llama.cpp#70: pinned commit 06d2326acbf515b10f8d6abeada09123d361acde
is not a commit of that PR (wrong paste, or force-pushed away);
fix the pin in scripts/unsloth/pr-set.json

The guard is doing its job. #70 was rebased at 2026-08-17T13:44:57Z (head_ref_force_pushed), which orphaned 06d2326; the first scheduled run afterwards aborted before fan-out, so CPU/macOS/CUDA/CUDA Windows/Vulkan/ROCm all reported 0s and no release was published.

This repins to the current head, edfd4c1a3b7a653303a85257ddac2a1f3ce39a2f.

What actually changed in the rewrite

The force-push kept the same three commits and rebased them from a614fab onto 34af94cd. Comparing the two patch sets against their own bases, the interdiff is 57 lines and almost all of it is base drift:

  • src/models/kimi-k3.cpp, LLM_KV_EXPERT_LATENT_LENGTH: the old base already had this key required, so the old patch only added a comment. The new base has it as , false, so the new patch flips it back to required. Same end state.
  • src/models/kimi-k3.cpp, layer.ssm_a: TENSOR_NOT_REQUIRED vs 0 tracks the same flag changing in the base. Both versions land on LLM_TENSOR_SSM_A_NOSCAN; the explanatory comment was shortened from six lines to four.

One real content change: the MXFP4 0xff E8M0 NaN-scale validation in conversion/base.py (5 lines, invalid E8M0 scale byte 0xff in N MXFP4 block(s)) was dropped. It is not in either base, so it is gone rather than upstreamed. That check was unrelated to #70's stated scope of the MoonViT-3d vision tower and full-size loading, so dropping it looks deliberate, but flagging it here since it is the only behavioural delta from the reviewed pin.

Other pins

Resolve tag aborts on the first bad pin, so #91 and #95 were never reached. Both re-verified as still valid against their PRs: c86ed26 is #91's head, 3db8cb5 is #95's head. The two upstream pins (ggml-org/llama.cpp#24423, #25731) resolved fine in the failing run.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ac4a72defb

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

"https://github.com/ggml-org/llama.cpp/pull/24423/commits/daca8075d871483545dd85d58ce11970b304b541",
"https://github.com/ggml-org/llama.cpp/pull/25731/commits/0a9841fa63edce1cd252ed6efea713715abb0cf2",
"https://github.com/unslothai/llama.cpp/pull/70/commits/06d2326acbf515b10f8d6abeada09123d361acde",
"https://github.com/unslothai/llama.cpp/pull/70/commits/edfd4c1a3b7a653303a85257ddac2a1f3ce39a2f",

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Recreate this change as a human-authored commit

The commit metadata identifies both the author and committer as Codex <codex@openai.com>, making this an automated commit; this repository explicitly prohibits automated commits and warns that they can cause immediate PR closure. Have a human contributor recreate this one-line pin update rather than submit this commit object.

AGENTS.md reference: AGENTS.md:L45-L49

Useful? React with 👍 / 👎.

@danielhanchen

Copy link
Copy Markdown
Member Author

Superseded by #103, now merged.

#103 moves the same #70 pin to edfd4c1a and additionally moves ggml-org#25731 from 0a9841fa to c44c9a11. That second move is required: this PR's preflight failed because 0a9841fa stopped merging once the aged base moved to b10472, conflicting in src/llama-arch.cpp, src/llama-model.cpp, tools/mtmd/clip-model.h and tools/mtmd/clip.cpp. The probe stops at the first bad pin, so #70 was never reached here.

Replaying the preflight probe loop locally against the current base confirms the difference:

pin set from #103 onto b10472          pin set from #104 onto b10472
CLEAN     ggml-org#24423               CLEAN     ggml-org#24423
CLEAN     ggml-org#25731 c44c9a11      CONFLICT  ggml-org#25731 0a9841fa
ADDITIVE  unslothai#70   edfd4c1a
CLEAN     unslothai#91   c86ed269
CLEAN     unslothai#95   3db8cb5b
all pins stack cleanly                 later pins not probed

Closing; the pin set on master is the one from #103.

@danielhanchen
danielhanchen deleted the fix-pr70-pin branch August 18, 2026 07:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant