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

[FSDP] Validate ignored_modules, ignored_states #104273

Closed
wants to merge 2 commits into from

Conversation

awgu
Copy link
Contributor

@awgu awgu commented Jun 27, 2023

Stack from ghstack (oldest at bottom):

This checks that ignored_modules and ignored_states have the expected type and provides a reasonable error message if not. Otherwise, if someone passes a mix of modules and parameters to ignored_states for example, then our code may be silently incorrect.

@pytorch-bot
Copy link

pytorch-bot bot commented Jun 27, 2023

🔗 Helpful Links

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

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

✅ No Failures

As of commit 551110e:
💚 Looks good so far! There are no failures yet. 💚

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

This checks that `ignored_modules` and `ignored_states` have the expected type and provides a reasonable error message if not. Otherwise, if someone passes a mix of modules and parameters to `ignored_states` for example, then our code may be silently incorrect.

[ghstack-poisoned]
Copy link
Member

@rohan-varma rohan-varma left a comment

Choose a reason for hiding this comment

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

LGTM. do we want to enforce this for composable API as well?

@awgu
Copy link
Contributor Author

awgu commented Jun 28, 2023

LGTM. do we want to enforce this for composable API as well?

Composable API uses the same code path, so I think the concern is more about unit testing.

@awgu awgu marked this pull request as ready for review June 28, 2023 00:41
@awgu awgu added the ciflow/trunk Trigger trunk jobs on your pull request label Jun 28, 2023
@facebook-github-bot facebook-github-bot deleted the gh/awgu/406/head branch July 1, 2023 14:16
vkallesh pushed a commit to amd/ZenDNN-pytorch that referenced this pull request Jul 5, 2023
ghstack-source-id: 356d1a8c5a674d7259fbb028989385ed78681f3b
Pull Request resolved: pytorch/pytorch#104273
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ciflow/trunk Trigger trunk jobs on your pull request Merged release notes: distributed (fsdp) release notes category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants