-
-
Notifications
You must be signed in to change notification settings - Fork 24
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
Add sub sampler resnet18 #69
Add sub sampler resnet18 #69
Conversation
Codecov Report
@@ Coverage Diff @@
## master #69 +/- ##
==========================================
+ Coverage 85.21% 85.74% +0.53%
==========================================
Files 20 21 +1
Lines 771 821 +50
==========================================
+ Hits 657 704 +47
- Misses 114 117 +3
Continue to review full report at Codecov.
|
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.
Great PR! I made a few suggestions to get this to be easily reusable on other resnet-based achitecture, let me know what you think!
Hi @frgfm , Thanks for your remarks. It is indeed a good idea to generalize to any resnet. Let me know what you think of this new version |
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.
Almost there! you can define each ssresnet size like in torchvision and move weight loading outside of the class definition, check my comments, and we'll be good to go!
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.
Thanks for the PR! This already looks good, I made a few suggestions to improve flexibility, let me know your thoughts on it!
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.
A few minor adjustments for the final touch, and we're good to go!
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.
Thanks a lot, looking forward to try it out!
The goal of this PR is to add the SubSamplerResnet18 model which is a derivative of the resnet18 that we want to train on the SubSamplerDataSet of the PR #66