Fix ImportError in transformers.exporters on torch < 2.8 - #47711
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes an ImportError when importing transformers.exporters on PyTorch versions <2.8 by removing the module-scope import of guard_or_true from the ExecuTorch exporter, ensuring the exporters package remains importable and other exporters (ONNX/Dynamo) remain reachable.
Changes:
- Removes the module-level
guard_or_trueimport inexporter_executorch.pyand defers it to the specific patch factory that needs it. - Adds a
min_versionsgate onExecutorchExporterto prevent hard crashes and surface a clear requirement message instead.
| required_packages = ["torch", "executorch"] | ||
| min_versions = {"torch": "2.8.0"} | ||
| tested_versions = {"torch": "2.12.0", "executorch": "1.3.1"} |
There was a problem hiding this comment.
Good catch, fixed. You are right that this relaxed the floor rather than
tightening it: min_versions is a plain class attribute, so ExecutorchExporter
already inherited {"torch": "2.11.0"} from DynamoExporter, and declaring
2.8.0 here overrode it. That would have let users past validate_environment
on torch 2.8 to 2.10 and then failed inside DynamoExporter.export, which is
worse than the clear error they get today.
The line is gone. The inherited torch>=2.11.0 floor stands, and it already
covers guard_or_true (torch 2.8) by a wide margin, so nothing here needs a
gate of its own. The remaining diff is just the import move.
| inputs conservatively (the pass is purely an optimisation). | ||
| """ | ||
| from executorch.exir.dialects._ops import ops as exir_ops | ||
| from torch.fx.experimental.symbolic_shapes import guard_or_true | ||
|
|
||
| def patch(self, graph_module, cat_node): | ||
| pruned = [arg for arg in cat_node.args[0] if guard_or_true(arg.meta["val"].numel() != 0)] |
There was a problem hiding this comment.
Added a test, though not quite the one suggested. Deleting guard_or_true and
reimporting the module means importlib.reload, which re-runs the ~25
register_patch / register_fx_*_fix decorators in this file against the
global registries and leaks that state into every test after it.
tests/exporters/test_utils.py::ImportSurfaceTest asserts instead that
guard_or_true is not in vars(exporter_executorch), which fails on exactly
the regression you describe with no reload and no side effects. Verified by
putting the module-scope import back: the whole test file then fails collection
on torch 2.6.
guard_or_true was added in torch 2.8, but setup.py declares torch>=2.5 and is_torch_available() does not check the version. Since auto.py imports the ExecuTorch exporter eagerly, the module-level import took down the ONNX and Dynamo exporters as well. Move the import into the only function that uses it, matching _patch_dim_order_from_stride in the same file. ExecutorchExporter already inherits min_versions torch>=2.11.0 from DynamoExporter, so no version gate is added here. Add a regression test asserting the symbol is not a module global, so the import cannot drift back to module scope.
b8f3eb1 to
8140ee9
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Suppressed comments (1)
tests/exporters/test_utils.py:139
- The PR description says it adds
min_versions = {"torch": "2.8.0"}onExecutorchExporter, but the current diff no longer adds/changesmin_versions(only theguard_or_trueimport move + regression test). To avoid confusing reviewers/users, please update the PR description to match the final implementation (or reintroduce the code change if it’s still intended).
class ImportSurfaceTest(unittest.TestCase):
"""`auto.py` imports every exporter module eagerly, so a symbol that only exists on a newer
torch than `setup.py` declares (`torch>=2.5`) takes the whole `transformers.exporters` package
down when imported at module scope — ONNX and Dynamo included. Version-gated symbols belong
inside the function that uses it."""
|
thanks, just remove the test please |
Requested by @IlyasMoutawwakil in review. The remaining diff is just the guard_or_true import move.
CI recapDashboard: View test results in Grafana |
|
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. |
What does this PR do?
Fixes #47710.
import transformers.exportersraisesImportErroron torch 2.5, 2.6 and 2.7, all inside thedeclared
torch>=2.5support range.exporter_executorch.pyimportsguard_or_trueat modulescope, and that symbol was added in torch 2.8 where it replaced
definitely_true.is_torch_available()checks that torch is installed, not which version, so the import runs andfails. Because
exporters/auto.pyimports the ExecuTorch exporter eagerly, the ONNX and Dynamoexporters become unreachable too, on torch versions neither of them needs.
First bad commit is
fde2b96b5(#41992), found withgit log -S guard_or_true.The change
One line moves. That is the whole diff.
guard_or_truemoves into_patch_remove_empty_tensors_from_cat, the only function that uses it.The sibling patch handler in the same file,
_patch_dim_order_from_stride, already importsguard_or_false, guard_or_truefunction-locally, so this follows existing convention.No
min_versionsentry is added. An earlier revision of this PR declaredmin_versions = {"torch": "2.8.0"}onExecutorchExporter; that was wrong and has been dropped.min_versionsis a plain class attribute, soExecutorchExporteralready inherits{"torch": "2.11.0"}fromDynamoExporter, and declaring2.8.0here overrode it downward. Thatwould have let users past
validate_environmenton torch 2.8 to 2.10 and into a later failureinside
DynamoExporter.export. Thanks to the Copilot review for catching it. The inheritedtorch>=2.11.0floor already coversguard_or_trueby a wide margin, so this file needs no gateof its own.
An earlier revision also added
tests/exporters/test_utils.py::ImportSurfaceTest, assertingguard_or_trueis not a module global ofexporter_executorch. Removed at @IlyasMoutawwakil'srequest.
No new helper, no new dependency, no behavior change on torch >= 2.8.
Verification on torch 2.6.0
import transformers.exportersfrom transformers.exporters import AutoHfExporterAUTO_EXPORTER_MAPPING['dynamo', 'executorch', 'onnx']python utils/checkers.py imports, repopytest tests/exporters/test_utils.pyThe fix was verified against the bug: restoring the module-scope import makes the whole test file
fail collection on torch 2.6.
The one remaining failure,
test_from_config_dispatches_dynamo, isDynamoExporter requires newer versions of: torch>=2.11.0 (found 2.6.0+cu124). That comes fromDynamoExporter.min_versionsand my local torch, not from this change. It failed before this PRtoo, silently, because nothing in the suite could be collected.
ruff checkandruff formatare clean.Before submitting
I used AI tooling to investigate this, and I have reviewed and verified every changed line and run
the tests and checks above myself.
Who can review?
@IlyasMoutawwakil, since you authored #41992. Review, and merge if it looks right to you. Happy to
switch to raising the declared torch floor instead if you prefer that route.