[staging CI] unslothai/unsloth#8435 - #263
Conversation
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 04ff59d2a0
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| _foot_wo = (_base_wo + _shared) / (1024 * 1024) | ||
| if _foot_wo > _budget_wo: | ||
| continue |
There was a problem hiding this comment.
On a heterogeneous or partially busy multi-GPU subset, the pooled footprint can pass here even though the least-available GPU cannot hold its replicated pipeline and compute-buffer reserve. The actual Auto placement below handles that case with _every_gpu_holds_reserve and may cap the context via _cap_ctx_to_per_device_reserve, but this probe immediately treats the uncapped context as a valid target-only placement. It can therefore conclude that the drafter does not fit and disable it even though the drafter would fit at the context the real placement selects; with an explicit context, the target may instead fall back to --fit on, contradicting the premise that it pins. Mirror the placement branch's per-device check and context cap before setting _target_fits_somewhere or comparing the drafter footprint.
Useful? React with 👍 / 👎.
04ff59d to
3cc756e
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 18ecab6753
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| _probe_ranked = sorted( | ||
| gpus, | ||
| key = lambda g: _gpu_usable(g, _probe_frac(False)), | ||
| reverse = True, |
There was a problem hiding this comment.
When GPUs have different total capacities or existing allocations and the flat MTP reserve applies, lowering the usable fraction for the drafter can reorder the devices. This probe ranks with _probe_frac(False), while the actual Auto placement later ranks with _pin_fraction, which includes the drafter reserve. A large, partly busy card can therefore lead the probe but contribute less than the extra pipeline overhead under the drafter budget, causing every probed prefix to fail even though the actual drafter-ranked leading GPU can hold both; Auto then incorrectly sets drafter_no_vram and disables the drafter. Rank the both-fit candidates using the same active fraction as the real placement, or evaluate both rankings.
Useful? React with 👍 / 👎.
…ble charging a priced override
…on the launched drafter
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
- --model-draft naming a path that is not on disk is no longer priced as a Hugging Face repo, so a typo cannot 409 a chat load over a 12 GiB reserve. - A drafter listing that carries no sizes falls through to the cache and then the reserve; zero is kept for the case where every family is an incomplete split and the fetch can load none of them. - Auto DFlash is not charged when the extras already name their own drafter. - An MLA embedded-MTP model keeps mla_mtp_disabled as its fallback reason, so the notice does not invite the user to force a path that is slower than the ngram-mod they are getting.
- Draft flags are last-wins, so a repo id followed by --model-draft leaves a path as the drafter. _extra_args_mtp_draft_source now returns the value and whether the flag that won carried a repo id, so a path is never priced as a Hugging Face repository. - split_listing_is_complete tracks distinct shard indices inside 1..total rather than counting matches, so 00001-of-00002 beside a stray 00003-of-00002 is no longer read as a whole set.
_cached_repo_gguf_bytes scanned huggingface_hub's import-time default, so a user who moved the cache had their cached drafter missed and priced at the flat reserve instead. Pass the active hub cache, as the rest of Studio's cache operations do. Also covers the underscore flag spelling llama.cpp accepts.
for more information, see https://pre-commit.ci
The reserve constant carried two overlapping headers, the extras-drafter note still described the local-file-only rule that was reverted once remote repos could be priced, and the zero-bound note no longer matched the code.
18ecab6 to
8e5302d
Compare
1 participant
Disposable CI run for unslothai/unsloth#8435. Do not merge; closed after CI.