NotificationsYou must be signed in to change notification settings
Fork26.3k
Star96k

[BC-breaking] Set NonStrict as default for export_for_training#150941

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Closed

gmagogsfm wants to merge1 commit intopytorch:mainfromgmagogsfm:export-D72724975

Closed

[BC-breaking] Set NonStrict as default for export_for_training#150941

gmagogsfm wants to merge1 commit intopytorch:mainfromgmagogsfm:export-D72724975

Conversation

Copy link

Contributor

gmagogsfm commentedApr 9, 2025•
edited by pytorch-botbot
Loading

Summary:

Flip default value ofstrict argument from True to False on torch.export.export_for_training API
All callsites have been updated to provide this argument explicitly to avoid behavior change.
If you see any breakages, that means you may have a new callsite that is missed, please setstrict=True explicitly to the callsite to mitigage.

Test Plan: CI

Differential Revision: D72724975

cc@ezyang @gchanan

gmagogsfm requested review fromangelayi,avikchaudhuri,tugsbayasgalan,ydwu4 andzhxchen17 ascode owners

April 9, 2025 17:28

pytorch-botbot added the release notes: export label

Apr 9, 2025

Copy link

pytorch-botbot commentedApr 9, 2025•
edited
Loading

🔗 Helpful Links

🧪 See artifacts and rendered test results athud.pytorch.org/pr/150941

📄 PreviewPython docs built from this PR
📄 PreviewC++ docs built from this PR
❓ Need help or want to give feedback on the CI? Visit thebot commands wiki or ouroffice hours

Note: Links to docs will display an error until the docs builds have been completed.

✅ No Failures

As of commit7183fb5 with merge base061832b ():
💚 Looks good so far! There are no failures yet. 💚

This comment was automatically generated by Dr. CI and updates every 15 minutes.

Copy link

Contributor

facebook-github-bot commentedApr 9, 2025

This pull request wasexported from Phabricator. Differential Revision:D72724975

facebook-github-bot added the fb-exported label

Apr 9, 2025

gmagogsfm added the module: bc-breakingRelated to a BC-breaking change label

Apr 9, 2025

pytorch-botbot added the topic: bc breakingtopic category label

Apr 9, 2025

gmagogsfm force-pushed theexport-D72724975 branch fromb995e48 tod1c0f02Compare

April 10, 2025 01:25

Copy link

Contributor

facebook-github-bot commentedApr 10, 2025

This pull request wasexported from Phabricator. Differential Revision:D72724975

[BC-breaking] Set NonStrict as default for export_for_training (pytor…

7183fb5

…ch#150941)Summary:- Flip default value of `strict` argument from True to False on torch.export.export_for_training API- All callsites have been updated to provide this argument explicitly to avoid behavior change.- If you see any breakages, that means you may have a new callsite that is missed, please set `strict=True` explicitly to the callsite to mitigage.Test Plan: CIDifferential Revision: D72724975

gmagogsfm force-pushed theexport-D72724975 branch fromd1c0f02 to7183fb5Compare

April 10, 2025 01:26

Copy link

Contributor

facebook-github-bot commentedApr 10, 2025

This pull request wasexported from Phabricator. Differential Revision:D72724975

ydwu4 approved these changes

Apr 10, 2025

View reviewed changes

pytorch-botbot added the ciflow/trunkTrigger trunk jobs on your pull request label

Apr 10, 2025

Copy link

Contributor

facebook-github-bot commentedApr 10, 2025

@pytorchbot merge

(Initiating merge automatically since Phabricator Diff has merged)

pytorchmergebot added the merging label

Apr 10, 2025

Copy link

Collaborator

pytorchmergebot commentedApr 10, 2025

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

Learn more about merging in thewiki.

Questions? Feedback? Please reach out to thePyTorch DevX Team

Advanced Debugging

Check the merge workflow status
here

pytorchmergebot added the Merged label

Apr 11, 2025

pytorchmergebot closed this inc93e4b8

Apr 11, 2025

pytorchmergebot removed the merging label

Apr 11, 2025

timocafe pushed a commit to timocafe/pytorch that referenced this pull request

Apr 16, 2025

[BC-breaking] Set NonStrict as default for export_for_training (pytor…

1a71d13

…ch#150941)Summary:- Flip default value of `strict` argument from True to False on torch.export.export_for_training API- All callsites have been updated to provide this argument explicitly to avoid behavior change.- If you see any breakages, that means you may have a new callsite that is missed, please set `strict=True` explicitly to the callsite to mitigage.Test Plan: CIDifferential Revision: D72724975Pull Requestresolved:pytorch#150941Approved by:https://github.com/ydwu4

amathewc pushed a commit to amathewc/pytorch that referenced this pull request

Apr 17, 2025

[BC-breaking] Set NonStrict as default for export_for_training (pytor…

9265e49

…ch#150941)Summary:- Flip default value of `strict` argument from True to False on torch.export.export_for_training API- All callsites have been updated to provide this argument explicitly to avoid behavior change.- If you see any breakages, that means you may have a new callsite that is missed, please set `strict=True` explicitly to the callsite to mitigage.Test Plan: CIDifferential Revision: D72724975Pull Requestresolved:pytorch#150941Approved by:https://github.com/ydwu4

CuiYifeng mentioned this pull request

Jun 16, 2025

Set strict=True explicitly for torch.export.export_for_trainingchuanqi129/inductor-tools#157

Merged

Copy link

Collaborator

EikanWang commentedJun 17, 2025•
edited
Loading

@gmagogsfm ,@ydwu4 since it is a break change and the comment ofstrict highlights that theFalse may result in a soundness issue, may I know the motivation to flip the default value ofstrict argument fromTrue toFalse ontorch.export.export_for_training API?