Add DefaultState and DefaultStoppingCriterionState implementations

[lkdvos]

This PR is an attempt to reduce some boilerplate in downstream implementations, both surrounding struct definitions as well as the initialize_state boilerplate code.

Before, we had to write the same three things before running a single iteration: a state struct declaring iterate, iteration and stopping_criterion_state, an initialize_state allocating it, and an initialize_state! resetting it.
Since none of this is really specific to any algorithm, I've added DefaultState, holding those three expected properties next to a single generic data field for anything else an algorithm carries from one step to the next.
Similarly, I gave DefaultStoppingCriterionState the same data field, defaulting to nothing so existing zero-argument construction is unaffected.

Then, I've added defaults for initialize_state and initialize_state! at both levels, taking their arguments positionally as (problem, algorithm, iterate, state_data, stopping_state_data) next to the keyword form the interface documents.
We can discuss the keyword form or the positional form, I felt like keywords here might lead to awkward dispatch and type-stability issues, but I'm open to reconsider this.
While this is in principle breaking for the new recommended way of doing this, it should be completely backwards compatible.

As a result, an algorithm now only has to provide a Problem, an Algorithm and a step!.
StopAfterIteration no longer carries its own initialization pair, which the criterion-level defaults now cover.

I've also reworked the documentation so all three pages lead with the default and present a state of your own as the option, keeping HeronState as the closing example on the interface page.

---

Something to discuss:
In principle, this approach is actually as expressive as the abstract State with subtypes, since you can always add whatever you like in the .data fields, and can additionally dispatch on that. In that sense, it might actually make sense to do a (breaking) change here and simply make DefaultState -> State, and remove the abstract type altogether. While this is breaking, it also enforces that the required properties are present, which seems like a good quality to have.
As a result, we could consider marking initialize_state as public but non-exported, since the default implementation now actually should be able to "just work".

As always, naming and design choices discussions more than welcome, this is just my attempt at simplifying some of the implementations and removing some of the boilerplate.

[lkdvos]

@mtfishman, @jack-dunham - linking you both here in case you would like to have a look/share opinions

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.63%. Comparing base (5a2d0b4) to head (0fa7cff).

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #32      +/-   ##
==========================================
+ Coverage   99.60%   99.63%   +0.02%     
==========================================
  Files           6        7       +1     
  Lines         255      274      +19     
==========================================
+ Hits          254      273      +19     
  Misses          1        1              

☔ View full report in Codecov by Harness.
📢 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.

[kellertuer]

I like the idea, though until now I mainly used a slightly different approach (in Manifolds.jl and Manopt.jl that is):

the DefaultState would illustrate the minimal things one needs, but not store data.
Any algorithm that needs something additional would repeat the 3 fields in its state and then add its own fields.

I am not saying my approach is per se better. I do see the danger of “overwrapping” in your case though. Somewhen you might end up with “DefaultData” and therein continue with some further default...

What do you think?

Another note, since we are discussing states, in Manopt.jl we introduced callbacks a while back, see JuliaManifolds/Manopt.jl#626 or https://manoptjl.org/stable/tutorials/HowToCallback/ for the new tutorial. Since we do talk about a default state, we could add that here as well, indeed. The field is just a dictionary of functions (of an agreed-upon-signature), the actual callback code is quite short https://github.com/JuliaManifolds/Manopt.jl/blob/master/src/base/state/callback.jl and the actually “hooks” (places where the callbacks are called) are also just one line each, see for example the generic ones at https://github.com/JuliaManifolds/Manopt.jl/blob/5e362ee0f07fb656dfdf1a40a616a94c3ba27260/src/solvers/solver.jl#L164-L177

[lkdvos]

I like the idea, though until now I mainly used a slightly different approach (in Manifolds.jl and Manopt.jl that is):

the DefaultState would illustrate the minimal things one needs, but not store data.
Any algorithm that needs something additional would repeat the 3 fields in its state and then add its own fields.

I am not saying my approach is per se better. I do see the danger of “overwrapping” in your case though. Somewhen you might end up with “DefaultData” and therein continue with some further default...

I definitely feel like I agree with you here, I think this was what I was getting at with the question about replacing the abstract State directly with the current implementation. I think I felt the things you are discussing but didn't really realize what was causing it :)

Let me try that out just to see how it would look, the only annoying part being that it is breaking but that's what 0.x versions are for I guess.

[kellertuer]

I still think the DefaultState is a very good idea. Sometimes an iterate and a a stopping criterion is all that is basically needed. Otherwise it still serves as a great reference what a state should contain :)

[lkdvos]

Alright, second attempt:

I took the State thing seriously now and moved a bunch of stuff around, I think overall this is definitely an improvement but I'd be interested to hear other opinions on this.

What I like the most about this approach is that we no longer rely on correctly defining the right fields for compliance with this package, and in the meantime get to hide a lot of the boilerplate that is related to that.

What I like a little less, although I think it might be acceptable, is that because we are defining the structs, it is a little less transparent what is going on and which data is put where. Ultimately, I think I quite like this approach.

[mtfishman]

I like the idea of having a DefaultState, though I think I agree with @kellertuer that a downside of the latest design is that if you want to dispatch you then need a struct you put in data so it pushes the complexity there. I.e. I feel like if you want a custom state object (say, you want to be able to write state.x = ... for a custom field x) it would be easier to define a new State subtype rather than define a new type to go in data and then dispatch on that (obviously both approaches can achieve the same thing but the mental burden of making a new data object feels higher to me).

[kellertuer]

In the last commit the abstract State was removed? I think this is still very relevant and important to have, there might be more complex states to have in mit than the (Default)State we now have.

[lkdvos]

This last commit was the idea to get rid of the abstract state, so the default path is without as much friction, and then have explicit implementations add a .data field they can use. One thing that might be useful there is that in principle I can make that a NamedTuple and forward the getproperty calls, which effectively turns the whole thing into a dynamic struct that can hold any data and fields you want, but is guaranteed to have the ones we need. I agree that for dispatch this is less suited, but the more I think about this the more I feel like probably a single algorithm/problem combination should be tied to a single state anyways, to avoid nightmares with ambiguity issues.

[jack-dunham]

The DefaultState idea is good, although my preference would personally be to remove the abstract State (as you have done), but also remove signatures that have ::State as an annotation, in favour of ::Any. The interface for a state object (which can subtype anything it likes) is then simply:

state.iterate     # returns the iterate (the data that gets passed around)
state.iteration   # returns the iteration. 

Then one can define concrete DefaultState with the data field for convenience, and overload the associated methods to also accommodate state.data on DefaultState.

[kellertuer]

I was maybe too much still in Manopt and thought every algorithm would need its own state. That is not the case, though different algorithms need very different “dynamic” parts they store in the state.

The “dynamic structure with names tuple” sounds a bit like a hack. For me it then depends a lot on how “safe” that is, whether I like that. I am not yet sure.

If we always one have one state, then sure we do not need a default one.