[staging CI] unslothai/unsloth#8701 - #312
Open
danielhanchen wants to merge 7 commits into
Open
Conversation
TRL 1.10.0 dropped the collective_rpc("reload_weights") call from
VLLMGeneration.generate, so the regex anchored on it matched nothing and
raised. The lora_request injection ran after that anchor in the same
function, so it never happened, and _init_vllm/sync_weights had already
been installed by then. fast_inference=True GRPO therefore sampled every
rollout from the base model with the adapter ignored, and reported finite
losses while doing it.
Wrap VLLMGeneration.generate instead of rewriting its source, and
intercept on the vLLM engine (generate/chat/collective_rpc), which is the
part Unsloth owns. Restore all three methods if any patch fails, so the
half-patched state is unreachable, and wake a sleeping engine before
sync_weights returns early.
Two metadata caps, and both have to move together or neither does anything. trl<=0.24.0 is metadata only. Nothing in unsloth or unsloth_zoo reads the TRL version and refuses 1.x at runtime; the only comparisons are floors at trainer.py:725 and :993. Measured against real TRL 1.10.0 with the trees on PYTHONPATH so no resolver ever sees the cap: import ok, tests/ -k grpo 991 passed, tests/version_compat 1702 passed, and four real 3-step GRPO runs whose grad-norms and reward stats match TRL 0.22.2 to every digit. The 5-test gap against 0.22.2 is this repo's own TRL >= 1.7.0 gated assertions, which 0.22.2 skips and 1.10.0 runs and passes. datasets<4.4.0 is the second cap, and raising only the TRL one ships a pin no user exercises: with the datasets cap in place, a resolver asked for trl<=1.10.0 settles on trl==0.29.1, because TRL 1.10.0 requires datasets>=4.7.0. So the datasets cap has to open at the same time. It opens as a hole rather than a floor bump. A datasets>=4.7.0 floor would drop Python 3.9 and force 4.x on TRL 0.22.2 users who do not need it. The hole !=4.4.*,!=4.5.0 is exactly what patch_datasets in import_fixes.py already refuses at runtime over the RLock recursion bug, so metadata and the runtime guard now agree instead of the metadata being the stricter of the two. Verified on datasets 4.8.5 with TRL 1.10.0: 991 / 1702 passed, exit 0 both, and the GRPO smoke numbers are identical to the datasets 3.6.0 run to every digit.
for more information, see https://pre-commit.ci
… shape
The failure this locks down is silent: with TRL 1.10.0's generate, the old
source patch anchored on collective_rpc("reload_weights") matched nothing,
raised, and took the lora_request injection down with it, so fast_inference
GRPO sampled every rollout from the base model at exit 0.
The tests are shape-driven rather than version-driven. A synthetic
trl.generation.vllm_generation goes into sys.modules with a VLLMGeneration
whose _init_vllm and sync_weights carry the anchors the source patches need,
and whose generate reaches a fake engine the way a given TRL era reaches it:
sync_weights on 1.10.0, collective_rpc on 0.22.2, chat for conversational
rollouts, and no engine at all in server mode. Then the real
vllm_generation_init_patch runs over it and the fake engine records what
arrived. No vLLM, no GPU, no network, and importlib_version is monkeypatched
so the installed TRL is irrelevant - these pass against the 0.25.1 in this
env, which the patch would otherwise skip outright.
Also covered: the engine methods are restored after the call so a later
model.fast_generate is clean, an engine that already carried its own generate
gets it back rather than deleted, a TRL-created engine is left alone, a
sleeping engine is woken before sync_weights returns early, a failed sibling
patch rolls all three methods back, and patching twice does not double wrap.
Seven of the ten fail on c160215.
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: 7602088d0c
ℹ️ 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".
| "if not (getattr(getattr(self, 'llm', None), 'shared_weights', False) or getattr(self, 'unsloth_fast_inference_lora', False)):", | ||
| "generate", | ||
| ) | ||
| generate_patched = install_generate_wrapper() |
There was a problem hiding this comment.
When a VLLMGeneration method is absent or inspect.getsource() fails, patch_vllm_generation_method() returns False rather than raising RuntimeError. These results are never checked, so the remaining methods—including this generate wrapper—are still installed and the advertised all-or-nothing rollback does not run. For example, a source-unavailable sync_weights leaves _init_vllm redirected to the shared engine while retaining upstream weight synchronization, which can reload or update that engine unexpectedly; treat any false result as a patch failure and restore the snapshot.
Useful? React with 👍 / 👎.
danielhanchen
force-pushed
the
pr-8701-ci
branch
from
August 14, 2026 03:04
7602088 to
a6e2c91
Compare
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.
Disposable CI run for unslothai/unsloth#8701. Do not merge; closed after CI.