Repository navigation
Fix incorrect tuple return annotations on forward methods returning a Tensor - #48359
Merged
Rocketknight1 merged 1 commit intoAug 28, 2026
Merged
Rocketknight1 merged 1 commit into
Rocketknight1 merged 1 commit into
Conversation
… Tensor A number of `forward` methods declare a `tuple[...]` return annotation but return a bare `torch.Tensor`. Same class of issue as huggingface#45208, fixed for `Qwen3MoeSparseMoeBlock` in huggingface#45352. Each of the 29 classes touched here was instantiated and called with a minimal config, and the returned object's type observed to be `torch.Tensor`. Annotation only: no runtime behaviour is affected. Eleven annotations were edited in `modular_*.py` sources and the corresponding `modeling_*.py` files regenerated with `utils/modular_model_converter.py`; `utils/check_modular_conversion.py` and `utils/check_copies.py` both pass.
Contributor
|
[For maintainers] Suggested jobs to run (before merge) run-slow: albert, canine, cohere, cohere2, cohere2_moe, cohere_compass, dbrx, dia, distilbert, gemma2, git, gpt_bigcode, internvl, minimax, minimax_m2, mixtral |
Contributor
CI recapDashboard: View test results in Grafana |
Rocketknight1
approved these changes
Aug 28, 2026
Rocketknight1
left a comment
Member
There was a problem hiding this comment.
Yes, this is a good type hint fix, and I appreciate you making the same fix everywhere in a single PR!
Rocketknight1
enabled auto-merge
August 28, 2026 13:57
|
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
Qubitium
added a commit
to ModelCloud/OMY
that referenced
this pull request
Aug 31, 2026
* Retry get_daily_ci_runs on stale GitHub API cache (huggingface#48374) * [debug] Poll GitHub API 10x to observe cache inconsistency in schedule runs Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Retry get_daily_ci_runs when GitHub API returns stale cache results The GitHub Actions search index (used for event=/branch= filters) can lag behind the database — different backend nodes return wildly different total_count values (190, 238, 311, 413 observed for the same URL within minutes), and the most-recent runs are missing from stale responses. Detect staleness by checking whether the current GITHUB_RUN_ID appears in the returned list; if absent, retry up to 3 times with a 30s delay. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * [debug] Use get_daily_ci_runs in debug job; add current_run_id param Add optional `current_run_id` parameter to `get_daily_ci_runs` so callers can supply a known schedule run ID for stale-cache detection when GITHUB_RUN_ID belongs to a non-schedule-triggered job (e.g. push). Update the debug workflow job to call the Python function directly (two quick back-to-back calls) instead of raw curl, so the retry logic is exercised end-to-end and visible in the CI logs. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * [debug] Force retry path with fake run ID to verify stale-detection logging Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Revert "[debug] Force retry path with fake run ID to verify stale-detection logging" This reverts commit e49340f. * [debug] Run get_daily_ci_runs 20 times to catch stale cache hit Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * [debug] Sleep 60s between get_daily_ci_runs calls to hit different cache nodes Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Increase get_daily_ci_runs max_attempts from 3 to 5 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Add unit tests for get_daily_ci_runs stale-cache retry logic - tests/utils/test_get_previous_daily_ci.py: 5 tests covering fresh hit (no retry), stale→fresh (one retry), all stale (max_attempts exhausted), no current_run_id (stale check skipped), and empty schedule fallback to workflow_run event - utils/get_previous_daily_ci.py: ruff formatting fix (long print line) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Infer stale-check eligibility from current run's workflow_id Instead of an explicit `current_run_id` argument, `get_daily_ci_runs` now always fetches the current run's metadata (GITHUB_RUN_ID) upfront and compares its `workflow_id` to the queried one: - Same workflow → stale-cache detection applies (max_attempts=5, 30s sleep) - Different workflow → skip (e.g. AMD CI querying Nvidia CI runs); left for a follow-up PR once the same-workflow case is confirmed stable Also adds a TODO comment on the event=workflow_run fallback path (AMD CI) and updates the 6 unit tests accordingly (new test for the different-workflow case; all tests account for the upfront current-run lookup call). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Gate stale-cache check on GITHUB_EVENT_NAME == 'schedule' Push- and dispatch-triggered runs won't appear in event=schedule results even when the API is fresh, causing spurious retries. Only enable the stale-cache retry loop when the current run is itself schedule-triggered. Adds a corresponding unit test (test_non_schedule_event_skips_stale_check). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Revert debug changes to self-scheduled-caller.yml Restore the workflow to its main-branch state; the debug polling job was only needed to verify the stale-cache behaviour. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Condense get_daily_ci_runs comments and refer to huggingface#48374 for rationale Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: ydshieh <ydshieh@users.noreply.github.com> * Fix `safe_open` mmap memory exhaustion on Windows by using `pread` backend (huggingface#48341) * Fix `safe_open` mmap memory exhaustion on Windows by using `pread` backend On Windows, memory-mapping safetensors files reserves copy-on-write commit charge for the entire file. For large multi-shard checkpoints this exhausts virtual memory. Switch to the `pread` backend on `win32` in both `_load_pretrained_model` and `MtpModel` loading paths. * Address review: add MPS backend fix to MtpModel, hoist backend selection before loop, use targeted sys.platform patch in test * Address review: remove tests * [ONNX] Skip affected models on torch 2.13 (two dynamo regressions) (huggingface#48191) * [ONNX] Skip affected models on torch >= 2.13 (two dynamo ONNX regressions) torch 2.13.0 introduced two regressions in dynamo ONNX export: - pytorch/pytorch#194381: aten.sub type-promotion failure for scalar - int_tensor - pytorch/pytorch#194382: aten.mul.Scalar missing ONNX decomposition Skip all 35 affected model classes under `EXPORT_SKIPS["onnx"]` when torch >= 2.13, using the existing skip infrastructure. Will be removed once the upstream PyTorch fixes land. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Scope ONNX skip to torch == 2.13.x only (auto-runs on 2.14+) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Add missing BigBird subclasses to torch 2.13 ONNX skip list Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * [ONNX] Clarify skip guard is torch == 2.13.x only (not >=) Update comment and skip reason strings to say `torch == 2.13` instead of `torch >= 2.13`, making it clear the guard auto-lifts on 2.14+. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * [ONNX] Fix the torch 2.13 scalar regressions instead of skipping the models Both regressions are a Python float meeting an integral tensor, whose promotion torch 2.13 mishandles: `1.0 - int_mask` crashes the decomposition pass (pytorch/pytorch#194381) and `int_mask * 2.0` reaches translation with no registered ONNX decomposition (pytorch/pytorch#194382). On the same torch, `1.0 - float_tensor` and `float_tensor * 2.0` export fine — so promoting the tensor operand up front, to the dtype the op already produces, is enough. The op and its overload are left alone, and the inserted cast carries the op's own `meta` because for these elementwise cases it is the same value. Decomposition also emits `mul.Scalar` with a *symbolic* second operand (a division result, not a literal). There is no constant to promote there, so that one is rewritten to `mul.Tensor`, which has the two-operand translation — the same rewrite `_fix_remainder_scalar` makes for the same reason. Reachable because the FX fixes run again right after `run_decompositions`. This drops the 39 skip entries: the 15 affected families export again on 2.13, including the vision models (Sam, SamHQ, GotOcr2, GroundingDino, SegGpt, EfficientLoFTR, DeepseekOcr2). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Resolve the promoted-op set on first use, not at import `exporter_onnx` is importable without torch — the CI job that imports transformers with PIL only proved this the hard way — and naming `torch.ops.aten.*` overloads in a module-level frozenset broke that with a `NameError` before anything ran. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Say what the `mul.Scalar` operand is, and check it Review raised two things. The `Node` check assumed a tensor operand: it is never one. Across the affected families all 33 sites are an `operator.truediv` result, i.e. a `SymFloat`, which `mul.Tensor`'s translation does take — so the rewrite stands, but the guard now names the forms that op accepts instead of trusting the node type, and anything else keeps the `mul.Scalar` overload and fails visibly in translation. The fixes are also deliberately not version-gated, which the docstring implied they were. Both rewrites are semantics-preserving on any torch — a cast to the dtype the op already produces, and an overload swap with the same meaning — so gating would only decide which torch exercises the path. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: ydshieh <ydshieh@users.noreply.github.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: IlyasMoutawwakil <moutawwakil.ilyas.tsi@gmail.com> Co-authored-by: Ilyas Moutawwakil <57442720+IlyasMoutawwakil@users.noreply.github.com> * [Qwen3VLMoe] Update `test_small_model_integration_test_batch` expected output (value drift) (huggingface#48376) Update Qwen3VLMoe batch integration test expected output (value drift) Co-authored-by: ydshieh <ydshieh@users.noreply.github.com> * [LongcatFlash] Fix test_longcat_generation_cpu: use device_map="cpu" to avoid MoE disk offload issue (huggingface#48377) * [LongcatFlash] Fix test_longcat_generation_cpu by using device_map="cpu" `device_map="auto"` causes accelerate to offload MoE expert weights to disk, which then fails to reload them due to an internal weight format incompatibility. Since the test already requires large CPU RAM, use `device_map="cpu"` to keep all weights in memory and avoid disk offloading entirely. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * [LongcatFlash] Update golden string and skip test_longcat_generation_cpu on small runners - `test_shortcat_generation`: update expected output to current model output (value drift) - `test_longcat_generation_cpu`: replace `@require_large_cpu_ram` with `@require_torch_accelerator_memory(memory=1100)` — the 562B parameter model requires ~1,047 GiB of bfloat16 weights, far exceeding the CI runner budget (84 GiB single / 168 GiB dual), and disk offloading fails due to MoE weight format incompatibility with accelerate Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * remove unused require_large_cpu_ram import Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: ydshieh <ydshieh@users.noreply.github.com> * fix: flash-attn fallback failing on torch2.13 (huggingface#48388) fix: probe the same flash-attn kernel version that the loader resolves * [docs] Pass ContinuousBatchingConfig and sliding window models (huggingface#48381) docs * Raise when a paged attention forward is called with no cache (huggingface#48297) * Fall back to the base attention implementation when a paged attention forward has no cache A standard forward on a model switched to a paged attention implementation (as done by ContinuousBatchingManager) silently computed non-causal attention: sdpa_attention_paged_forward treats cache as optional and mask creation early-exits to None for paged implementations. Now, when the continuous batching kwargs are absent, the paged sdpa/flash forwards delegate to their base implementation, the eager paged forward materializes a causal mask, and mask creation resolves 2D padding masks with the base implementation's mask function. * Raise instead of falling back when a paged attention forward has no cache Drops the redirect to the base implementation and the paged-aware mask creation, per review. `paged|sdpa` and `paged|eager` now refuse a call with no paged cache rather than silently attending bidirectionally. --------- Co-authored-by: Rémi Ouazan <83456801+remi-or@users.noreply.github.com> * Bump transformers-mlinter to 0.1.5 and clear the new findings (huggingface#48259) * future mlinter 0.1.5 * fix rule 58 * Add tokenizer tests for TRF038 Adds the missing tokenizer test files that mlinter's TRF038 reports, now that the rule covers tokenization_*.py files. * fix roberta * Allowlist the three alias-only tokenizer modules for TRF038 bart, mobilebert and squeezebert started firing when mlinter 0.1.5 extended TRF038 to tokenizer files. Their tokenization_*.py define no classes at all, each being a two-line back-compat alias left by "rm slow tokenizers" (huggingface#40936), which deleted their test files on purpose. A test file here could only assert alias identity, and the aliased tokenizers are already covered by tests/models/roberta and tests/models/bert. Allowlisted as a placeholder: the better fix is upstream, where TRF038 should skip a source file that defines no classes. TRF038 now reports no findings. * Route composite sub-configs through CONFIG_MAPPING for TRF009 mlinter 0.1.5 extended TRF009 from modeling files to every model file, which surfaced six configs importing a sibling model's config class directly. Each now resolves it through CONFIG_MAPPING with AutoConfig as the sub_configs marker, which is what the rule prescribes and what 99 other composite configs already do: cohere_asr ParakeetEncoderConfig -> parakeet_encoder esmfold2 EsmcConfig -> esmc gemma4_assistant Gemma4TextConfig -> gemma4_text gemma4_unified_assistant Gemma4UnifiedTextConfig -> gemma4_unified_text ovis2 Qwen2Config -> qwen2 sam3 CLIPTextConfig -> clip_text_model cohere_asr already used CONFIG_MAPPING for instantiation, so only its sub_configs value and import changed. Where the imported class also appeared in a type annotation it is widened to PreTrainedConfig, and esmfold2's _init_nested helper takes the class from CONFIG_MAPPING since it calls cls() directly. Verified: default construction and to_dict/from_dict round-trip resolve the same concrete config class as before for all six, and tests/models for the six models pass (552 passed, 551 skipped). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Backport mlinter rule-prose compression and allowlist the rest of TRF009 Two changes to utils/rules.toml. Backports transformers-mlinter d0ac218 ("compress the rule explanation prose", before: upstream's file as the base, with this repo's licence header and its allowlist_models entries and their justification comments re-applied on top, so the only remaining difference from the installed mlinter/rules.toml is the header and those allowlists. make fix-repo regenerated docs/source/en/modeling_rules.md. Allowlists the ten TRF009 findings left after the CONFIG_MAPPING fixes, in three groups, each with its reasoning recorded in the file: - Back-compat tokenizer shims (bart, convbert, distilbert, fnet, mobilebert, squeezebert): the cross-model import is the module's whole purpose. - generation_*.py (nemotron3_5_asr, nemotron_asr_streaming): the rule points at modular files, but the converter has no generation_*.py support, so the prescribed remedy does not exist yet. - sam3: CLIPTextModelWithProjection is needed for checkpoint key compatibility. shieldgemma2: fixable with a modular file, left as follow-up. make typing and make check-repo now pass. * Fix vibevoice for mlinter 0.1.5 after rebase VibeVoice (huggingface#40546) landed on main after this branch forked, so it was never checked against the 0.1.5 rules and brought in two findings. TRF058: VibeVoiceDiffusionHeadSinusoidalEmbedding created `freq` via `self.register_buffer(...)`, which cannot be inherited and tweaked from a modular file. Assign `nn.Buffer(..., persistent=False)` instead, which also makes the `freq: torch.Tensor` linting workaround unnecessary. Same non-persistent buffer and same forward output. TRF009: allowlist `vibevoice`. generation_vibevoice.py imports a runtime cache class from its own sub-model package; the family already routes its sub-configs through CONFIG_MAPPING, and a cache class cannot come from CONFIG_MAPPING. It also sits in a generation_*.py file, which the modular converter does not support, the same blocker as the nemotron entries. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Adopt transformers-mlinter main: sync rule prose, drop dead allowlist entries Re-ran the checker against transformers-mlinter git main (e6533da). Green, and the bump makes a chunk of this branch's curation obsolete. Synced the rule prose the repo copy mirrors, which had gone stale because the described behavior changed upstream: TRF029 -- an optional `None`-default parameter is now an override, not a second source of truth, so it is exempt TRF035 -- F401/F821/F822 are now accepted in modular files TRF041 -- guards (an `if` with no `else` that only raises or only logs) and a `DEFAULT_EXEMPT_ATTRIBUTES` list of framework plumbing are exempt Dropped 39 allowlist entries that no longer fire, mostly mlinter#27 (the rule engine now resolves a class's base chain across models instead of only inside the linted file), which this branch had flagged upstream rather than worked around: TRF018 16 -> 1 only `radio`, the one real case, is left TRF020 1 -> 0 the `axk2` false positive is gone TRF034 17 -> 5 the 12 cross-file false positives are gone; the 5 real findings stay as follow-ups TRF029 2 -> 1 TRF035 5 -> 0 TRF041 14 -> 9 Each rule's comment block now describes what is left and why. Verified minimal: emptying any of these six allowlists reproduces exactly the kept entries. 24 further entries (TRF002, TRF004, TRF005, TRF006, TRF009 `maskformer`, TRF010, TRF019, TRF056) are also dead, but they were already dead before this bump, so they are left alone here. TRF037's `x_clip` stays: the rule is opt-in, so it never fires and the entry documents what would. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Regenerate modeling_rules.md for the synced rule prose Generated from utils/rules.toml by utils/check_modeling_rules_doc.py; picks up the TRF029, TRF035 and TRF041 prose synced from transformers-mlinter main. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Drop 19 more allowlist entries that no longer fire Swept every rule the way the earlier mlinter#27 pass was done: emptied each allowlist, re-ran, and checked what actually fires. 213 entries -> 194. "Does not fire" has four causes, so each candidate was classified rather than deleted on the strength of a green run: - the model is compliant now - the pattern survives only in a generated file, copied from a parent - the model is grandfathered by cutoff_date, so it is never checked - the rule is opt-in and never runs at all Only the first two are safe to drop. To separate them, each candidate rule was run directly against the model's own source with grandfathering off. Removed as compliant, verified rule-by-rule against the source (14): TRF005 9 -> 0 the rule accepts an explicit `_no_split_modules = None`, a modular `AttributeError` sentinel, and raw-string elements TRF010 2 -> 0 both configs carry @strict(accept_kwargs=True) TRF006 4 -> 3 TRF009 15 -> 14 (maskformer) TRF056 2 -> 1 (kimi_k25) Removed as generated-only copies (5): TRF004 9 -> 4 hubert, sew, unispeech, unispeech_sat and wavlm define no `tie_weights` in their modular files; the override exists only in their generated modeling_*.py, copied from Wav2Vec2ForCTC. wav2vec2 still fires, so the root cause stays tracked -- listing all five implied five separate bugs. Kept deliberately, and now documented in the file (5): TRF002 lighton_ocr -- `base_model_prefix = ""` is real and authored in its own modular file; the rule misses it only because it resolves base chains within a single file, so `LightOnOcrModel(Mistral3Model)` is not recognized as a pretrained class TRF019 blip_2, grounding_dino, omdet_turbo -- grandfathered by cutoff_date; they still set `_defaults` and fire once grandfathering is off TRF037 x_clip -- the rule is opt-in, so the entry records what would fire Verified: emptying every allowlist now reproduces exactly the kept entries, except those 5, each of which has a stated reason. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Adopt the TRF034 trunk scoping from mlinter main mlinter#65 scoped TRF034 to the model's checkpointable token-mixing trunk. Sync the rule prose and cut the allowlist from 5 models to 1. `dinov3_convnext`, `openai` and `xcodec2` never set `supports_gradient_checkpointing`, so `gradient_checkpointing_enable()` raises for them rather than skipping a layer and there was nothing to report. `tipsv2_dpt`'s three entries are DPT head layers, not a token-mixing stack. `x_clip`'s `PromptGeneratorLayer` stays: an attention stack on a model that does enable checkpointing, so it is a real finding and a follow-up for that model. Over this checkout with every cutoff neutralised, TRF034 goes from 103 findings over 76 models to 20 over 19, and every survivor is a transformer stack in a checkpointable model. * Pin transformers-mlinter 0.1.5 --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> * [CB] Fix wrong device scoping (huggingface#48370) * Add device scoping * Added a regression test * [Improvement] Make gated delta rule more explicit (huggingface#47625) * MAke gdr more explicit and support per-channel decay * Nits and tests * regenerated * Delete the KDA mode of GDN * Bake in the UT transform * Review compliance 1/n * Apply suggestions from code review Co-authored-by: Anton Vlasjuk <73884904+vasqu@users.noreply.github.com> * Readability * Review 2/n * Memory and speed optims * Added test file * Fix export * Merge mess up fix * fix2 * Fix in place identity * Review compliance * second review 1/n * second review 2/n * second review 3/n * second review 4/n * second review 5/n * Fix repo * Last review * Remove test class * Make fix repo * Fix BPE tokenizer test --------- Co-authored-by: Anton Vlasjuk <73884904+vasqu@users.noreply.github.com> * fix some failure in xpu (huggingface#48252) * fix some failure in xpu Signed-off-by: Wang, Yi <yi.a.wang@intel.com> * fmt Signed-off-by: Wang, Yi <yi.a.wang@intel.com> * update Signed-off-by: Wang, Yi <yi.a.wang@intel.com> * update Signed-off-by: Wang, Yi A <yi.a.wang@intel.com> --------- Signed-off-by: Wang, Yi <yi.a.wang@intel.com> Signed-off-by: Wang, Yi A <yi.a.wang@intel.com> * Update Tailscale action version in workflow (huggingface#48394) * skip mtp slow tests for now (huggingface#48328) (huggingface#48329) skip mtp slow tests for now * [VibeVoice] Skip generate export tests (flaky) (huggingface#48396) * [VibeVoice] Skip generate export tests (flaky, decompose_prefill_decode incompatible) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * [VibeVoice] Skip flaky static-cache compile tests Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: ydshieh <ydshieh@users.noreply.github.com> * Fix interval merge invariant in _find_disjoint (huggingface#47860) Fix interval merge invariant in `_find_disjoint` `_find_disjoint` implements a sweep-line merge over tensor memory ranges, but assigns `last_stop = stop` instead of tracking the running maximum. When a tensor is strictly contained inside an earlier, larger one, `last_stop` shrinks to the contained tensor's end pointer, so a subsequent tensor that genuinely overlaps the larger one is classified as disjoint. Use `last_stop = max(last_stop, stop)`, the standard sweep-line invariant. Note on impact: this is a latent correctness fix, not a user-facing bug fix. Triggering the mis-classification requires strict containment, which in turn guarantees the resulting group is not "identical" in `_find_identical`, so `remove_tied_weights_from_state_dict` raises the same RuntimeError either way. Verified exhaustively over all 3- and 4-interval configurations (54k cases): the corrected version matches ground-truth connected components in 100% of cases, and the raise-vs-succeed outcome never differs. The observable improvement is that the error message now reports the complete set of overlapping tensors instead of a truncated one. * Fix incorrect tuple return annotations on forward methods returning a Tensor (huggingface#48359) A number of `forward` methods declare a `tuple[...]` return annotation but return a bare `torch.Tensor`. Same class of issue as huggingface#45208, fixed for `Qwen3MoeSparseMoeBlock` in huggingface#45352. Each of the 29 classes touched here was instantiated and called with a minimal config, and the returned object's type observed to be `torch.Tensor`. Annotation only: no runtime behaviour is affected. Eleven annotations were edited in `modular_*.py` sources and the corresponding `modeling_*.py` files regenerated with `utils/modular_model_converter.py`; `utils/check_modular_conversion.py` and `utils/check_copies.py` both pass. Co-authored-by: Gustavo <gustavo.vp350@gmail.com> * Avoid print to stdout that fails the job `check_failed_tests` job (huggingface#48391) fix Co-authored-by: ydshieh <ydshieh@users.noreply.github.com> * [Docs]: Update GLM 5.3 (huggingface#48401) * docs: update GLM 5.3 Signed-off-by: Shijin Zhang <75300765+Dovis01@users.noreply.github.com> * docs: update toc Signed-off-by: Shijin Zhang <75300765+Dovis01@users.noreply.github.com> * fix Signed-off-by: Shijin Zhang <75300765+Dovis01@users.noreply.github.com> --------- Signed-off-by: Shijin Zhang <75300765+Dovis01@users.noreply.github.com> --------- Signed-off-by: Wang, Yi <yi.a.wang@intel.com> Signed-off-by: Wang, Yi A <yi.a.wang@intel.com> Signed-off-by: Shijin Zhang <75300765+Dovis01@users.noreply.github.com> Co-authored-by: Yih-Dar <2521628+ydshieh@users.noreply.github.com> Co-authored-by: ydshieh <ydshieh@users.noreply.github.com> Co-authored-by: eryk-roch <eryk.roch.rybak@intel.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: IlyasMoutawwakil <moutawwakil.ilyas.tsi@gmail.com> Co-authored-by: Ilyas Moutawwakil <57442720+IlyasMoutawwakil@users.noreply.github.com> Co-authored-by: NanoCode012 <nano@axolotl.ai> Co-authored-by: Steven Liu <59462357+stevhliu@users.noreply.github.com> Co-authored-by: Quentin Gallouédec <45557362+qgallouedec@users.noreply.github.com> Co-authored-by: Rémi Ouazan <83456801+remi-or@users.noreply.github.com> Co-authored-by: Tarek Ziade <tarek@ziade.org> Co-authored-by: Anton Vlasjuk <73884904+vasqu@users.noreply.github.com> Co-authored-by: Wang, Yi <yi.a.wang@intel.com> Co-authored-by: Guillaume LEGENDRE <glegendre01@gmail.com> Co-authored-by: Vikas Sharma <vikasharmadev8@gmail.com> Co-authored-by: Gustavo Dias Apolinário <100954782+Gronoxx@users.noreply.github.com> Co-authored-by: Gustavo <gustavo.vp350@gmail.com> Co-authored-by: Shijin Zhang <75300765+Dovis01@users.noreply.github.com>
sbucaille
pushed a commit
to sbucaille/transformers
that referenced
this pull request
Sep 16, 2026
… Tensor (huggingface#48359) A number of `forward` methods declare a `tuple[...]` return annotation but return a bare `torch.Tensor`. Same class of issue as huggingface#45208, fixed for `Qwen3MoeSparseMoeBlock` in huggingface#45352. Each of the 29 classes touched here was instantiated and called with a minimal config, and the returned object's type observed to be `torch.Tensor`. Annotation only: no runtime behaviour is affected. Eleven annotations were edited in `modular_*.py` sources and the corresponding `modeling_*.py` files regenerated with `utils/modular_model_converter.py`; `utils/check_modular_conversion.py` and `utils/check_copies.py` both pass. Co-authored-by: Gustavo <gustavo.vp350@gmail.com>
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.
What does this PR do?
This is my first contribution to the repo, so let me be upfront about the size of it. The diff is 29 return annotations and no runtime behavior. I read CONTRIBUTING, including the part about avoiding busywork PRs and the note that code agents tend to zero in on theoretical bugs found by static analysis. That is exactly why I am starting with the use case instead of the scan.
I hit this in my own work, worked around it, and finished what I was doing. I am opening the PR now because the next person who hits it will spend the same time I did figuring out what went wrong, and the signature will keep telling them they are right the whole way.
How I ran into it
I hit this in code that needs to iterate
TransformerBlockobjects by hand instead of callingmodel(...). Nothing about it is special to the failure. Anyone stepping through the blocks manually is in the same position.My code came from 4.x. On
v4.57.6,TransformerBlock.forwarddid literally this:annotated
-> tuple[torch.Tensor, ...]. The annotation was correct, andx = block(x, attn)[0]was the right way to write it. On 5.x the body isreturn ffn_output, a plain Tensor. The annotation did not change. The signature still endorses the[0].Measured with the config from the reproducer below:
(4, 5, 32)(4, 5, 32)out[0](5, 32), 3 of the 4 samples goneIn my case the blocks were chained, so the next block received a rank 2 tensor where it expected rank 3 and raised
IndexError. I got an exception rather than wrong numbers.Anyone calling a single block without chaining, which is the normal shape of activation analysis, feature extraction and hooks, gets no exception at all. They get
(5, 32)where they expected(4, 5, 32)and move on.torch.allclose(out[0], out)also passes, since(5, 32)broadcasts against(4, 5, 32), so a naive equality check does not catch it either.What resolved it for me was reading the body of
forward, not the signature.Who this affects
It breaks people who iterate layers by hand: interpretability, pruning, early exit, distillation, activation analysis. Those are the people reading the signature on IDE hover, and the signature induces the
[0].It does not affect anyone using high level
Model.forward, which is most users, since they never touch these classes.The win for type checkers is honestly small. Measured with
ty0.0.75, the versionmake typinguses:blk(x, None)via__call__UnknownUnknownblk.forward(x, None)tuple[Tensor, ...]TensorPyTorch types
nn.Module.__call__asAny, so the annotation only becomes visible on a directforwardcall. Practical for people manipulating layers, theoretical for everyone else.This is not a one off
Comparing each class against
v4.57.6:v4.57.6tuple[...]annotation was correct,forwardchanged in 5.x and the annotation stayed behindtorch.Tensor, the two swapped sides in 5.xtuple[...]annotation, already wrong back thenv4.57.6The first row is the one that hurts:
AlbertLayerGroup,DbrxFFN,DbrxRouter,DiaEncoderLayer,Gemma2DecoderLayer,GitAttention,GitLayer,TransformerBlock,ZambaMambaDecoderLayer,Zamba2MambaDecoderLayer. Anyone who wrote[0]on 4.x, which was correct, and then migrated, still has a signature backing that[0].The five
SparseMoeBlockclasses (Mixtral, MiniMax, Qwen2-MoE, Qwen3-Next, Qwen3-Omni-Talker) are the inverted case. On 4.x they returned(final_hidden_states, router_logits)annotatedtorch.Tensor. Today they return the tensor annotatedtuple[torch.Tensor, torch.Tensor]. The two just swapped places.What changed
29 classes, 36 files, 40 lines. Every edited line has the same shape:
11 annotations were edited in
modular_*.py, the rest directly inmodeling_*.pyfor models without a modular file. Generated files came out ofutils/modular_model_converter.py. Several models changed by inheritance alone, for exampleMiniMaxSparseMoeBlock(MixtralSparseMoeBlock)andQwen3_5MoeSparseMoeBlock(Qwen3NextSparseMoeBlock(Qwen2MoeSparseMoeBlock)). Annotations only, no runtime behavior changes.How the list was built
An AST scan over every
modeling_*.pyfound 1734forwardmethods annotatedtuple[...], of which 76 return a single name that never receives a tuple construction anywhere in the function. Each of the 76 was instantiated and called with a minimal config, and the return type observed.Two were refuted that way,
Data2VecVisionSelfAttentionandFlavaSelfAttention, which do return tuples. They are out. The 29 in this PR were all confirmed at runtime against the patched code.One more that the verification killed:
RecurrentGemmaRecurrentBlockreturns a Tensor on 5.9.0, but onmainthe body isreturn (hidden_states, None). The annotation is correct today. It was on my list and I removed it.Checks
utils/check_modular_conversion.pyutils/check_copies.pyruff check(0.14.10, the version pinned insetup.py)ruff format --checkutils/checkers.py --fix(modular_conversion, copies, ruff_check, ruff_format, types, modeling_structure, imports, init_isort, inits, repo)I ran the
make fix-repocheckers that touch this patch with--fixand none of them wanted to change anything. The fullmake fix-repodoes not run on this machine, since some checkers need the completepip install -e '.[dev]'.Tests
CONTRIBUTING asks to run the tests covering the changed files.
utils/tests_fetcher.pyexpanded to around 500 files, since the diff touches modular files of base models that many others inherit from. So I ran the full suite for the 27 directly touched models twice, once on the branch and once on cleanmain.27 of 27 identical, same
passed,failedanderrorcounts on both sides. 4,267 tests executed (8,585 collected) and 46,029 subtests.One caveat worth declaring: a few failures show up on both sides because
accelerateis not installed on this machine, and installing it drags in a conflictingtorch. Those aretest_can_load_with_device_context_manager,test_can_load_with_global_device_set,test_causal_lm_can_accept_training_kwargs,test_generate_with_static_cacheandtest_generate_from_inputs_embeds_with_static_cache.Related work
The same defect was confirmed by a maintainer in issue #45208 and fixed for
Qwen3MoeSparseMoeBlockin PR #45352, which is merged.Open PR #43446,
[typings] Automatically type decorator return types as tuple | X, addresses the same family from another angle, on methods decorated withcan_return_tuple. It touches 12 of the same files but none of the 29 classes here, so the two are complementary.One adjacent finding I did not include:
Cohere2MoeTopKRouterhas the inverse problem, annotated-> torch.Tensorwhile returning(router_logits, router_scores, selected_experts). Happy to send that separately if you want it.Minimal reproducer
Code Agent Policy
The Transformers repo is currently being overwhelmed by a large number of PRs and issue comments written by
code agents. These often are low-quality, or fix extremely minor issues that occur rarely or never in practice.
As a result, we're instituting a rule that first-time contributors should not use code agents to submit PRs or issues.
We'd also ask autonomous "OpenClaw"-like agents not to open any PRs or issues.
Issues/PRs from first-time contributors that violate this rule will probably just be closed without review, and we
might block you, especially if you open more than one or appear to be deliberately ignoring this. We especially do not
want new contributors to jump in on random issues to contribute an agent-written fix. This creates lots of noise
for reviewers and other users and will almost certainly get you blocked.
For more information, please read
CONTRIBUTING.md.Before submitting
Pull Request checks?
to it if that's the case.
Three of these are unchecked on purpose. There is no issue covering this, so the closest thing to prior approval is the maintainer confirmation in #45208 and the merged #45352, both cited above. No docstrings changed, so there is nothing to document. I did not write new tests, since the change is annotation only, but I ran the existing suite for all 27 touched models on both sides and got identical results.
Who can review?
@Rocketknight1, since you reviewed and merged #45352, which is the same defect on one class.
Happy to adjust anything.
:)