-
Notifications
You must be signed in to change notification settings - Fork 25.7k
[ONNX] Set fallback=False by default #162726
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/162726
Note: Links to docs will display an error until the docs builds have been completed. ⏳ No Failures, 2 PendingAs of commit 8b9acb6 with merge base 07d2531 ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
|
Co-authored-by: justinchuby <11205048+justinchuby@users.noreply.github.com>
6c435c9
to
8b9acb6
Compare
@pytorchbot merge |
Merge startedYour change will be merged once all checks pass (ETA 0-4 Hours). Learn more about merging in the wiki. Questions? Feedback? Please reach out to the PyTorch DevX Team |
@pytorchbot merge -f "all related tests passed" |
The merge job was canceled or timed out. This most often happen if two merge requests were issued for the same PR, or if merge job was waiting for more than 6 hours for tests to finish. In later case, please do not hesitate to reissue the merge command |
Merge startedYour change will be merged immediately since you used the force (-f) flag, bypassing any CI checks (ETA: 1-5 minutes). Please use Learn more about merging in the wiki. Questions? Feedback? Please reach out to the PyTorch DevX Team |
@pytorchbot cherry-pick --onto release/2.9 --fixes "Critical fixes to new features: ONNX exporter's switch to pt2 export as default - corrected behavior" -c critical |
Cherry picking #162726Command
Details for Dev Infra teamRaised by workflow job |
@pytorchbot cherry-pick --onto release/2.9 --fixes "Critical fixes to new features: ONNX exporter's switch to pt2 export as default - corrected behavior" -c critical |
Cherry picking #162726Command
Details for Dev Infra teamRaised by workflow job |
This change addresses confusing error messages users encounter when using the ONNX exporter with default settings. Previously, `fallback=True` was the default, which would attempt to fall back to the TorchScript exporter when the dynamo path failed, leading to mixed error messages that obscured the actual issues. ## Problem When `fallback=True` by default: - Users get confusing error messages mixing dynamo and TorchScript export failures - Error messages tell users to provide the `f` argument unnecessarily - Dynamo error messages get flushed with TorchScript errors when both paths fail - Users expecting the dynamo path get unexpected fallback behavior ## Solution Changed the default from `fallback=True` to `fallback=False` in both: - `torch.onnx.export()` function - `torch.onnx._internal.exporter._compat.export_compat()` function ## Impact **Before:** ```python # Would fallback to TorchScript on dynamo failure, causing mixed error messages torch.onnx.export(model, args) ``` **After:** ```python # Clean dynamo-only errors by default torch.onnx.export(model, args) # Advanced users can still opt-in to fallback behavior torch.onnx.export(model, args, fallback=True) ``` Fixes pytorch#162697 Pull Request resolved: pytorch#162726 Approved by: https://github.com/titaiwangms, https://github.com/xadupre
This change addresses confusing error messages users encounter when using the ONNX exporter with default settings. Previously, `fallback=True` was the default, which would attempt to fall back to the TorchScript exporter when the dynamo path failed, leading to mixed error messages that obscured the actual issues. ## Problem When `fallback=True` by default: - Users get confusing error messages mixing dynamo and TorchScript export failures - Error messages tell users to provide the `f` argument unnecessarily - Dynamo error messages get flushed with TorchScript errors when both paths fail - Users expecting the dynamo path get unexpected fallback behavior ## Solution Changed the default from `fallback=True` to `fallback=False` in both: - `torch.onnx.export()` function - `torch.onnx._internal.exporter._compat.export_compat()` function ## Impact **Before:** ```python # Would fallback to TorchScript on dynamo failure, causing mixed error messages torch.onnx.export(model, args) ``` **After:** ```python # Clean dynamo-only errors by default torch.onnx.export(model, args) # Advanced users can still opt-in to fallback behavior torch.onnx.export(model, args, fallback=True) ``` Fixes pytorch#162697 Pull Request resolved: pytorch#162726 Approved by: https://github.com/titaiwangms, https://github.com/xadupre
This change addresses confusing error messages users encounter when using the ONNX exporter with default settings. Previously, `fallback=True` was the default, which would attempt to fall back to the TorchScript exporter when the dynamo path failed, leading to mixed error messages that obscured the actual issues. ## Problem When `fallback=True` by default: - Users get confusing error messages mixing dynamo and TorchScript export failures - Error messages tell users to provide the `f` argument unnecessarily - Dynamo error messages get flushed with TorchScript errors when both paths fail - Users expecting the dynamo path get unexpected fallback behavior ## Solution Changed the default from `fallback=True` to `fallback=False` in both: - `torch.onnx.export()` function - `torch.onnx._internal.exporter._compat.export_compat()` function ## Impact **Before:** ```python # Would fallback to TorchScript on dynamo failure, causing mixed error messages torch.onnx.export(model, args) ``` **After:** ```python # Clean dynamo-only errors by default torch.onnx.export(model, args) # Advanced users can still opt-in to fallback behavior torch.onnx.export(model, args, fallback=True) ``` Fixes pytorch#162697 Pull Request resolved: pytorch#162726 Approved by: https://github.com/titaiwangms, https://github.com/xadupre
This change addresses confusing error messages users encounter when using the ONNX exporter with default settings. Previously, `fallback=True` was the default, which would attempt to fall back to the TorchScript exporter when the dynamo path failed, leading to mixed error messages that obscured the actual issues. ## Problem When `fallback=True` by default: - Users get confusing error messages mixing dynamo and TorchScript export failures - Error messages tell users to provide the `f` argument unnecessarily - Dynamo error messages get flushed with TorchScript errors when both paths fail - Users expecting the dynamo path get unexpected fallback behavior ## Solution Changed the default from `fallback=True` to `fallback=False` in both: - `torch.onnx.export()` function - `torch.onnx._internal.exporter._compat.export_compat()` function ## Impact **Before:** ```python # Would fallback to TorchScript on dynamo failure, causing mixed error messages torch.onnx.export(model, args) ``` **After:** ```python # Clean dynamo-only errors by default torch.onnx.export(model, args) # Advanced users can still opt-in to fallback behavior torch.onnx.export(model, args, fallback=True) ``` Fixes pytorch#162697 Pull Request resolved: pytorch#162726 Approved by: https://github.com/titaiwangms, https://github.com/xadupre
@pytorchbot cherry-pick --onto release/2.9 --fixes "Critical fixes to new features: ONNX exporter's switch to pt2 export as default - corrected behavior" -c critical |
@Camyll i think this was merged as part of a cherry-pick? |
The cherry pick attempt failed due to a merge conflict, I'm trying again with the bot to see if it got resolved, otherwise I'll manually cherrypick |
I meant it should be part of #162637 |
Cherry picking #162726Command
Details for Dev Infra teamRaised by workflow job |
I see thanks for clarifying, I misread the bot message! We have a workflow that still doesn't recognize it as being cherrypicked which is why it got flagged to me. I'll close the issue thanks! |
This change addresses confusing error messages users encounter when using the ONNX exporter with default settings. Previously,
fallback=True
was the default, which would attempt to fall back to the TorchScript exporter when the dynamo path failed, leading to mixed error messages that obscured the actual issues.Problem
When
fallback=True
by default:f
argument unnecessarilySolution
Changed the default from
fallback=True
tofallback=False
in both:torch.onnx.export()
functiontorch.onnx._internal.exporter._compat.export_compat()
functionImpact
Before:
After:
Fixes #162697
cc @titaiwangms