Skip to content

[ONNX] Skip affected models on torch 2.13 (two dynamo regressions) - #48191

Merged
ydshieh merged 8 commits into
mainfrom
skip_onnx_torch213_regression
Aug 27, 2026
Merged

[ONNX] Skip affected models on torch 2.13 (two dynamo regressions)#48191
ydshieh merged 8 commits into
mainfrom
skip_onnx_torch213_regression

Conversation

@ydshieh

@ydshieh ydshieh commented Aug 21, 2026

Copy link
Copy Markdown
Collaborator

CPU CI GPU run-slow

Summary

torch 2.13.0 introduced two regressions in dynamo ONNX export that break 45 tests across 15 model families. This PR skips all affected model classes when torch >= 2.13 using the existing EXPORT_SKIPS infrastructure.

Root causes (both filed upstream):

  • pytorch/pytorch#194381aten.sub type-promotion pass crashes for scalar - int_tensor patterns (e.g. 1.0 - attention_mask) — step 2/3
  • pytorch/pytorch#194382aten.mul.Scalar has no registered ONNX decomposition for real-valued inputs — step 3/3

Verified: both bugs pass on torch 2.12.1, fail on torch 2.13.0 (cu126 and cu130), and still fail on nightly 2.14.0.dev20260811.

Changes

Single change to tests/exporters/test_export.py: adds a version-gated EXPORT_SKIPS["onnx"] update block (35 model classes, 2 issue references) that activates only when is_torch_greater_or_equal("2.13").

Cleanup

This block should be deleted once pytorch/pytorch#194381 and #194382 are resolved and a fixed torch version is available.

🤗 From the Hugging Face Transformers CI team.

ydshieh and others added 3 commits August 21, 2026 18:52
…ions)

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>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@ydshieh

ydshieh commented Aug 21, 2026

Copy link
Copy Markdown
Collaborator Author

run-slow: prophetnet, got_ocr2

@github-actions

Copy link
Copy Markdown
Contributor

Workflow Run ⚙️

This comment contains run-slow, running the specified jobs:

models: ["models/got_ocr2", "models/prophetnet"]
quantizations: []

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>
@ydshieh ydshieh changed the title [ONNX] Skip affected models on torch >= 2.13 (two dynamo regressions) [ONNX] Skip affected models on torch 2.13 (two dynamo regressions) Aug 21, 2026
@github-actions

Copy link
Copy Markdown
Contributor

CI Results

Workflow Run ⚙️

Commit Info

Context Commit Description
RUN d634cd8b workflow commit (merge commit)
PR 1a979889 branch commit (from PR)
main e453228e base commit (on main)

✅ No failing test specific to this PR 🎉 👏 !

@HuggingFaceDocBuilderDev

Copy link
Copy Markdown

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.

@IlyasMoutawwakil

Copy link
Copy Markdown
Member

a lot of those are actually important vision models 🥲 can you please ask your agent to add patches/fixes in the onnx backend instead ?

@ydshieh

ydshieh commented Aug 23, 2026

Copy link
Copy Markdown
Collaborator Author

a lot of those are actually important vision models 🥲 can you please ask your agent to add patches/fixes in the onnx backend instead ?

It's best to leave that for the ONNX people - they are already on that.

microsoft/onnxscript#3005

I can refine the condition, so as long as a new release (onnx script?) is out, those models would still be included and tested

@IlyasMoutawwakil

Copy link
Copy Markdown
Member

@justinchuby will the onnxscript fix these failures ? im asking bcz it'd be a shame to just leave all those vision models broken. asking bcz we can also fix the intermediate exported program to avoid the failures from our side.

@justinchuby

Copy link
Copy Markdown
Contributor

The first failure is a true pytorch regression. The second is something we can fix in onnxscript.

@justinchuby

justinchuby commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

But to fix that you may consider explicitly converting the python constant to a pytorch tensor in transformers, I think. That may be able to fix both cases.

@ydshieh

ydshieh commented Aug 24, 2026

Copy link
Copy Markdown
Collaborator Author

But to fix that you may consider explicitly converting the python constant to a pytorch tensor in transformers, I think. That may be able to fix both cases.

@IlyasMoutawwakil Is this something we want to do on transformers side?

@IlyasMoutawwakil

Copy link
Copy Markdown
Member

let me check, i will push fixes to this branch / pr

…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>
@IlyasMoutawwakil
IlyasMoutawwakil force-pushed the skip_onnx_torch213_regression branch from d6b9242 to 6fa59a8 Compare August 26, 2026 13:13
@IlyasMoutawwakil

Copy link
Copy Markdown
Member

run-slow: prophetnet, got_ocr2

@github-actions

Copy link
Copy Markdown
Contributor

Workflow Run ⚙️

This comment contains run-slow, running the specified jobs:

models: ["models/got_ocr2", "models/prophetnet"]
quantizations: []

@github-actions

Copy link
Copy Markdown
Contributor

CI Results

Workflow Run ⚙️

Commit Info

Context Commit Description
RUN ee408d64 workflow commit (merge commit)
PR 6fa59a8e branch commit (from PR)
main 36bc98ef base commit (on main)

✅ No failing test specific to this PR 🎉 👏 !

@ydshieh

ydshieh commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator Author

@IlyasMoutawwakil In order to check the progress, we have to revert the change I made (i.e. the skipping list) , otherwise the run-slow just not running the relevant tests and shows all green.

@ydshieh

ydshieh commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator Author

Ah sorry, you already made the changes, nice!

IlyasMoutawwakil and others added 2 commits August 26, 2026 16:00
`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>
@ydshieh

ydshieh commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator Author

@askserge review Ilyas beautiful elegant PR

sergereview[bot]
sergereview Bot previously requested changes Aug 27, 2026

@sergereview sergereview Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤗 Serge says:

The actual diff does not match the PR description at all. The description claims a single change to tests/exporters/test_export.py that adds 35 model classes to EXPORT_SKIPS["onnx"] gated by is_torch_greater_or_equal("2.13"), but the diff only modifies src/transformers/exporters/exporter_onnx.py and adds two FX-node rewrites instead of test skips. That needs to be reconciled before merge.

Correctness / scope

  • The new FX fixes (_fix_integral_tensor_float_scalar and _fix_mul_scalar_symbolic) are registered unconditionally, contradicting the description that they activate only on torch >= 2.13. If they are safe on older torch versions, the description must be updated; otherwise the fixes should be version-gated.
  • _fix_mul_scalar_symbolic rewrites aten.mul.Scalar to aten.mul.Tensor when the second argument is a Node. aten.mul.Tensor requires both operands to be tensors, but the Node may represent a scalar symbolic value (e.g. a SymFloat from a division). Please confirm that the rewritten Node is always a tensor; if not, this rewrite can introduce a new lowering failure.

Documentation

  • The PR description, summary, and cleanup note all refer to a skip-list block in tests/exporters/test_export.py that does not appear in the diff. Update the description (or the code) so they agree.

serge v0.1.0 · model: moonshotai/Kimi-K2.7-Code · 15 LLM turns · 20 tool calls · 19.3s · 298644 in / 3889 out tokens

Comment thread src/transformers/exporters/exporter_onnx.py
Comment thread src/transformers/exporters/exporter_onnx.py
return True


@register_fx_node_fix("onnx")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rewriting aten.mul.Scalar to aten.mul.Tensor when the second argument is a Node assumes that Node represents a tensor. If it represents a scalar symbolic value (e.g. a SymFloat from truediv), aten.mul.Tensor will receive a non-tensor operand and fail. Please verify the operand type or guard the rewrite more narrowly.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

addressed

@ydshieh

ydshieh commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator Author

@IlyasMoutawwakil Thank you a lot. Serge review about the PR description is because we haven't updated it. Could you update it (even with our best friend)?

And check quickly if what other things Serge said make sense maybe?

Trust you on this topic, you are the best and I am dumb on this.

If you confirm, I will merge 🙏 🚀

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>
@github-actions

Copy link
Copy Markdown
Contributor

CI recap

Dashboard: View test results in Grafana
Latest run: 33006339812:1
Result: success | Jobs: 16 | Tests: 183,714 | Failures: 0 | Duration: 11h 20m

@IlyasMoutawwakil

Copy link
Copy Markdown
Member

@ydshieh thanks for your patience 🙏 please merge

@ydshieh
ydshieh disabled auto-merge August 27, 2026 13:55
@ydshieh
ydshieh added this pull request to the merge queue Aug 27, 2026
Merged via the queue into main with commit 1987631 Aug 27, 2026
112 checks passed
@ydshieh
ydshieh deleted the skip_onnx_torch213_regression branch August 27, 2026 14:11
Qubitium added a commit to ModelCloud/OMY that referenced this pull request Aug 28, 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>

* chore(pypcre): use pcre for regex in vibevoice conversion script

---------

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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants