-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
[metrics] change default behaviour of state dict #4685
Conversation
Codecov Report
@@ Coverage Diff @@
## master #4685 +/- ##
======================================
- Coverage 93% 93% -0%
======================================
Files 117 117
Lines 8932 8932
======================================
- Hits 8299 8298 -1
- Misses 633 634 +1 |
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.
Makes sense to me, I think I overlooked this when we first implemented them
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. Just one comment.
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 !
* fix state dict * Update docs/source/metrics.rst Co-authored-by: Rohit Gupta <[email protected]> * changelog Co-authored-by: Rohit Gupta <[email protected]> Co-authored-by: chaton <[email protected]>
* fix state dict * Update docs/source/metrics.rst Co-authored-by: Rohit Gupta <[email protected]> * changelog Co-authored-by: Rohit Gupta <[email protected]> Co-authored-by: chaton <[email protected]>
What does this PR do?
Fixes
#4361
#4645
#4666
As default metrics add their internal state to the models
state_dict
. However, it seems counter intuitive to users (already 3 issues on this specific topic), and even though we since v1.0.6 provides a.persistent(mode)
to change this behavior, the feedback indicates that it is probably better as default to not add the metric state tostate_dict
(and then the method can be used to switch it on).Before submitting
PR review
Anyone in the community is free to review the PR once the tests have passed.
Before you start reviewing make sure you have read Review guidelines. In in short, see following bullet-list:
Did you have fun?
Make sure you had fun coding 🙃