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
summarize total size of model params in bytes #5590
summarize total size of model params in bytes #5590
Conversation
Hello @kartik4949! Thanks for updating this PR.
Comment last updated at 2021-01-25 01:10:41 UTC |
@kartik4949 nice enhancement, mind rebase it on EDIT: resolved, no action needed any more... |
Co-authored-by: Rohit Gupta <rohitgr1998@gmail.com>
@rohitgr7 @Borda @justusschock completed the PR, I think it's ready for merge. suggestions are welcomed :) |
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.
good work
just needs a bit more polish I think :))
Co-authored-by: Adrian Wälchli <aedu.waelchli@gmail.com>
@awaelchli @rohitgr7 done with all suggested changes, |
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.
this looks super clean now. great stuff. please ping me with your follow up that you planned, I'd like to have a close look :)
just one final request, could you add this to the changelog file? thx |
Codecov Report
@@ Coverage Diff @@
## release/1.2-dev #5590 +/- ##
===============================================
Coverage 92% 92%
===============================================
Files 153 153
Lines 10845 10860 +15
===============================================
+ Hits 10022 10037 +15
Misses 823 823 |
@rohitgr7 |
@kartik4949 yes. |
done! |
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, nice feature!
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.
Overall, looks good !
added test to check model_size with different precision. as suggested by @rohitgr7. |
What does this PR do?
A simplified version of model size i.e only consider total params for calculating model size.
followup pr of #4810
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 short, see the following bullet-list:
Did you have fun?
Make sure you had fun coding 🙃