Skip to content

Conversation

bagibence
Copy link
Owner

No description provided.

bagibence and others added 30 commits March 12, 2025 12:00
…rue)`.

Now, I noticed that the downstream `AbstractGaussNewton.terminate` method does not actually check the result when deciding whether to terminate. Rather than adjusting just the one solver, I decided to simply lift this condition into the top-level `_iterate.py`.

As such we might consider whether we really need `.terminate` to return both `termintae: bool` and `result: RESULTS`. To consider what each combination means:

- `terminate=True`, `result=RESULTS.successful`: done, and all went well.
- `terminate=True`, `result=RESULTS.foo`: something went wrong; halt.
- `terminate=False`, `result=RESULTS.successfull`: keep making more iterations.
- `terminate=False`, `result=RESULTS.foo`: <invalid>

So we really do need two return arguments: it's not enough to just stop if `result != RESULTS.successful`, because otherwise we have no way to communicate success-and-terminate.

Now we might wonder whether this change produces a change in behaviour. I think technically yes: a solver may have set `terminate=False` and `result=RESULTS.foo`, and then later decided to switch back to being successful. In practice I don't think we ever did that, and it seems pretty sketchy anyway.
Fixed Gauss-Newton including a spurious `lx.linear_solve(..., throw=True)`
…_fix

Support Optax solvers that include a linesearch
bagibence added 26 commits June 13, 2025 17:45
… using Zoom

In the accepted branch avoid recalculating the gradient,
and reuse it reading from ZoomState.current_point.grad
as this is the gradient at y_eval if y_eval was accepted.
Suggested by all sources and required for test_compat to pass with BFGS.
Instead of passing lin_fn and options to searches,
add a _needs_grad_at_y_eval to the searches.

In AbstractQuasiNewton and AbstractGradientDescent:
if the search needs the gradient, calculate it and include it
in f_eval_info.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants