Add keyword arguments to fixedpoint for hasconverged and shouldstop stopping criteria#397
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests.
🚀 New features to boost your workflow:
|
| # default stopping criteria initialized on alg parameters that have to be select first | ||
| isnothing(hasconverged) && (hasconverged = OptimKit.DefaultHasConverged(alg.optimizer_alg.gradtol)) | ||
| isnothing(shouldstop) && (shouldstop = OptimKit.DefaultShouldStop(alg.optimizer_alg.maxiter)) |
There was a problem hiding this comment.
This is a bit annoying. Can we get around this by not setting any default kwarg values at all here, deleting (; finalize!, hasconverged, shouldstop) from the kwargs passed to select_algorithm, and then do the opposite (only retain (; finalize!, hasconverged, shouldstop)) and when returning fixedpoint(operator, peps₀, env₀, alg; kwargs...)? Or would this be just as annoying?
There was a problem hiding this comment.
I gave this a go and I think I prefer your suggestion. It's still a bit annoying having to filter the kwargs but I think it's less annoying that way and saves lines.
| extra_kwarg_keys = (:finalize!, :hasconverged, :shouldstop) # these will not be passed to `select_algorithm`, only to the 2nd `fixedpoint` call | ||
| alg = select_algorithm(fixedpoint, env₀; filter(kw -> !(first(kw) in extra_kwarg_keys), kwargs)...) | ||
| return fixedpoint(operator, peps₀, env₀, alg; filter(kw -> first(kw) in extra_kwarg_keys, kwargs)...) |
There was a problem hiding this comment.
Not beautiful, but I think it's slightly less hacky than setting the nothing defaults first and then replacing them.
Maybe @lkdvos can give an independent opinion?
There was a problem hiding this comment.
I don't think I can come up with much better, the only alternative I could think off is to explicitly put the optimization keywords into the alg, so we don't have to peel them but I think this is probably fine?
There was a problem hiding this comment.
I considered that but was ultimately against it because I felt that having Functions as fields could become annoying.
3 participants
This PR adds the possibility for customized converging and stopping criteria through OptimKit's
hasconvergedandshouldstopkeyword arguments. This can be very usefu e.g.l if one would like to do early stopping of an optimization based on converging energies or if the optimization gets stuck in linesearching and thereby produces vanishing energy differences.