-
Notifications
You must be signed in to change notification settings - Fork 411
Add onnxscript 0.4.0 as a dependency #127
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
base: main
Are you sure you want to change the base?
Conversation
According to the pytorch docs (https://docs.pytorch.org/docs/stable/onnx_export.html), onnx export depends on onnxscript being installed. In PyTorch 2.9, the export behavior now defaults to torch.export instead of torch.onnx, which is a codepath that imports onnxscript eagerly. An alternative is to set dynamo=False on onnx export, but this is not recommended by PyTorch release notes. https://github.com/pytorch/pytorch/releases/tag/v2.9.0
|
(note that onnxscript 0.4.0 is the version used in pytorch, see https://github.com/pytorch/pytorch/blob/8aa465f18e962fbb35eead029122cc564ed9d400/.ci/docker/common/install_onnx.sh#L23) |
|
Hey @Kukanani, the file you linked shows torchscript 0.5.4. Do you think it makes sense to set 0.4.0 as a minimum requirement instead of a hard constraint? |
|
Also, do we need a different license for this? |
|
Between making the change and permalinking the code, it was updated via pytorch/pytorch#165883 to 0.5.4! So yes, that should be the correct version now. |
|
onnxscript is MIT licensed, which is BSD 3-clause compatible, so I think no licensing changes are needed. |
|
Hi @Kukanani, I added the license and opened a PR in your fork. |
>=0.5.4 to pick up future changes, but <1.0 to prevent future breaking changes
|
I merged your MR that targeted mine |
This is a follow-up fix based on an mjlab issue: mujocolab/mjlab#230
According to the pytorch docs (https://docs.pytorch.org/docs/stable/onnx_export.html), onnx export depends on
onnxscriptbeing installed. In PyTorch 2.9, the export behavior now defaults totorch.exportinstead oftorch.onnx, which is a codepath that importsonnxscripteagerly, now requiring an explicit dependency. An alternative is to setdynamo=Falseon onnx export, but this is not recommended by PyTorch release notes (https://github.com/pytorch/pytorch/releases/tag/v2.9.0)