refactor line-search solvers to return latest iterate#227
Open
jpbrodrick89 wants to merge 2 commits intopatrick-kidger:devfrom
Open
refactor line-search solvers to return latest iterate#227jpbrodrick89 wants to merge 2 commits intopatrick-kidger:devfrom
jpbrodrick89 wants to merge 2 commits intopatrick-kidger:devfrom
Conversation
Collaborator
|
Edit: just changed the base to a new |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
As suggested in your comment in #225, I have refactored line-search solvers, specifically
AbstractGradientDescent,AbstractQuasiNewtonandAbstractGaussNewton(and their subclasses implicitly), such thatstepalways receives and returns the latest iterate rather than the last accepted step. The last accepted step is now stored instate.y_acceptedinstead ofstate.y_eval. This means we don't have to usetree_whereto constructprev_auxanymore and all minimisers and least square solvers compose withBestSoFarMinimiserin the same way—including the very latest iterate in the sequence rather than the lagged accepted. Note that this means that theyreturned will be worse thany_acceptedon rejected steps, but asBestSoFarMinimisersees all iterates this is not an issue.Once this is merged I can have a think about whether a wider unification of line-search solvers is possible.