-
Notifications
You must be signed in to change notification settings - Fork 26
Incremental Mean Volume Checkpoint #1071
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
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #1071 +/- ##
===========================================
+ Coverage 88.76% 88.80% +0.03%
===========================================
Files 126 126
Lines 11969 12001 +32
===========================================
+ Hits 10624 10657 +33
+ Misses 1345 1344 -1 ☔ View full report in Codecov by Sentry. |
|
@j-c-c , since I added a bunch of doc string stuff probably best to start working through the review. Thanks |
Sounds good. I'll start taking a look today. |
j-c-c
left a 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.
Looks good. Just a couple things.
j-c-c
left a 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.
Looks good!
janden
left a 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.
Looks great! Just a few things.
Close #1070 by adding incremental checkpoint and max iteration limits.
Altered proposed solution in issue slightly. Once I got in the code I noticed it is more effective to return and restore coef directly than convert to/from Volume.
Still considering options for testing this.