Separate State and StoppingCriterionState initialization

[lkdvos]

This is a slight refactor of the initialization procedure, where I did the following

• Separate out the initialization of the state and stopping state, such you don't need to remember to call initialize_state! on the stopping state.
• Update the docs to include a custom stopping implementation with and without stateful stopping criterion
• Update the docs and tests to no longer reset the iterate in the initialize_state! calls, and only generate a random starting point in initialize_state as this led to confusion in solve! without initialize_state! #8. The idea is that initialize_state! really means making sure everything is set up to correctly start running the algorithm, which does not necessarily include a reset of the iterate but rather means setup like caches, counters, auxiliary memory etc.

[lkdvos]

@mtfishman, does this align slightly better with what you had in mind?

[codecov]

Codecov Report

❌ Patch coverage is 78.26087% with 5 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (main@c0ad8e3). Learn more about missing BASE report.

Files with missing lines	Patch %	Lines
src/stopping_criterion.jl	70.00%	3 Missing ⚠️
src/interface/stopping.jl	77.77%	2 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main       #9   +/-   ##
=======================================
  Coverage        ?   57.70%           
=======================================
  Files           ?        6           
  Lines           ?      227           
  Branches        ?        0           
=======================================
  Hits            ?      131           
  Misses          ?       96           
  Partials        ?        0           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[lkdvos]

https://juliamanifolds.github.io/AlgorithmsInterface.jl/previews/PR9

[kellertuer]

Oh, this is quiiiiiite some rework. I do not get understand what changed in practice though. It seems like now it's random whether initialise_state has an actual (algorithm) state or a stopping criterion state as third parameter? Is that the new part?

Or phrased differently: one now has to pass a sc-state to the init, but where is that from / initialised before?

[lkdvos]

Apologies, I could have explained this a whole lot better.

The main goal was to separate out state initialization from stopping state initialization, since I think it feels slightly awkward that you have to remember that when you are implementing an algorithm, you have to remember to also call initialize_state(!) for the stopping criterion.
Since this is something that always has to happen anyways, I tried separating that out into initialize_stopping_state! and initialize_state! in the solve! function, such that this is handled automatically.

Then I wanted to do the same thing for initialize_state and initialize_stopping_state, but since we have to attach the stopping_criterion_state into the generate state, I still have to pass that as an argument to the initialize_state, which then takes the stopping_criterion_state and generates a state from that.

Walking through the flow of solve(::Problem, ::Algorithm), we therefore have:

1. initialize_stopping_state(problem, algorithm, stopping_criterion = algorithm.stopping_criterion) -> stopping_criterion_state
2. initialize_state(problem, algorithm, stopping_criterion_state) -> state
3. enter solve!
4. initialize_stopping_state!(problem, algorithm, state, stopping_criterion = algorithm.stopping_criterion, stopping_criterion_state = state.stopping_criterion_state)
5. initialize_state!(problem, algorithm, state)
6. remainder of the algorithm

I can see the confusion with the third parameter being a state or stopping_criterion_state, but this is mostly because the signatures of the ! methods don't line up nicely with their non-mutating counterparts, and I still wanted to pass on all the parameters to the different functions in case any implementation requires access to that.

Finally, I wanted to not disallow workflows where the initialization of the stopping criterion state and of the regular state have to be intertwined and cannot be cleanly separated out.
To achieve this, I am only calling initialize_state(problem, algorithm) in the solve call, and have a default implementation that dispatches to initialize_state(problem, algorithm, initialize_stopping_state(problem, algorithm)).

[mtfishman]

It's a small change, but in practice what I've been doing is writing the definition of initialize_state more like this:

function AlgorithmsInterface.initialize_state(
        problem::SqrtProblem, algorithm::HeronAlgorithm,
        stopping_criterion_state::StoppingCriterionState;
        x0 = rand()
    )

Maybe I was being dense, but it took me some time to realize that was a "valid" way to define it and still have control over the parts of the state initialization that I wanted to control, since that allows running solve as:

solve(SqrtProblem(16.0), HeronAlgorithm(StopAfterIteration(10)); x0 = 1.0)

I think seeing the example written that way would have helped me understand how I should define initialize_state.

However, I realized a slightly awkward thing about defining initialize_state in that way and then calling solve as solve(SqrtProblem(16.0), HeronAlgorithm(StopAfterIteration(10)); x0 = 1.0) is that then x0 gets passed to both initialize_state and initialize_state!. Maybe that's not a problem in practice but I found it to be a bit strange (i.e. should initialize_state! use x0 or not?), and also made me confused about the roles of intialize_state vs. initialize_state!. I think the suggestion in this PR of just having initialize_state! reset iteration helps clarify that issue a bit.

[lkdvos]

The comment about the kwargs is a really good one, I was looking at this a bit before and it's somewhat difficult to come up with a way to split arbitrary keywords generically, but it could even make sense to just not pass the keyword arguments to initialize_state! anymore if they have already been passed to initialize_state?

[mtfishman]

I was thinking about that, indeed maybe it makes sense to not pass the keyword arguments to initialize_state!. That could make the distinction between initialize_state and initialize_state! clearer, since then initialize_state handles external inputs (i.e. initial guesses for the iterate) while initialize_state! just handles resetting the "internal" state, such as the iteration number and stopping criterion state. Calling solve! means that you made the state already, so it seems like anything handled through keyword arguments to initialize_state! could instead be handled by just modifying the state directly (i.e. before calling solve!/initialize_state!).

[kellertuer]

Hm, I disagree here, for me functions of same name should do the same and accept the same keywords. Or to be more precise

initialize_state should create a state from new memory but also
initialize_state! applied to the same memory state should set it to the same situation as well.

[kellertuer]

I think for me all this is getting a bit too much.

My original goal was to maybe integrate Manopt a bit better into the Julia world so people could use it. That failed with JuMP and it also failed with Optimization.
This package here was the idea to integrate it differently, but by now I noticed, the form we ended up with would mean to basically rewrite Manopt from scratch. That is some 10k lines of code.
I did rewrite LieGroups.jl last winter and I am currently rewriting the Manifolds.jl tests. Afterwards everything is a bit nicer, but it takes a huuuuge effort of time.
I will not do that further I think. So Manopt will probably not switch to this. You seem to have super good ideas and use cases already, so should we then maybe move it to some TensorKit namespace and you continue this overall?

I do not mean this in any negative way, I think it was nice to ponder about these ideas a bit, I just notice that I do not have the capacity to continue here - neither in motivation nor time. Nor does it look like I will be able to afford any future JuliaCons anyways (celebrating 10 years without grants by now).

Let me know what you think. Without me you can also easily follow the ideas above of doing something completely different in the mutating versus non mutating variants of functions and such.