-
Notifications
You must be signed in to change notification settings - Fork 34
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
[WIP] Fix HagerZhang bugs #136
Conversation
a4ab100
to
a26ec4e
Compare
I think this is good to go with the exception of some tests failing. The tests are maybe depending on the old behavior? Does that make them good tests? Any help there is appreciated. Optim tests pass, so that's a sign that I didn't totally screw up. |
I think that test is probably just check that behavior hasn't changed, but it seems quite likely that your changes come with changes to that exact step length. I Don't have time right now, but I'll review asap. |
Most are just to flag if we unintentionally change behaviour and are not good tests. They were added long after the implementation of the algorithms so little thought has been put into them. Feel free to update them if you believe the new behaviour is the correct one ;) |
All good now. |
Codecov Report
@@ Coverage Diff @@
## master #136 +/- ##
===========================================
- Coverage 69.92% 42.74% -27.18%
===========================================
Files 8 8
Lines 389 648 +259
===========================================
+ Hits 272 277 +5
- Misses 117 371 +254
Continue to review full report at Codecov.
|
Thanks! I believe code coverage is really bugged in julia these days.. |
I've tagged a bugfix release https://github.com/notifications |
Awesome thanks for the quick merge! |
This PR closes #133, #134, and #135. There is one error I am getting in LineSearches tests which seems unrelated to the changes made. Optim tests pass. I will look into this further. Also, I need to check if other places error when the search direction is not a descent direction and try to make it robust to computational error when starting at the optimal solution. Finally, someone please comment on my use of
mayterminate
since I don't understand how it will be used apart from checking when the quadratic step is taken.