-
Notifications
You must be signed in to change notification settings - Fork 16
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 a function for analysis after LS #51
Conversation
I would let the function also take the iter number |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #51 +/- ##
==========================================
+ Coverage 88.52% 88.53% +0.01%
==========================================
Files 46 46
Lines 1839 1841 +2
==========================================
+ Hits 1628 1630 +2
Misses 211 211
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
src/polysolve/nonlinear/Solver.cpp
Outdated
@@ -195,6 +195,7 @@ namespace polysolve::nonlinear | |||
this->m_stop.fDelta, this->m_stop.gradNorm, this->m_stop.xDelta); | |||
|
|||
update_solver_info(objFunc.value(x)); | |||
objFunc.step_accepted(this->m_current.iterations, x); |
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.
Do we need it here? If so it has a misleading name
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.
Yes we need it here, do you have a better name in mind?
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.
The name was chosen to match with solution_changed
in style, but happy to change to if you have a suggestion @teseoch
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.
then, can it be implemented inside solution changed?
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.
or post step?
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.
This cannot be put inside solution changed since it gets called a lot in the line search and we want something once per iteration. Post step could work, but we need something that also gets called after the first solution changed. So we'd either need some function like this (we can change the name) or add a post_step function call between lines 182 and 198.
No description provided.