(GRPO) Fix PEFT replacement for TRL >= 1.7.0, add missing compute_aux_loss for TRL >= 1.7.0 by marcandrelarochelle · Pull Request #6904 · unslothai/unsloth

marcandrelarochelle · 2026-07-06T13:28:08Z

Fix PEFT replacement for TRL >= 1.7.0
Add missing compute_aux_loss for TRL >= 1.7.0

gemini-code-assist

Code Review

This pull request introduces support for TRL version 1.7.0+ by adjusting PEFT initialization logic in unsloth/models/rl.py, and adds auxiliary loss computation support (compute_aux_loss) to _get_per_token_logps_and_entropies in unsloth/models/rl_replacements.py. However, a critical bug was identified in unsloth/models/rl_replacements.py where _extra_moe_kwargs is not initialized before a key is assigned to it, which will lead to a NameError at runtime.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

gemini-code-assist · 2026-07-06T13:29:54Z

+                    if compute_aux_loss is not None:
+                        _extra_moe_kwargs["output_router_logits"] = compute_aux_loss
+                    else:
+                        _extra_moe_kwargs = {}


A NameError will occur here because _extra_moe_kwargs is not initialized before we attempt to assign a key to it in the if branch. Please initialize _extra_moe_kwargs as an empty dictionary first, or use a dictionary comprehension/conditional assignment.

_extra_moe_kwargs = {"output_router_logits": compute_aux_loss} if compute_aux_loss is not None else {}

Confirmed and fixed. Initializing _extra_moe_kwargs before use, and gating on truthiness of compute_aux_loss (which is False, never None, for non-MoE under TRL 1.7.x) so router logits are only requested for MoE.

danielhanchen · 2026-07-06T16:08:08Z

Thanks for this. I pushed a few fixes on top and validated the branch end to end with a tiny Qwen3-MoE GRPO run on TRL 1.7.1 (also transformers 5.8 / torch 2.10). Summary of what changed and why.

Fixes pushed

_extra_moe_kwargs (the item Gemini flagged): it was referenced before assignment, so any call with compute_aux_loss set raised UnboundLocalError. It is now initialized first, and output_router_logits is only requested when aux loss is actually wanted (gating on truthiness, since compute_aux_loss is False/True but never None at TRL's call site, so the old is not None check misfired for dense models too).
Return arity. This is the one that actually blocks training. TRL 1.7.x _generate_and_score_completions unpacks three values at several sites, e.g. old_per_token_logps, _, _ = self._get_per_token_logps_and_entropies(...), and those calls do not pass compute_aux_loss. The original patch returned a 2-tuple in that case, so a real run crashes with ValueError: not enough values to unpack (expected 3, got 2). I made _get_per_token_logps_and_entropies return (logps, entropies, aux_loss) for trl >= 1.7.0 and keep the 2-tuple for older TRL, gated at injection time on trl_version. Verified the emitted source across TRL 0.22.2 through 2.0.0.
PEFT removal regex in rl.py. The pattern ended on param.data = param.data.to(torch.bfloat16), which with re.DOTALL reaches past the ref-adapter block all the way to the QLoRA cast, also deleting the intervening gradient-checkpointing block:
```
if is_peft_model(model) and args.gradient_checkpointing:
    model.enable_input_require_grads()
```
I anchored the end on ref_param.data.copy_(param.data) so only the ref-adapter elif block is removed, and neutralize TRL 1.7.0's if _is_quantized_model: bf16 cast the same way the existing is_loaded_in_4bit cast is handled. Checked against real TRL 1.7.0 and 1.7.1 source: the transform removes only the intended block, keeps enable_input_require_grads(), and ast.parses clean.

With these, a Qwen3-MoE GRPO run trains to completion under TRL 1.7.1 where the previous state raised the unpack error.

Follow-up needed (MoE aux loss is not yet functional)

Heads up that the aux-loss piece does not yet actually apply the router load-balancing loss in Unsloth's GRPO. On the optimized path Unsloth returns hidden states and the model forward does not populate outputs.aux_loss (qwen3_moe/gpt_oss set it to None; DeepSeek-V3 is auxiliary-loss-free so it does not apply there). On top of that, the _get_per_token_logps_and_entropies chunk loop where aux_loss would be gathered is bypassed by the default-on sequence-packing / PrefixGrouper paths. In an instrumented run, aux_loss comes back None on every call and the aux_loss metric is never logged.

Making it functional is best done at the model-forward chokepoint (populate aux_loss from router_logits in the unsloth_zoo qwen3_moe and gpt_oss forward patches) so it survives all packing paths and flows into compute_loss's loss + router_aux_loss_coef * aux_loss. That is a separate change in unsloth_zoo. The fixes here keep training correct and crash-free in the meantime.

marcandrelarochelle · 2026-07-06T21:56:26Z

@danielhanchen Sorry for the small bug with the _extra_moe_kwargs, I cherry-picked it from my fork where I was initially passing it to the _extra_vision_kwargs as a workaround.

I tested the new changes you made and I can confirm it works with TRL 1.7.0, haven't updated to 1.7.1 yet, but will pretty soon!

If you have time, I also created 2 PR on Unsloth Zoo (for the SAPO + Luspo loss and one for the Importance Sampling + VESPO bug fix -> needed for TRL >= 1.6.0)

danielhanchen · 2026-07-07T05:41:43Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3e9a3b20ed

ℹ️ 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".

chatgpt-codex-connector · 2026-07-07T05:52:25Z

+            aux_loss = (
+                torch.stack(all_aux_losses).mean()
+                if (compute_aux_loss and len(all_aux_losses) > 0)
+                else None


Return aux_loss when packed GRPO path is used

For TRL >= 1.7 MoE training with compute_aux_loss=True, the default sequence-packing or PrefixGrouper path sets logprobs and empties zipped_inputs, so the padded loop that appends to all_aux_losses never runs. This expression then returns None for aux_loss, and TRL's loss path expects a tensor to add/log the router auxiliary loss, so packed text batches either crash or silently lose the MoE aux objective unless packing falls back.

Useful? React with 👍 / 👎.

Fixed in 4079206: return a zero placeholder (logprobs.new_zeros(())) instead of None whenever compute_aux_loss is set, so the packed/PrefixGrouper path no longer yields None and TRL can add/log the term without a crash. The router aux objective itself is not yet wired into the Unsloth GRPO loss; that is tracked as a separate unsloth_zoo change.

chatgpt-codex-connector · 2026-07-07T05:52:25Z

+                            if compute_aux_loss and getattr(outputs, "aux_loss", None) is not None:
+                                all_aux_losses.append(outputs.aux_loss)


Avoid collecting aux_loss under inference mode

When sequence packing falls back and compute_aux_loss=True, this aux loss is produced inside the surrounding _get_inference_mode_context_manager(model) block, so it is detached from autograd before being returned. In the padded fallback path TRL can add/log a tensor, but the router auxiliary objective contributes no gradients, which means MoE load-balancing is silently not trained whenever this path is used.

Useful? React with 👍 / 👎.

Fixed in 4079206: removed the aux_loss collection that ran inside the inference-mode block, so no detached tensor is returned. aux_loss is now a zero placeholder pending the separate change that computes the router loss with gradients.

danielhanchen · 2026-07-07T06:08:28Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 407920673f

ℹ️ 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".

chatgpt-codex-connector · 2026-07-07T06:16:17Z

+            # The MoE router aux loss is not yet wired into the Unsloth GRPO loss, so return a
+            # zero placeholder (never None) to keep the TRL >= 1.7.0 3-tuple contract and let TRL
+            # add/log it without a crash.
+            aux_loss = logprobs.new_zeros(()) if compute_aux_loss else None


Return the real MoE router auxiliary loss

When GRPO runs with TRL >= 1.7 on an MoE model with the router auxiliary loss enabled, this synthesizes a zero scalar instead of requesting/returning the model's actual router aux loss. The replacement compute_loss has no other path that adds that load-balancing term, so such runs silently drop the configured MoE regularizer while reporting/adding zero. Please pass the needed router-loss kwargs through the forward call and return the actual model aux loss, or explicitly keep aux loss disabled.

Useful? React with 👍 / 👎.

Went with your second option: aux loss is now explicitly disabled by default. #6938 (merged) sets router_aux_loss_coef to 0 for GRPO on TRL >= 1.7.0, so aux_loss_enabled is False and TRL never adds or logs the term. The zero scalar here is only reached if a user opts in with router_aux_loss_coef > 0, where it keeps the 3-tuple return crash-safe. Dropped the duplicate default from this branch in 7969160.

danielhanchen · 2026-07-07T12:40:49Z

On the MoE aux_loss: I have defaulted router_aux_loss_coef to 0 for GRPO on TRL >= 1.7.0 (d8474ae).

TRL 1.7.0 turns the router load-balancing aux loss on by default (router_aux_loss_coef = 0.001, with aux_loss_enabled = is_moe and coef != 0.0). Unsloth's optimized GRPO forward returns hidden states and does not compute the reduced aux_loss, so rather than log a fake zero we default the coefficient to 0. Then aux_loss_enabled is False, TRL never adds or logs the term, and behaviour matches every pre-1.7.0 release. Anyone who wants it can still opt in with router_aux_loss_coef > 0.

Verified on TRL 1.7.1 (tiny Qwen3-MoE) and TRL 0.25.1:

GRPO default: router_aux_loss_coef=0.0, aux_loss_enabled=False, trains, no aux metric.
GRPO opt-in (router_aux_loss_coef=0.01): aux_loss_enabled=True, trains without error.
SFT and DPO train unchanged. The PEFT ref-adapter strip only matches grpo_trainer, and the _is_quantized_model cast removal is the same one already applied to the older is_loaded_in_4bit spelling across trainers.
No-op on TRL < 1.7.0 (the argument does not exist).

danielhanchen · 2026-07-07T12:51:36Z

Removed the router_aux_loss_coef = 0 default from this branch in 7969160, since it landed separately on main in #6938. This PR now stays focused on the TRL >= 1.7.0 crash fixes (PEFT ref-adapter removal, return arity, _is_quantized_model cast) and rebases cleanly on top of the aux default.

danielhanchen · 2026-07-07T13:06:41Z

@codex review

danielhanchen · 2026-07-07T13:06:53Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 796916086f

ℹ️ 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".

chatgpt-codex-connector · 2026-07-07T13:14:19Z

+            # aux loss defaults off (router_aux_loss_coef set to 0 in models/rl.py), so
+            # compute_aux_loss is normally False -> None. On explicit opt-in the optimized forward
+            # cannot compute it, so return a zero placeholder (never None) for TRL's 3-tuple contract.
+            aux_loss = logprobs.new_zeros(()) if compute_aux_loss else None


Return the real MoE auxiliary loss

With TRL >= 1.7 on MoE models where router_aux_loss_coef is non-zero/default, compute_aux_loss is true and TRL folds this third return value into the policy loss and aux-loss metrics. Returning a fabricated zero here silently disables the router load-balancing penalty for those runs, so expert routing can train differently from the requested configuration; please request/propagate the model's actual aux loss (or explicitly disable the coefficient) instead of returning zero.

Useful? React with 👍 / 👎.

Rebased onto current main (8dfd996), which now defaults router_aux_loss_coef to 0 (#6938, merged). So aux_loss_enabled is False by default and TRL never folds in or logs the term; this zero is reached only if a user opts in with router_aux_loss_coef > 0, where it keeps the 3-tuple return crash-safe. That is the explicitly-disable-the-coefficient path you noted.

…or TRL 1.7.0

for more information, see https://pre-commit.ci

rl.py: for trl >= 1.7.0, scope the PEFT removal regex to the ref-adapter block only by anchoring the end on ref_param.data.copy_(param.data), so it no longer also deletes the following gradient-checkpointing enable_input_require_grads() block. Neutralize TRL 1.7.0's `if _is_quantized_model:` bf16 cast the same way the existing is_loaded_in_4bit cast is handled. rl_replacements.py: initialize _extra_moe_kwargs before use (it was referenced before assignment whenever compute_aux_loss was passed) and only request output_router_logits when the aux loss is actually wanted. rl_replacements.py: _get_per_token_logps_and_entropies now returns a 3-tuple (logps, entropies, aux_loss) for trl >= 1.7.0 and a 2-tuple for older TRL, matching how every TRL call site unpacks the result. Without this, TRL 1.7.x _generate_and_score_completions unpacks 3 values from a 2-tuple and raises "not enough values to unpack (expected 3, got 2)".

danielhanchen · 2026-07-07T13:24:22Z

Rebased this branch onto current main (head 8dfd996e0). Two effects:

The diff is now just the two relevant files (rl.py PEFT ref-adapter removal + _is_quantized_model cast, rl_replacements.py return arity + aux placeholder); the earlier large diff was only the branch being behind main.
It inherits the router_aux_loss_coef = 0 default that landed in main via GRPO: default router_aux_loss_coef to 0 on TRL >= 1.7.0 #6938, so the MoE router aux loss is off by default and the zero placeholder here is only reached on explicit opt-in (router_aux_loss_coef > 0).

danielhanchen · 2026-07-07T13:24:28Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8dfd996e0d

ℹ️ 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".

chatgpt-codex-connector · 2026-07-07T13:34:24Z

+            # aux loss defaults off (router_aux_loss_coef set to 0 in models/rl.py), so
+            # compute_aux_loss is normally False -> None. On explicit opt-in the optimized forward
+            # cannot compute it, so return a zero placeholder (never None) for TRL's 3-tuple contract.
+            aux_loss = logprobs.new_zeros(()) if compute_aux_loss else None


Reject explicit router-aux opt-in

When compute_aux_loss is true (i.e. a MoE run explicitly sets a nonzero router_aux_loss_coef), this returns a synthetic zero instead of computing the model's router auxiliary loss. That satisfies TRL's 3-tuple unpacking but silently trains without the requested load-balancing penalty, so experiments that opt in get misleading results; either compute the aux loss or fail fast for this unsupported optimized path.

Useful? React with 👍 / 👎.

Addressed in 4785bb4. The optimized GRPO forward cannot produce the router aux loss, so instead of returning a fabricated zero it now fails fast at trainer init when router_aux_loss_coef > 0 (NotImplementedError), and returns None for the aux slot of TRL's 3-tuple otherwise. The default stays at 0 (aux off) via #6938, so the common path is unchanged.

Verified on TRL 1.7.1: default GRPO trains, explicit opt-in raises at construction, SFT/DPO are unaffected, and the injection arity gate still yields a 2-tuple for TRL < 1.7.0.

…ux placeholder Unsloth's optimized GRPO forward cannot compute the MoE router auxiliary loss. Previously an explicit opt-in (router_aux_loss_coef > 0) returned a fabricated zero, silently training without the requested load-balancing penalty. Now reject it at trainer init with a clear NotImplementedError, and return None (not zero) for the aux slot of TRL's 3-tuple. Default stays off (coef 0), so the common path is unaffected.

for more information, see https://pre-commit.ci

danielhanchen · 2026-07-07T13:55:18Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ab5030304a

ℹ️ 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".

chatgpt-codex-connector · 2026-07-07T14:04:36Z

                    with torch.amp.autocast(device_type = "cuda", dtype = self._autocast_dtype):
                        if pixel_values is None:
-                            logits_chunk = unwrapped_model(
+                            outputs = unwrapped_model(


Release hidden-state outputs before log-softmax

When the GRPO hidden-states fallback is active for models without native Unsloth hidden-state support, the wrapper in rl.py requests output_hidden_states=True and returns a ModelOutput that still contains every layer's hidden_states. Binding that full object here keeps those tensors (and any caches) alive while chunked_hidden_states_selective_log_softmax runs; the previous unwrapped_model(...).logits form allowed the ModelOutput to be freed immediately after extracting logits. On large fallback models this can turn old/ref logprob computation into an avoidable OOM; extract outputs.logits and del outputs before the chunked log-softmax (same pattern in the VLM branch below).

Useful? React with 👍 / 👎.

Good catch. Fixed in 52c1299: both the text and VLM fallback branches now extract outputs.logits and del outputs before the chunked log-softmax, so the ModelOutput (and its per-layer hidden_states on the output_hidden_states=True path) is freed instead of being held alive during the reduction. Verified default GRPO trains and the opt-in still fails fast at init.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6374c94eb4

ℹ️ 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".

chatgpt-codex-connector · 2026-07-08T03:10:33Z

+    # `main` is scheduled/dispatch-only so PR jobs stay fast and a bleeding-edge
+    # TRL break does not red every PR; `latest` also runs on PRs to guard our
+    # own source-transforms against the current TRL release.
+    if: ${{ matrix.channel != 'main' || github.event_name != 'pull_request' }}


Move matrix gate out of the job-level if

GitHub's context availability table for jobs.<job_id>.if lists only github, needs, vars, and inputs (while matrix is available on jobs.<job_id>.name and steps.if), so this job-level expression is rejected before the matrix is expanded with an Unrecognized named-value: 'matrix' workflow error. On any PR/scheduled run that loads this workflow, version-compat-ci.yml can fail validation instead of running; move this condition to the steps or express it with matrix exclude/separate jobs.

Useful? React with 👍 / 👎.

Fixed in 170c7b0. matrix is indeed not available in jobs.<job_id>.if, so that expression would fail the whole workflow with Unrecognized named-value: 'matrix'. Dropped the matrix: the job now runs against TRL latest unconditionally and re-runs against TRL main only on schedule/dispatch via a step-level if: ${{ github.event_name != 'pull_request' }} (which uses only the github context). Validated with actionlint.

chatgpt-codex-connector · 2026-07-08T03:10:33Z

+sys.path.insert(0, str(_SPOOF_DIR))
+import _zoo_aggressive_cuda_spoof as _spoof  # noqa: E402
+
+_spoof.apply()


Skip fake-run collection when torch is absent

When daily-fresh-fetch runs, the workflow installs only pytest before collecting tests/version_compat/, but this top-level _spoof.apply() imports torch before the fixture can skip missing runtime dependencies. In that context (and any local version-compat run without torch), collection fails with ModuleNotFoundError: No module named 'torch' instead of skipping this fake-run test; gate the spoof application behind an import check or use a module-level skip when torch is unavailable.

Useful? React with 👍 / 👎.

Fixed in 170c7b0. Added a module-level guard that skips the whole file when torch is absent (if importlib.util.find_spec("torch") is None: pytest.skip(..., allow_module_level=True)) before the spoof import, so daily-fresh-fetch (pytest-only, collects tests/version_compat/) skips it cleanly instead of hitting ModuleNotFoundError at collection.

chatgpt-codex-connector · 2026-07-08T03:10:33Z

                            # Essentially, for VLMs we do not go via the optimized path in models/,
                            # so we don't encounter the Flash Attn left-padding issue.
-                            logits_chunk = unwrapped_model(
+                            outputs = unwrapped_model(


Forward TRL 1.7 VLM kwargs in logprob replacement

For TRL >= 1.7 multimodal batches that carry the new vision fields such as spatial_shapes, num_tiles, or image_position_ids, upstream GRPO passes those through _get_per_token_logps_and_entropies, but this replacement still calls the model without chunking or forwarding them. Models whose forward depends on those fields will either error or compute logprobs against incomplete vision inputs during GRPO, so include the new VLM kwargs in the chunking and model call when present.

Useful? React with 👍 / 👎.

Correct that TRL 1.7's _get_per_token_logps_and_entropies forwards spatial_shapes / num_tiles / image_position_ids and this replacement does not. Two notes on scope: this is pre-existing (the VLM branch and its _extra_vision_kwargs set are on main, unchanged by this PR), and forwarding these correctly means replicating TRL's per-tile slicing (num_tiles cumsum, image_position_ids[img_start:img_end]) through the chunking loop, which cannot be exercised on the CPU fake-run and needs a real tiled-VLM under GRPO to validate. It only affects VLM architectures that require those fields (the common pixel_values + image_grid_thw path is already forwarded). Keeping this PR focused on the TRL >= 1.7 crash/arity/PEFT fixes and tracking the VLM field forwarding as a separate change so it can be validated properly rather than shipped blind.

- Drop the invalid job-level matrix if (matrix is not available in jobs.<id>.if -> 'Unrecognized named-value: matrix' fails the whole workflow). Use a single job that runs vs TRL latest always and re-runs vs TRL main only on schedule/dispatch via a step-level github.event_name guard. Validated with actionlint. - Module-level skip the fake-run test when torch is absent so daily-fresh-fetch (pytest-only, collects tests/version_compat/) does not crash on the top-level spoof import.

danielhanchen · 2026-07-08T03:17:44Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 170c7b02f1

ℹ️ 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".

chatgpt-codex-connector · 2026-07-08T03:27:26Z

                if os.environ.get("UNSLOTH_FORCE_FLOAT32", "0") == "1":
                    self._autocast_dtype = torch.float16

+            compute_aux_loss = kwargs.get("compute_aux_loss", None)


Thread the new TRL 1.7 VLM kwargs through logprob forwards

For TRL >= 1.7 this method is called with additional multimodal kwargs such as spatial_shapes, num_tiles, and image_position_ids, and _generate_and_score_completions stores them in inputs; the new 1.7 handling only pulls compute_aux_loss, while the chunking code and model calls below still drop those fields. In GRPO on LFM2-VL or image-position based VLMs, the patched trainer will omit required vision inputs when recomputing logprobs, causing failures or incorrect logprobs even though the new fake-run only covers text/older VLM kwargs. Please propagate these kwargs through the chunk slicing and _extra_vision_kwargs alongside this 1.7 update.

Useful? React with 👍 / 👎.

Agreed this is a real gap for LFM2-VL / image-position VLMs on TRL 1.7. It is pre-existing (the VLM branch and its kwarg set are unchanged by this PR) and forwarding these correctly means replicating TRL's per-tile / per-image slicing (num_tiles cumsum for spatial_shapes, image_position_ids[img_start:img_end]) through both chunk-construction paths, which the CPU fake-run cannot exercise and which needs a real tiled VLM under GRPO to validate. Rather than ship that blind alongside the crash/arity fix here, I opened #6960 to track it so it can get a proper logprob-parity check on GPU. Keeping this PR scoped to the TRL >= 1.7 crash/arity/PEFT fixes.

chatgpt-codex-connector · 2026-07-08T03:27:26Z

+        import unsloth  # noqa: F401  -- _gpu_init bootstrap under spoof
+    except Exception as e:  # pragma: no cover - env issue, not a regression
+        pytest.skip(f"`import unsloth` failed under spoof: {type(e).__name__}: {str(e)[:200]}")


Fail the fake-run when unsloth import breaks

In the new grpo-fake-run CI job the environment has just installed unsloth, trl, and their runtime deps, so an exception from import unsloth is exactly the import-time TRL/transformers drift this canary is meant to catch. Catching every exception and converting it to pytest.skip lets the job go green with skipped tests instead of failing when the patcher cannot even import, masking the regressions this workflow was added to detect; only known missing-dependency cases should skip.

Useful? React with 👍 / 👎.

Fixed in f2870e6. Dropped the broad except Exception -> pytest.skip around import unsloth (and the SFT/DPO helper): the grpo-fake-run job installs unsloth/trl/deps, so an import failure is the import-time drift this canary must fail on. Only the find_spec not-installed checks skip now.

unsloth/trl are installed in the grpo-fake-run job, so a failing import is the import-time drift this canary must catch. Keep only the not-installed find_spec skips; let a real import error fail the test.

for more information, see https://pre-commit.ci

danielhanchen · 2026-07-08T03:32:55Z

@codex review

danielhanchen · 2026-07-08T03:53:44Z

@codex review

chatgpt-codex-connector · 2026-07-08T04:03:57Z

Codex Review: Didn't find any major issues. Breezy!

Reviewed commit: 7acd368f6f

ℹ️ 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".

The TRL < 1.7.0 per-token-logps return downgrade was an exact-string replace anchored on the full return line incl. its comment, so a reformat (e.g. pre-commit) could silently no-op it and ship a 3-tuple to older TRL. Switch to a regex tolerant of comment/whitespace drift, and raise if the anchor stops matching (re.subn count != 1) instead of failing silently. Add a monkeypatched trl_version unit test asserting both arities, since CI only installs TRL >= 1.7.0 and never exercised the downgrade otherwise.

for more information, see https://pre-commit.ci

danielhanchen · 2026-07-08T05:12:36Z

@codex review

danielhanchen · 2026-07-08T05:51:17Z

@codex review

danielhanchen · 2026-07-08T05:54:52Z

@codex review

The SFT/DPO fake patch runs only checked the generated trainer parses. Also assert the shared QLoRA _is_quantized_model bf16 cast is neutralized (TRL 1.7's spelling, present in both sft_trainer and dpo_trainer), so a structural TRL change to that block is caught for SFT/DPO too, not just GRPO.

for more information, see https://pre-commit.ci

danielhanchen · 2026-07-08T06:18:40Z

@codex review

chatgpt-codex-connector · 2026-07-08T06:25:03Z

Codex Review: Didn't find any major issues. Delightful!

Reviewed commit: 56cbb64243

ℹ️ 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".

The elif is_peft_model(model) and args.beta != 0.0: ref-adapter block was introduced in TRL 1.4.0 and is unchanged through 1.7.x, but the removal was gated at >= 1.7.0, so for 1.4 <= TRL < 1.7 the transform fell through to the 0.27 branch (which matches the older if is_peft_available()... form) and silently no-oped: a PEFT + beta != 0 GRPO run then computed the KL reference from the copied ref adapter instead of the base model. Lower the gate to 1.4.0 and keep the 1.7.0-only router aux-loss fail-fast nested. Widen the pinned-symbol contract test to run from 1.4.0 so the covered versions are actually exercised.

for more information, see https://pre-commit.ci

danielhanchen · 2026-07-08T10:41:56Z

@codex review

chatgpt-codex-connector · 2026-07-08T10:48:14Z

Codex Review: Didn't find any major issues. You're on a roll.

Reviewed commit: f390d679f2

ℹ️ 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".

danielhanchen

Verified the TRL >= 1.7.0 GRPO transforms against real TRL 1.7.1 and the version-compat contract tests; the PEFT ref-adapter removal, QLoRA cast neutralization, and 3-tuple arity gate are correct. Also lowered the PEFT-removal gate to the TRL 1.4.0 floor where the elif block first appeared. Fake-run and CPU fake-train validated green.

marcandrelarochelle requested review from Datta0, danielhanchen and pluesclues as code owners July 6, 2026 13:28

marcandrelarochelle changed the title ~~Fix PEFT replacement for TRL >= 1.7.0, add missing compute_aux_loss for TRL >= 1.7.0~~ (GRPO) Fix PEFT replacement for TRL >= 1.7.0, add missing compute_aux_loss for TRL >= 1.7.0 Jul 6, 2026

gemini-code-assist Bot reviewed Jul 6, 2026

View reviewed changes

unslothai deleted a comment from chatgpt-codex-connector Bot Jul 7, 2026

chatgpt-codex-connector Bot reviewed Jul 7, 2026

View reviewed changes

danielhanchen mentioned this pull request Jul 7, 2026

GRPO: default router_aux_loss_coef to 0 on TRL >= 1.7.0 #6938

Merged

chatgpt-codex-connector Bot reviewed Jul 7, 2026

View reviewed changes

marcandrelarochelle and others added 4 commits July 7, 2026 13:20

Fix PEFT replacement for TRL >= 1.7.0, add missing compute_aux_loss f…

d1d57df

…or TRL 1.7.0

[pre-commit.ci] auto fixes from pre-commit.com hooks

4d86b7d

for more information, see https://pre-commit.ci

Return zero aux_loss placeholder and drop inference-mode aux collection

8dfd996

danielhanchen force-pushed the feature/grpo-trl-1.7.0 branch from 7969160 to 8dfd996 Compare July 7, 2026 13:23

chatgpt-codex-connector Bot reviewed Jul 7, 2026

View reviewed changes

danielhanchen and others added 2 commits July 7, 2026 13:53

[pre-commit.ci] auto fixes from pre-commit.com hooks

ab50303

for more information, see https://pre-commit.ci

chatgpt-codex-connector Bot reviewed Jul 7, 2026

View reviewed changes

chatgpt-codex-connector Bot reviewed Jul 8, 2026

View reviewed changes

fake-run test: do not skip on import failure

f2870e6

unsloth/trl are installed in the grpo-fake-run job, so a failing import is the import-time drift this canary must catch. Keep only the not-installed find_spec skips; let a real import error fail the test.

danielhanchen mentioned this pull request Jul 8, 2026

GRPO: forward TRL 1.7 VLM kwargs (spatial_shapes / num_tiles / image_position_ids) in the logprob replacement #6960

Open

[pre-commit.ci] auto fixes from pre-commit.com hooks

7acd368

for more information, see https://pre-commit.ci

danielhanchen and others added 2 commits July 8, 2026 05:11

[pre-commit.ci] auto fixes from pre-commit.com hooks

1569df8

for more information, see https://pre-commit.ci

unslothai deleted a comment from chatgpt-codex-connector Bot Jul 8, 2026

danielhanchen and others added 2 commits July 8, 2026 05:55

[pre-commit.ci] auto fixes from pre-commit.com hooks

56cbb64

for more information, see https://pre-commit.ci

danielhanchen mentioned this pull request Jul 8, 2026

version-compat CI: fake CPU training runs for SFT/GRPO/DPO #6965

Merged

danielhanchen and others added 2 commits July 8, 2026 10:39

[pre-commit.ci] auto fixes from pre-commit.com hooks

f390d67

for more information, see https://pre-commit.ci

danielhanchen approved these changes Jul 8, 2026

View reviewed changes

danielhanchen merged commit 07c8bbb into unslothai:main Jul 8, 2026
44 of 47 checks passed

This was referenced Jul 22, 2026

fix/pr 7333 persisted plugin repro ci Imagineer99/unsloth#48

Closed

fix/pr 7328 positional repro ci Imagineer99/unsloth#49

Closed

		if compute_aux_loss and getattr(outputs, "aux_loss", None) is not None:
		all_aux_losses.append(outputs.aux_loss)

Uh oh!

Uh oh!

Conversation

marcandrelarochelle commented Jul 6, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Jul 6, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

danielhanchen commented Jul 6, 2026

Fixes pushed

Follow-up needed (MoE aux loss is not yet functional)

Uh oh!

marcandrelarochelle commented Jul 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

danielhanchen commented Jul 7, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Jul 7, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Jul 7, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

danielhanchen commented Jul 7, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Jul 7, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

danielhanchen commented Jul 7, 2026

Uh oh!

danielhanchen commented Jul 7, 2026

Uh oh!

danielhanchen commented Jul 7, 2026

Uh oh!

danielhanchen commented Jul 7, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Jul 7, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

danielhanchen commented Jul 7, 2026

Uh oh!

danielhanchen commented Jul 7, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Jul 7, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

danielhanchen commented Jul 7, 2026

Uh oh!

marcandrelarochelle commented Jul 6, 2026 •

edited

Loading