-
Notifications
You must be signed in to change notification settings - Fork 41
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
:reinitialize_direction_update
for quasi-Newton
#388
Conversation
Looks good at first glance (will take a closer look later); documenter fails, because of caching; it might use the cache from the other PR (of cached packages). Maybe that is a disadvantage of caching packages. |
Are ALM and EPM supposed to keep the state of |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #388 +/- ##
=======================================
Coverage 99.76% 99.76%
=======================================
Files 71 71
Lines 7205 7221 +16
=======================================
+ Hits 7188 7204 +16
Misses 17 17 ☔ View full report in Codecov by Sentry. |
I think they should not, you are right. Maybe we should have noticed the necessary initialisation there already. |
Caching seems to have fixed itself. By the way, I think |
I think that sounds nice, how breaking would that be? The old one just took a gradient direction right? I would consider that mildly breaking and still ok |
Not that breaking, I will change it if you are OK with it. |
We also introduced the rule for how to handle non-descent directions as a non-braking improvement, so I am fine with this as well, but please write a detailed change entry about it. |
Is it sufficiently detailed in the latest commit? |
I prefer to end keywords with |
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 a few minor comments.
Addresses point 3 from #382. Also a minor cleanup for
QuasiNewtonCautiousDirectionUpdate
.