Repository navigation
unsloth: pin #253 (no CUDA graph cache count cap) in place of #241 - #256
Merged
Merged
Conversation
…nt cap #253 is #241 plus one commit that removes the 64 entry cap on the shape keyed CUDA graph cache, which made tensor split re-capture graphs on every token (unslothai/unsloth#12468). Its branch now merges #241's current head, so pinning it instead of #241 carries every #241 item and the fix, and keeps pin_contract from reading the removed cap lines as a lost #241 change.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
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
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.
Pins #253 (no count cap on the CUDA graph cache) in place of #241.
#253 is #241 plus one commit. That commit removes the 64 entry LRU cap that the shape keyed graph cache from #241 added. With the cap, tensor split re-captures graphs on every token, because it needs about
2 * n_layers + 1graphs per device. The result is 2.4x to 4.3x slower decode than the official build (unslothai/unsloth#12468). Upstream dropped the same cap in review of ggml-org#21611.Why replace #241 rather than add #253 after it
b96a713, which still had the NextN graph that upstream replaced with ggml-org#29928. I merged Carry what the dropped GLM-5-Next, Qwen MTP and readahead pins had beyond upstream #241's current head9dd7972into it. ggml-cuda: drop the CUDA graph cache count cap, as upstream does #253 is now20dd977: Carry what the dropped GLM-5-Next, Qwen MTP and readahead pins had beyond upstream #241 plus the cap removal (+5/-14 incommon.cuh), nothing else.pin_contract. ggml-cuda: drop the CUDA graph cache count cap, as upstream does #253 deletes the cap lines Carry what the dropped GLM-5-Next, Qwen MTP and readahead pins had beyond upstream #241 adds, so the contract reports Carry what the dropped GLM-5-Next, Qwen MTP and readahead pins had beyond upstream #241 as not intact (common.cuh kept 4/10). Pinning ggml-cuda: drop the CUDA graph cache count cap, as upstream does #253 alone carries every Carry what the dropped GLM-5-Next, Qwen MTP and readahead pins had beyond upstream #241 item and the fix, and the contract passes.feature-checks.json: Carry what the dropped GLM-5-Next, Qwen MTP and readahead pins had beyond upstream #241'suncheckedentry moves to ggml-cuda: drop the CUDA graph cache count cap, as upstream does #253, with the cap removal added to the reason. It needs a multi-GPU CUDA host, and no runner has one.Verification
Merge loop (diff3, then
additive_merge.py, asunsloth-prebuilt.ymlruns it) pluspin_contract.py:pin_contractc811cb8f0The
feature-checks.jsoncoverage lint passes: 15 pins, 7 with a feature check, 8 knowingly unchecked.