-
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
Summarize model size in MegaBytes [WIP] #4810
Summarize model size in MegaBytes [WIP] #4810
Conversation
WIP please suggest how to proceed from here. |
@awaelchli @rohitgr7 I'm sensing a usage of LRU cache in model summary, as calculating out and input sizes can be redundant, |
Codecov Report
@@ Coverage Diff @@
## release/1.2-dev #4810 +/- ##
================================================
+ Coverage 93% 93% +1%
================================================
Files 153 135 -18
Lines 10815 10005 -810
================================================
- Hits 10006 9339 -667
+ Misses 809 666 -143 |
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.
Would you mind adding some tests ?
@tchaton |
Hello @kartik4949! Thanks for updating this PR.
Comment last updated at 2020-12-07 07:02:33 UTC |
@justusschock can u suggest tests? |
You could start with simple models like one or two layers, where you know the size / can calculate them by hand and then compare it to the automatically inferred size. Also you could do it once for a more complex model. |
Thanks doing the same:) |
@awaelchli @ananyahjha93 @williamFalcon @Borda I think this PR is ready , need to add some complex models for testing |
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.
nice work, just need to finish some minor nits...
@Borda dropped the idea of input size, we are good with example_input_array only which will act as custom input_size. |
60ee020
to
8b1c9d0
Compare
@Borda
All Tests are passed :) |
Co-authored-by: Jirka Borovec <[email protected]>
Co-authored-by: Rohit Gupta <[email protected]>
@awaelchli @justusschock Hey lets calculate model total trainable params size only |
@kartik4949 Yes, I think that's best. And for the more complicated part, maybe it's better to do this in a separate follow up PR :) |
@justusschock thanks for responding will remove other sizes and conclude this one :) |
14480c7
to
63010a8
Compare
@justusschock hey, done with changes. |
eb6282b
to
63010a8
Compare
Fixes # (issue)
#4807
This PR adds a Summarization of the model with estimated total model size.
Estimated Total model size is "input_size + forward/backward pass size + total params size".
e.g
VGG16 with input (1, 1, 28, 28)
Input Size (MBs) --> 0.57
Total Params Size (MBs) --> 528 (e.g downloading weights size)
Forward/Backward Size (MBs) --> 218
Total Estimated Model Size (MBs) --> 747
Testing
tests knownet model with different input shapes and batch sizes.
we test the corresponding model sizes with pre calculated model size.
tests a DummyNetwork with different input types.
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 🙃