[ONNX] Reland: Update training state logic to support ScriptedModule #86745
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/86745
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit 9b44054: This comment was automatically generated by Dr. CI and updates every 15 minutes. |
pytorch-bot
bot
added
the
release notes: onnx
samdow
added
the
triaged
There was a problem hiding this comment.
LGTM, @justinchuby please add test case as TiTai suggested.
For my understanding by looking at related code, is it that training is not possible for ScriptModules?
justinchuby
force-pushed
the
justinchu/scripted-module
branch
from
ff23aee
to
cc1f552
Compare
|
Thanks for reviewing! Tests added. |
|
@pytorchbot merge -g |
Merge startedYour change will be merged once all checks on your PR pass since you used the green (-g) flag (ETA: 0-4 Hours). Learn more about merging in the wiki. Questions? Feedback? Please reach out to the PyTorch DevX Team |
Merge failedReason: The following mandatory check(s) failed (Rule Dig deeper by viewing the failures on hud Details for Dev Infra teamRaised by workflow job |
|
@pytorchbot merge -g |
Merge startedYour change will be merged once all checks on your PR pass since you used the green (-g) flag (ETA: 0-4 Hours). Learn more about merging in the wiki. Questions? Feedback? Please reach out to the PyTorch DevX Team |
Merge failedReason: The following mandatory check(s) failed (Rule Dig deeper by viewing the failures on hud Details for Dev Infra teamRaised by workflow job |
|
@pytorchbot merge -f "Unrelated functorch errors; onnx tests passed" |
Merge startedYour change will be merged immediately since you used the force (-f) flag, bypassing any CI checks (ETA: 1-5 minutes). Learn more about merging in the wiki. Questions? Feedback? Please reach out to the PyTorch DevX Team |
That is my current understanding |
|
@pytorchbot revert -m " https://hud.pytorch.org/pytorch/pytorch/commit/960b98128e475b15b66119f325232039799852cd broke onnx tests on trunk " -c weird I could not interpret the failing logs on my phone, but this PR definitely introduced these regressions. |
|
@pytorchbot successfully started a revert job. Check the current status here. |
|
@justinchuby your PR has been successfully reverted. |
…86745)" This reverts commit 960b981. Reverted #86745 on behalf of https://github.com/janeyx99 due to https://hud.pytorch.org/pytorch/pytorch/commit/960b98128e475b15b66119f325232039799852cd broke onnx tests on trunk
|
@janeyx99 thanks for catching this. The failures are legit. Somehow they were green when I merged |
In pytorch#86325, it was reported that ScriptedModule do not have a training attribute and will fail export because we don't expect them as input. We should also relax the type constraints in a following PR. Fixes pytorch#86325
|
Turns out it's because the branch was not up to date with master. Rebased. |
justinchuby
force-pushed
the
justinchu/scripted-module
branch
from
4cd5e1a
to
399fe2e
Compare
|
@pytorchbot merge -g |
Merge startedYour change will be merged once all checks on your PR pass since you used the green (-g) flag (ETA: 0-4 Hours). Learn more about merging in the wiki. Questions? Feedback? Please reach out to the PyTorch DevX Team |
justinchuby
changed the title
…ytorch#86745) In pytorch#86325, it was reported that ScriptedModule do not have a training attribute and will fail export because we don't expect them as input. Also - Parameterized the test_util_funs test Thanks @borisfom for the suggestion! Fixes pytorch#86325 Pull Request resolved: pytorch#86745 Approved by: https://github.com/AllenTiTaiWang, https://github.com/BowenBao
…86745) (#87457) In #86325, it was reported that ScriptedModule do not have a training attribute and will fail export because we don't expect them as input. Also - Parameterized the test_util_funs test Thanks @borisfom for the suggestion! Fixes #86325 Pull Request resolved: #86745 Approved by: https://github.com/AllenTiTaiWang, https://github.com/BowenBao Co-authored-by: Justin Chu <justinchu@microsoft.com>
In #86325, it was reported that ScriptedModule do not have a training attribute and will fail export because we don't expect them as input.
Also
Thanks @borisfom for the suggestion!
Fixes #86325