Repository navigation
unsloth: pin #253 in place of #241 to drop the CUDA graph cache cap (stacked on #252) - #254
Open
danielhanchen wants to merge 3 commits into
Open
danielhanchen wants to merge 3 commits into
danielhanchen wants to merge 3 commits into
Conversation
#253 is #241's pinned commit plus one commit that removes the 64 entry cap on the CUDA graph cache, which evicts graphs still in use under --split-mode tensor and makes decode as slow as running without CUDA graphs (unslothai/unsloth#12468). Upstream has no count cap. It replaces #241 rather than following it: a later pin that removes lines an earlier pin adds fails pin_contract.py by design, and #253 already carries everything #241's pin does.
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. |
…-cap # Conflicts: # scripts/unsloth/feature-checks.json # scripts/unsloth/pr-set.json
This branch has not been deployed
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.
Stacked on #252. Takes #252's pin set and puts #253 in #241's slot, so the nightly ships #241 without the 64 entry CUDA graph cache cap behind the
--split-mode tensordecode regression in unslothai/unsloth#12468 (2.4x to 4.3x slower than the official ggml-org build on 2x RTX 5070 Ti, 1.57x on 2x B200). Against #252 this is a 2 line change.Why #253 replaces #241 instead of following it
#253's head (
20dd977ea) is #241's head9dd797259, which #252 pins, merged with the one commit that removes the cap. Its tree is #241's with onlyggml/src/ggml-cuda/common.cuhchanged (+5 -14), so it carries everything #241's pin does.Listing #253 after #241 cannot work:
pin_contract.pychecks that every line a pin adds is still in the merged tree, so #241 would fail on the removedmax_cuda_graphsline, andunsloth-prebuilt.ymlexits on that check. Taking #241's slot keeps the order and the composition, minus the cap.feature-checks.jsonmoves #241'suncheckedreason to #253, noting that the cap only shows with--split-mode tensoron two GPUs.Pin set merges additively
Replayed locally the way
unsloth-prebuilt.ymldoes it (diff3 merge per pin, thenscripts/unsloth/additive_merge.pyon a conflict), ontob11491, the base the preflight picked today:additive_merge.pymerge_checks.pypin_contract.pymax_cuda_graphslines in the mergedcommon.cuhggml_cuda_graph_get_keyllamaandmtmd) and buildsllama-benchwith CUDA (sm_100).llama-bench(Qwen3-4B Q4_K_M,-sm tensor, 2x B200, shared GPUs on a loaded host, so this shows engagement rather than a speed figure), CUDA graphs beatGGML_CUDA_DISABLE_GRAPHS=1in all 3 rounds: 150 vs 85, 194 vs 30, 93 vs 50 tok/s. With the cap the two were equal.b11443-mix-d65395frelease, replayed onto b11443, merges all 15 pins (Add TML Inkling architecture ggml-org/llama.cpp#25731, kimi-k3 : the MoonViT-3d vision tower and full-size loading fixes #70 and EmbeddingGemma-2 support #247 additively), as that nightly did.The preflight run on this branch before it was stacked failed at ggml-org#25731 on b11491. That is the Inkling conflict #252 fixes, and it never reached #253.
Order
Merge #252 first, then this. If #252 changes again, this needs only the #241 line re-swapped to a #253 head that contains #241's new head.