-
Notifications
You must be signed in to change notification settings - Fork 24.7k
Consolidate the naming of named_parameter and state_dict for CheckpointWrapper #80089
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
Conversation
…ntWrapper named_parameter() should return the same parameter names as state_dict() but the current CheckpointWrapper does not enforce this naming rule. This PR resolves this issue. Differential Revision: [D37344200](https://our.internmc.facebook.com/intern/diff/D37344200/) [ghstack-poisoned]
🔗 Helpful links
✅ No Failures (0 Pending)As of commit 56150c9 (more details on the Dr. CI page): Expand to see more💚 💚 Looks good so far! There are no failures yet. 💚 💚 This comment was automatically generated by Dr. CI (expand for details).Please report bugs/suggestions to the (internal) Dr. CI Users group. |
…ntWrapper named_parameter() should return the same parameter names as state_dict() but the current CheckpointWrapper does not enforce this naming rule. This PR resolves this issue. Differential Revision: [D37344200](https://our.internmc.facebook.com/intern/diff/D37344200/) ghstack-source-id: 159696213 Pull Request resolved: #80089
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm, thanks
Overrides :meth:`named_parameters()` to intercept parameter names and | ||
remove all occurrences of _CHECKPOINT_PREFIX. | ||
""" | ||
# Determine which logic to use based on the context at call time |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: there is no context here?
…or CheckpointWrapper" named_parameter() should return the same parameter names as state_dict() but the current CheckpointWrapper does not enforce this naming rule. This PR resolves this issue. Differential Revision: [D37344200](https://our.internmc.facebook.com/intern/diff/D37344200/) [ghstack-poisoned]
…ntWrapper Pull Request resolved: #80089 named_parameter() should return the same parameter names as state_dict() but the current CheckpointWrapper does not enforce this naming rule. This PR resolves this issue. ghstack-source-id: 159744641 Differential Revision: [D37344200](https://our.internmc.facebook.com/intern/diff/D37344200/)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
…or CheckpointWrapper" named_parameter() should return the same parameter names as state_dict() but the current CheckpointWrapper does not enforce this naming rule. This PR resolves this issue. Differential Revision: [D37344200](https://our.internmc.facebook.com/intern/diff/D37344200/) [ghstack-poisoned]
…ntWrapper Pull Request resolved: #80089 named_parameter() should return the same parameter names as state_dict() but the current CheckpointWrapper does not enforce this naming rule. This PR resolves this issue. ghstack-source-id: 160471938 Differential Revision: [D37344200](https://our.internmc.facebook.com/intern/diff/D37344200/)
…or CheckpointWrapper" named_parameter() should return the same parameter names as state_dict() but the current CheckpointWrapper does not enforce this naming rule. This PR resolves this issue. Differential Revision: [D37344200](https://our.internmc.facebook.com/intern/diff/D37344200/) [ghstack-poisoned]
…ntWrapper Pull Request resolved: #80089 named_parameter() should return the same parameter names as state_dict() but the current CheckpointWrapper does not enforce this naming rule. This PR resolves this issue. ghstack-source-id: 160637693 Differential Revision: [D37344200](https://our.internmc.facebook.com/intern/diff/D37344200/)
@pytorchbot merge |
@pytorchbot successfully started a merge job. Check the current status here |
Hey @fegin. |
…ntWrapper (#80089) (#80089) Summary: named_parameter() should return the same parameter names as state_dict() but the current CheckpointWrapper does not enforce this naming rule. This PR resolves this issue. Pull Request resolved: #80089 Approved by: https://github.com/rohan-varma Test Plan: contbuild & OSS CI, see https://hud.pytorch.org/commit/pytorch/pytorch/e0eeb06ec6b047eaa9ec7d9d08debd636dffb9a2 Reviewed By: mehtanirav Differential Revision: D37344200 Pulled By: fegin fbshipit-source-id: e4ae3136d68114bff548f3f1774356a30515047d
Stack from ghstack (oldest at bottom):
named_parameter() should return the same parameter names as state_dict() but the current CheckpointWrapper does not enforce this naming rule. This PR resolves this issue.
Differential Revision: D37344200