Experimental: correct varlen sample packing for hybrid linear-attention models - #7249
Conversation
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
…ence contamination
… for packing guard
Feed seq_idx to the causal conv and cu_seqlens to the gated-delta scan so sample packing / padding-free reset state at sequence boundaries for hybrid linear-attention models (Qwen3.5, Qwen3-Next). Gated behind UNSLOTH_EXPERIMENTAL_HYBRID_PACKING and fail-closed: when the flag is off or the accelerated kernels (causal_conv1d + fla) are unavailable, the guard keeps these models on the padded path.
for more information, see https://pre-commit.ci
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces support for sample packing and padding-free training on hybrid linear-attention models (such as Qwen3.5) by dynamically patching their gated-delta conv and scan kernels with sequence boundary metadata. It also refines packing behavior for vision-language models (VLMs) and adds comprehensive unit tests. The reviewer feedback highlights two important issues: first, position_ids could be passed as a positional argument, which would bypass the varlen boundary extraction in the patched forward pass; second, on Windows platforms, CRLF line endings could cause the source code string replacements in rl_replacements.py to fail silently. Addressing these issues will ensure robust and cross-platform behavior.
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.
| if varlen is not None and kwargs.get("cu_seqlens") is None: | ||
| kwargs["cu_seqlens"] = varlen[0] | ||
| return _orig(*args, **kwargs) | ||
|
|
||
| module.causal_conv1d_fn = conv_fn | ||
| module.chunk_gated_delta_rule = scan_fn | ||
| module._unsloth_varlen = None | ||
| module._unsloth_varlen_wrapped = True | ||
|
|
||
| # Refresh the boundary stash on the outermost forward (runs once per step, | ||
| # outside gradient-checkpoint recompute, so it stays valid for recomputed | ||
| # inner forwards of the same batch). | ||
| if not getattr(model, "_unsloth_varlen_forward_wrapped", False): | ||
| forward_orig = model.forward | ||
|
|
There was a problem hiding this comment.
If position_ids is passed as a positional argument (which can happen during torch.compile, tracing, or custom training loops), kwargs.get("position_ids") will return None. This silently bypasses the varlen boundary metadata extraction, leading to state leakage across sequence boundaries during training. We can inspect the signature of forward_orig once to robustly extract position_ids from both positional and keyword arguments.
if not getattr(model, "_unsloth_varlen_forward_wrapped", False):
forward_orig = model.forward
try:
sig = inspect.signature(forward_orig)
pos_ids_idx = list(sig.parameters.keys()).index("position_ids")
except (ValueError, TypeError):
pos_ids_idx = None
@wraps(forward_orig)
def forward_with_varlen(*args, **kwargs):
position_ids = kwargs.get("position_ids")
if position_ids is None and pos_ids_idx is not None and len(args) > pos_ids_idx:
position_ids = args[pos_ids_idx]
try:
varlen = _varlen_from_position_ids(position_ids)
except Exception:
varlen = None
for module in gated_delta_modules:
module._unsloth_varlen = varlen
return forward_orig(*args, **kwargs)
model.forward = forward_with_varlen
model._unsloth_varlen_forward_wrapped = TrueThere was a problem hiding this comment.
At head the wrapper binds positional args to names before reading position_ids (packing.py:474, forward_sig.bind_partial(*args).arguments), so a positionally-passed position_ids is not bypassed; the metadata path also prefers packed_seq_lengths and is fail-closed behind UNSLOTH_EXPERIMENTAL_HYBRID_PACKING.
| function = inspect.getsource(fast_sft_prepare_dataset) | ||
| function = function.replace( |
There was a problem hiding this comment.
On Windows platforms, files checked out with Git may have CRLF (\r\n) line endings. Since inspect.getsource returns the raw source code of the function, the multiline .replace calls with \n will fail to match, silently skipping the code modifications. Normalizing the line endings to \n first ensures the replacements work reliably across all platforms.
| function = inspect.getsource(fast_sft_prepare_dataset) | |
| function = function.replace( | |
| function = inspect.getsource(fast_sft_prepare_dataset).replace("\r\n", "\n") | |
| function = function.replace( |
There was a problem hiding this comment.
inspect.getsource returns universal-newline-normalized text (read via linecache/tokenize.open in text mode, PEP 278), so even a CRLF-on-disk file yields \n and the existing .replace(...\n...) matches on Windows; the suggested .replace("\r\n","\n") would be a no-op here.
Make patch_hybrid_linear_attention_varlen robust across transformers 4.57.6 through 5.x and TRL 0.22.2 through 1.x, following the import_fixes.py style: - Read UNSLOTH_EXPERIMENTAL_HYBRID_PACKING at call time so the flag takes effect when set after importing unsloth. - Idempotent: repeat calls on a patched model return True without re-validating the wrappers or double-wrapping; signatures are checked on captured originals. - Prefer the authoritative packed_seq_lengths (via get_packed_info_from_kwargs) over position_ids resets, handling pad_to_multiple_of trailing tokens. - Suppress injection for cached forwards (use_cache / past_key_values) so generation and eval are left on the untouched decode path. - Validate every gated-delta module before mutating any (transactional). - Bind position_ids / use_cache from both positional and keyword args. - Verify dispatch at runtime (Unsloth wraps each module forward, so the mixer source is not statically inspectable) and warn once if the shim is never hit. - Emit one deduped diagnostic on each fail-closed path. Add CPU unit tests covering the hybrid guard detection, the boundary builders, and the shim (fail-closed, active, idempotent, cached no-op, runtime handshake).
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: 3a1778c6a9
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| try: | ||
| hybrid_varlen_active = patch_hybrid_linear_attention_varlen(model) | ||
| except Exception: |
There was a problem hiding this comment.
When SFTConfig(loss_type="chunked_nll") is used with the hybrid packing flag, this patch is installed before original_init, but current TRL installs its chunked-CE model.forward during SFTTrainer.__init__ and the labels path calls the backbone directly rather than the captured outer forward. That overwrites/bypasses forward_with_varlen, so packed hybrid training proceeds with _unsloth_varlen unset and the conv/scan wrappers do not inject seq_idx/cu_seqlens, reintroducing the cross-sample state leak the guard is meant to prevent.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in f4a5b09. _chunked_loss_bypasses_forward detects the TRL chunked_nll path (loss_type None or chunked_nll, _patch_chunked_ce_lm_head present, Liger off) and keeps the hybrid model on the padded path. That loss calls the backbone directly and bypasses the outer forward wrapper that refreshes the _unsloth_varlen stash, so packing there would reintroduce the leak. Covered by test_hybrid_chunked_loss_stays_on_padded_path.
| model = args[0] if len(args) >= 1 else kwargs.get("model") | ||
| is_vlm = False | ||
| is_unsupported_model = False | ||
| is_hybrid = False | ||
| hybrid_varlen_active = False | ||
| if model is not None: | ||
| model_config = getattr(model, "config", None) | ||
| if model_config is not None: | ||
| model_types = get_transformers_model_type(model_config) | ||
| is_unsupported_model = any(x in PADDING_FREE_BLOCKLIST for x in model_types) | ||
|
|
||
| architectures = getattr(model_config, "architectures", None) | ||
| if architectures is None: | ||
| architectures = [] | ||
| is_vlm = any(x.endswith("ForConditionalGeneration") for x in architectures) | ||
| is_vlm = is_vlm or hasattr(model_config, "vision_config") | ||
|
|
||
| processing_class = kwargs.get("processing_class") or kwargs.get("tokenizer") | ||
| data_collator = kwargs.get("data_collator") | ||
| is_vlm = _is_vlm_config(model_config, model_types) | ||
| is_hybrid = _is_hybrid_linear_attention_model(model) |
There was a problem hiding this comment.
If callers use the documented SFTTrainer(model="Qwen/Qwen3-Next-...", args=SFTConfig(packing=True)) form, model is still a string here, so no config/modules are inspected, is_hybrid remains false, and the new hybrid block never runs before original_init loads the actual model. In that context enable_sample_packing is still applied after init, but the GatedDeltaNet kernels were never patched or blocked, so packed Qwen3-Next batches can silently leak state across samples.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in f4a5b09. When model is a string, _resolve_string_model_config loads the config via AutoConfig (forwarding trust_remote_code / revision / subfolder from model_init_kwargs) before original_init, and a detected string hybrid is kept on the padded path since the per-instance shim has no built module to patch yet. Covered by test_string_hybrid_model_disables_packing.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3a1778c6a9
ℹ️ 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 varlen is not None and not getattr(model, "_unsloth_varlen_handshake_done", False): | ||
| model._unsloth_varlen_handshake_done = True | ||
| if not any(getattr(m, "_unsloth_varlen_hit", False) for m in gated_delta_modules): | ||
| _hybrid_reject( | ||
| "varlen shim never invoked on a packed batch (dispatch changed?)" | ||
| ) |
There was a problem hiding this comment.
When a supported-looking mixer does not actually dispatch through the patched instance kernel attributes, this branch only logs a warning; it leaves the shim marked active and the SFTTrainer continues using packed/padding-free batches. In that scenario neither seq_idx nor cu_seqlens reaches the real kernels, so recurrent/conv state still crosses sample boundaries and silently corrupts the opt-in training results. Make the failed handshake deactivate packing (or raise before subsequent training batches) rather than merely reporting it.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in bcb5157. The forward wrapper now tracks conv and scan dispatch separately per gated-delta module (_unsloth_varlen_conv_hit / _unsloth_varlen_scan_hit) and requires both to fire on the first packed forward. If any module fails the handshake it nulls the varlen stash and raises RuntimeError before the loss/backward runs, so a failed dispatch aborts the step rather than training on leaked cross-sample state. Covered by the conv-only / scan-only / no-dispatch abort tests in tests/utils/test_packing.py.
| or is_processor | ||
| or is_auto_processor_vlm | ||
| or is_vision_dataset | ||
| or is_unsupported_model | ||
| or (is_hybrid and not hybrid_varlen_active) |
There was a problem hiding this comment.
For text-only T5ForConditionalGeneration, BartForConditionalGeneration, and Whisper configurations, _is_vlm_config is false and there are no vision columns, so this now leaves blocked false and enables packing (the new test explicitly asserts that behavior). The packed metadata is only consumed by the causal-model attention patches, while enable_sample_packing removes the normal attention_mask; these encoder-decoder models therefore attend across concatenated samples instead of respecting sequence boundaries, corrupting their training inputs. Continue allowing decoder-only text VLMs, but separately block config.is_encoder_decoder models.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in f4a5b09. trainer.py now resolves is_encoder_decoder up front and blocks those configs before packing is enabled (with a dedicated encoder-decoder reason string). Decoder-only text VLMs still take the packing path. Added test_encoder_decoder_disables_packing (t5 / bart / whisper) and test_decoder_only_conditional_generation_keeps_packing (csm) to lock the split.
The runtime handshake used a single per-module hit flag written by both the conv and scan wrappers, so a partial dispatch (only one kernel routed through self.<kernel>) passed the any() check and trained on contaminated data, and a missing dispatch only logged a warning. Track conv and scan dispatch separately, require both on every gated-delta module on the first packed forward, and raise before loss/backward when either is missing (the batch is already flattened, so there is no padded recovery at that point). Also skip an empty packed_seq_lengths before it reaches max(), and document the position_ids fallback's left-pad assumption. Add tests for no-dispatch and partial (conv-only / scan-only) abort, the packed_seq_lengths preference over a competing position_ids, MRoPE 3D position ids, and the pad_to_multiple_of trailing-segment path through the metadata builder.
for more information, see https://pre-commit.ci
…string-name models The varlen shim only helps decoder-only hybrid models that run their mixer through self.<kernel> on a live nn.Module forward. Three cases slipped past the guard: - Encoder-decoder configs (is_encoder_decoder) reached the packing path even though flattening a cross-attention batch is unsound. Block them explicitly. - TRL's chunked_nll loss (the 1.x default) calls the backbone directly and bypasses model.forward, so the per-instance forward wrapper that refreshes the varlen stash never runs. Detect that path and keep the model padded. - A string model_name reaches the trainer before the module exists, so the instance shim has nothing to patch. Resolve the config up front and keep string hybrids on the padded path. Adds encoder-decoder / decoder-only / chunked-loss / string-model tests.
|
@codex review |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
…r string models The wrapped-packing injection rewrote the sourced unsloth_zoo sft_prepare_dataset with str.replace anchored on the exact 'All Unsloth Zoo code licensed under LGPLv3' comment. str.replace never raises on a missing anchor, so a supported newer unsloth_zoo (the dependency is only lower-bounded) that moved that header would silently drop the setup while the truncation and pack_dataset edits still referenced _unsloth_wrapped_packing / _inspect, raising NameError on every SFT dataset preparation. - Install the setup at the sft_prepare_dataset signature via re.subn (a structural anchor that always exists) and raise if even that is missing. - Route the remaining edits through a _require_replace helper that fails loudly on a missing required anchor (or warns once for an optional one), formalizing the verify-then-replace idiom the DPO patchers in this file already use. - Reuse the guarded _unsloth_pack_has_strategy at the pack_dataset call instead of re-calling inspect.signature(pack_dataset) unguarded, so a non-introspectable pack_dataset cannot crash there after the setup already handled it. - _resolve_string_model_config now forwards token / use_auth_token / cache_dir / code_revision, so a private hybrid resolves its config instead of falling through as non-hybrid and enabling packing without the varlen shim. Adds regression tests for the drift-resistant injection, the helper, and the string-model auth forwarding.
|
@codex review |
for more information, see https://pre-commit.ci
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9117ec558a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| try: | ||
| from transformers import AutoConfig | ||
|
|
||
| init_kwargs = getattr(config_arg, "model_init_kwargs", None) or {} |
There was a problem hiding this comment.
When callers use a string model with SFTConfig(trust_remote_code=True, packing=True) and leave model_init_kwargs empty, this probe drops the trust flag even though TRL later merges args.trust_remote_code before loading string models (TRL source). For remote-code hybrid models, AutoConfig can fail here, model_config stays None, and the guard treats the model as non-hybrid, so packing is enabled without the varlen shim. This is fresh evidence beyond the earlier string-model thread: f8 forwards trust_remote_code only when it is already inside model_init_kwargs, not when it is set on the SFT config.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in b343c7e. Confirmed against TRL: it does model_init_kwargs.setdefault("trust_remote_code", args.trust_remote_code) before create_model_from_path, so a remote-code hybrid set with the common SFTConfig(trust_remote_code=True) was missed by the model_init_kwargs-only probe. _resolve_string_model_config now merges the top-level trust_remote_code via forward.setdefault (model_init_kwargs wins), mirroring TRL. Covered by test_resolve_string_model_config_merges_top_level_trust_remote_code.
…odel
TRL merges the top-level args.trust_remote_code into the load via
model_init_kwargs.setdefault("trust_remote_code", args.trust_remote_code) before
create_model_from_path, so a remote-code hybrid is commonly set with
SFTConfig(trust_remote_code=True) rather than inside model_init_kwargs. The config
probe only read model_init_kwargs, so AutoConfig could fail for such a model, leave
model_config None, and let the guard treat it as non-hybrid, enabling packing
without the varlen shim. Mirror TRL's setdefault (model_init_kwargs wins).
|
@codex review |
|
Codex Review: Didn't find any major issues. Another round soon, please! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
@codex review |
1 similar comment
|
@codex review |
|
Codex Review: Didn't find any major issues. What shall we delve into next? Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
Resolve conflicts after #7211 landed in main. #7249 stacks on #7211 and carries the more general versions of the shared code, so keep them: - trainer.py: the varlen shim gating (hybrid_varlen_active), the encoder-decoder block, and the string-model config resolution supersede the base hybrid guard. - rl_replacements.py: the _require_replace helper and _WRAPPED_PACKING_SETUP constant supersede the inline function.replace injection. - test_packing.py: keep the encoder-decoder / decoder-only split and the _require_replace / drift-resistant tests; drop the now-duplicated base fixtures.
|
@codex review |
for more information, see https://pre-commit.ci
…on models (unslothai#7249) * Fix text-only VLM CPT packing truncation * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Handle streaming vision datasets in packing * Harden multimodal packing detection * Preserve safe packing boundaries * Scope stream packing checks to VLMs * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Narrow VLM packing detection * Align packing mode and eval safety * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Add qwen3_5/qwen3_next to PADDING_FREE_BLOCKLIST to avoid packed-sequence contamination * Detect hybrid linear-attention models structurally instead of by name for packing guard * Add experimental varlen packing for hybrid linear-attention models Feed seq_idx to the causal conv and cu_seqlens to the gated-delta scan so sample packing / padding-free reset state at sequence boundaries for hybrid linear-attention models (Qwen3.5, Qwen3-Next). Gated behind UNSLOTH_EXPERIMENTAL_HYBRID_PACKING and fail-closed: when the flag is off or the accelerated kernels (causal_conv1d + fla) are unavailable, the guard keeps these models on the padded path. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Harden hybrid linear-attention varlen packing shim Make patch_hybrid_linear_attention_varlen robust across transformers 4.57.6 through 5.x and TRL 0.22.2 through 1.x, following the import_fixes.py style: - Read UNSLOTH_EXPERIMENTAL_HYBRID_PACKING at call time so the flag takes effect when set after importing unsloth. - Idempotent: repeat calls on a patched model return True without re-validating the wrappers or double-wrapping; signatures are checked on captured originals. - Prefer the authoritative packed_seq_lengths (via get_packed_info_from_kwargs) over position_ids resets, handling pad_to_multiple_of trailing tokens. - Suppress injection for cached forwards (use_cache / past_key_values) so generation and eval are left on the untouched decode path. - Validate every gated-delta module before mutating any (transactional). - Bind position_ids / use_cache from both positional and keyword args. - Verify dispatch at runtime (Unsloth wraps each module forward, so the mixer source is not statically inspectable) and warn once if the shim is never hit. - Emit one deduped diagnostic on each fail-closed path. Add CPU unit tests covering the hybrid guard detection, the boundary builders, and the shim (fail-closed, active, idempotent, cached no-op, runtime handshake). * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Abort hybrid packing when the varlen shim is not fully dispatched The runtime handshake used a single per-module hit flag written by both the conv and scan wrappers, so a partial dispatch (only one kernel routed through self.<kernel>) passed the any() check and trained on contaminated data, and a missing dispatch only logged a warning. Track conv and scan dispatch separately, require both on every gated-delta module on the first packed forward, and raise before loss/backward when either is missing (the batch is already flattened, so there is no padded recovery at that point). Also skip an empty packed_seq_lengths before it reaches max(), and document the position_ids fallback's left-pad assumption. Add tests for no-dispatch and partial (conv-only / scan-only) abort, the packed_seq_lengths preference over a competing position_ids, MRoPE 3D position ids, and the pad_to_multiple_of trailing-segment path through the metadata builder. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Import the hybrid packing patch from its submodule to satisfy the import-hoist lint * Fail closed for hybrid packing on encoder-decoder, chunked-loss, and string-name models The varlen shim only helps decoder-only hybrid models that run their mixer through self.<kernel> on a live nn.Module forward. Three cases slipped past the guard: - Encoder-decoder configs (is_encoder_decoder) reached the packing path even though flattening a cross-attention batch is unsound. Block them explicitly. - TRL's chunked_nll loss (the 1.x default) calls the backbone directly and bypasses model.forward, so the per-instance forward wrapper that refreshes the varlen stash never runs. Detect that path and keep the model padded. - A string model_name reaches the trainer before the module exists, so the instance shim has nothing to patch. Resolve the config up front and keep string hybrids on the padded path. Adds encoder-decoder / decoder-only / chunked-loss / string-model tests. * Harden the SFT source-injection replacements and forward auth args for string models The wrapped-packing injection rewrote the sourced unsloth_zoo sft_prepare_dataset with str.replace anchored on the exact 'All Unsloth Zoo code licensed under LGPLv3' comment. str.replace never raises on a missing anchor, so a supported newer unsloth_zoo (the dependency is only lower-bounded) that moved that header would silently drop the setup while the truncation and pack_dataset edits still referenced _unsloth_wrapped_packing / _inspect, raising NameError on every SFT dataset preparation. - Install the setup at the sft_prepare_dataset signature via re.subn (a structural anchor that always exists) and raise if even that is missing. - Route the remaining edits through a _require_replace helper that fails loudly on a missing required anchor (or warns once for an optional one), formalizing the verify-then-replace idiom the DPO patchers in this file already use. - Reuse the guarded _unsloth_pack_has_strategy at the pack_dataset call instead of re-calling inspect.signature(pack_dataset) unguarded, so a non-introspectable pack_dataset cannot crash there after the setup already handled it. - _resolve_string_model_config now forwards token / use_auth_token / cache_dir / code_revision, so a private hybrid resolves its config instead of falling through as non-hybrid and enabling packing without the varlen shim. Adds regression tests for the drift-resistant injection, the helper, and the string-model auth forwarding. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Honor top-level SFTConfig.trust_remote_code when resolving a string model TRL merges the top-level args.trust_remote_code into the load via model_init_kwargs.setdefault("trust_remote_code", args.trust_remote_code) before create_model_from_path, so a remote-code hybrid is commonly set with SFTConfig(trust_remote_code=True) rather than inside model_init_kwargs. The config probe only read model_init_kwargs, so AutoConfig could fail for such a model, leave model_config None, and let the guard treat it as non-hybrid, enabling packing without the varlen shim. Mirror TRL's setdefault (model_init_kwargs wins). * Tighten hybrid-packing comments for concision * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: alkinun <alkinunl@gmail.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Etherl <61019402+Etherll@users.noreply.github.com>
3 participants
Summary
Follow up to #7211. That PR re-enables sample packing / padding-free for text-only VLM training. Hybrid linear-attention models (Qwen3.5, Qwen3-Next) need extra handling: they mix a gated-delta recurrence with a causal conv1d, and packing flattens the batch into one row, so both operators leak state across packed sequence boundaries. #7211 keeps them on the padded path via a structural guard (
_is_hybrid_linear_attention_model).This PR makes packing actually correct for those models, behind an opt-in flag, by feeding the boundary metadata the kernels already accept.
What it does
patch_hybrid_linear_attention_varlen(model)inunsloth/utils/packing.py. For eachGatedDeltaNetinstance it wraps the two training/prefill kernel callables (self.causal_conv1d_fn,self.chunk_gated_delta_rule) and injectsseq_idx(conv) andcu_seqlens(scan). The boundaries come from the authoritativepacked_seq_lengthswhen present (via the existingget_packed_info_from_kwargs), falling back to the padding-freeposition_idsresets. Metadata is refreshed once on the outerforward, so it stays valid through gradient-checkpoint recomputation.trainer.pycalls it when it detects a hybrid model. If it returns True (flag on and kernels present) packing is allowed; otherwise the Fix text-only VLM CPT packing truncation #7211 guard keeps the model on the padded path.Safety
UNSLOTH_EXPERIMENTAL_HYBRID_PACKING=1(read at call time, so it works when set afterimport unsloth).causal_conv1d+fla) are absent, a pure-torch fallback is active, or the kernel signatures do not accept the boundary args, the patch returns False and the model stays padded. The default code path is unchanged.causal_conv1d_update,recurrent_gated_delta_rule) are untouched, and cached forwards (use_cache/past_key_values) never receive varlen metadata, so generation and eval are unaffected.Robustness and compatibility
Hardened following the
import_fixes.pyhouse style (feature-detect, fail closed, idempotent, deduped diagnostics):GatedDeltaNetinstance is validated before any is mutated.position_ids/use_cachefrom both positional and keyword arguments.inspect.getsourcecannot see the mixer body. Instead the shim records whether it was actually invoked on the first packed forward and warns once if it was not (a future transformers that stops dispatching throughself.<kernel>is surfaced rather than silently wrong).Compatibility contract:
self.causal_conv1d_fn/self.chunk_gated_delta_rule) holds on 4.57.6 (Qwen3-Next only), all 5.x, and main. From 5.9.0 the mixer forwardsseq_idx/cu_seq_lens_qnatively; the shim only fills a value that isNone, so it never clobbers a real one. Qwen3.5 first appears in transformers 5.2.0.position_idsandpacked_seq_lengthscontract holds across 0.22.2 and 1.x.sys.platform.SFTTrainer. The GRPO packed path (rl_replacements.py) is already self-verifying (it compares the packed forward against the per-row forward and falls back to the padded loop on mismatch), so a hybrid model there is not silently corrupted; wiring the shim into that path for efficiency is a follow-up. vLLM is never patched; Qwen3.5 +fast_inferenceneeds vLLM >= 0.17.0 (Qwen3-Next >= 0.11.2).Numerical validation (Qwen3.5-2B, bf16, B200)
Packed run compared against the same sequences run independently:
seq_idxalone orcu_seqlensalone reintroduces the contamination, so both are load-bearing.patch_hybrid_linear_attention_varlenAPI: both thepacked_seq_lengthsandposition_idsmetadata paths collapse the second-segment contamination to the noise floor; the patch activates, is idempotent, and the cached path injects nothing.Unit tests (
tests/utils/test_packing.py, CPU) cover the guard detection, the boundary builders (includingpad_to_multiple_oftrailing tokens and no-op cases), and the shim (fail-closed, active, idempotent, cached no-op, runtime handshake).Notes
This stacks on #7211 (it depends on the structural guard added there), so the diff currently also shows the #7211 commits. Once #7211 lands this collapses to the varlen commits on top (
unsloth/utils/packing.py,unsloth/utils/__init__.py, the guard flip inunsloth/trainer.py, and the tests).The flag stays experimental until the full version matrix runs in CI. The default padded path for these models is unchanged.