Skip to content
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

[Traceable FSDP2][Animesh's PR #125340] [dynamo][fsdp] Track FSDPNNModuleVariable for mutations #129045

Closed
wants to merge 6 commits into from

Conversation

yf225
Copy link
Contributor

@yf225 yf225 commented Jun 19, 2024

[ghstack-poisoned]
Copy link

pytorch-bot bot commented Jun 19, 2024

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/129045

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

✅ You can merge normally! (13 Unrelated Failures)

As of commit 1ed17c1 with merge base 3a18577 (image):

FLAKY - The following job failed but was likely due to flakiness present on trunk:

UNSTABLE - The following jobs failed but were likely due to flakiness present on trunk and has been marked as unstable:

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

yf225 added a commit that referenced this pull request Jun 19, 2024
…ariable's param/buffer/submodule is mutated

ghstack-source-id: 304e0000f1df32eea04b0de1319e9e6459b62227
Pull Request resolved: #129045
@yf225 yf225 changed the title [Traceable FSDP2][Dynamo] Track side effect if UnspecializedNNModuleVariable's param/buffer/submodule is mutated [Traceable FSDP2] [Dynamo] Track side effect if UnspecializedNNModuleVariable's param/buffer/submodule is mutated Jun 19, 2024
@yf225 yf225 requested a review from anijain2305 June 19, 2024 07:19
Copy link
Contributor

@anijain2305 anijain2305 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should do this - #125340

Feel free to copy and open a PR if urgent. I can otherwise try to land it by Friday.

@yf225 yf225 changed the title [Traceable FSDP2] [Dynamo] Track side effect if UnspecializedNNModuleVariable's param/buffer/submodule is mutated [Animesh's PR #125340] [dynamo][fsdp] Track FSDPNNModuleVariable for mutations Jun 19, 2024
[ghstack-poisoned]
@pytorch-bot pytorch-bot bot added the oncall: distributed Add this issue/PR to distributed oncall triage queue label Jun 19, 2024
@yf225 yf225 requested a review from anijain2305 June 19, 2024 23:42
@@ -484,6 +516,17 @@ def test_fsdp_aot_eager(self):
outputs = fsdp_m(inputs)
self.assertTrue(same(correct_outputs, outputs))

@skip_if_lt_x_gpu(1)
@unittest.skipIf(not has_triton(), "Inductor+gpu needs triton and recent GPU arch")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can xfail the test also, if nopython=True does not work today.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or mabe remove nopython=True and add a comment that we will work on it later.

[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
yf225 added a commit that referenced this pull request Jun 20, 2024
ghstack-source-id: 75d0ce85eef6411fa890b37a7c5768440b98aa2e
Pull Request resolved: #129045
@yf225
Copy link
Contributor Author

yf225 commented Jun 20, 2024

@pytorchbot merge -f "unrelated failures"

@pytorchmergebot
Copy link
Collaborator

Merge started

Your change will be merged immediately since you used the force (-f) flag, bypassing any CI checks (ETA: 1-5 minutes). Please use -f as last resort and instead consider -i/--ignore-current to continue the merge ignoring current failures. This will allow currently pending tests to finish and report signal before the merge.

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging
Check the merge workflow status
here

@yf225 yf225 changed the title [Animesh's PR #125340] [dynamo][fsdp] Track FSDPNNModuleVariable for mutations [Traceable FSDP2][Animesh's PR #125340] [dynamo][fsdp] Track FSDPNNModuleVariable for mutations Jun 27, 2024
@github-actions github-actions bot deleted the gh/yf225/48/head branch July 28, 2024 02:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ciflow/inductor Merged module: dynamo oncall: distributed Add this issue/PR to distributed oncall triage queue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants