Properly add methods to `show` to have valid `repr`

[mateuszbaran]

I've noticed that for example many stopping criteria have currently invalid repr due to not producing parsable output in two-arg show. We should move the non-parsable output to the three-argument show like explained here: https://scientificcoder.com/user-defined-show-method-in-julia#heading-custom-show .

[mateuszbaran]

Example here: 8268499

[kellertuer]

Sure, might be a bit of work, to do that for all of them consistently, but sounds reasonable.

[kellertuer]

Hm, I have a question to this I think.

For now a solver run reports its result nicely, cf the first run in https://manoptjl.org/stable/tutorials/CountAndCache/#Counting

For that, a solver state has to overwrite the 2-argument show method, right?
Because all stopping criteria, all solver states and several other data structures follow then the same route to provide that detailed summary.

How should that then be refactored to? I would still like to have the “easy” access that return_state=true should give the nice report as in the link above.
This would mean at least for states I would not be able to follow this issue or am I wrong?
If so, then “where to draw the line” between those that do report a human-readable summary on REPL and those that “have valid repr”?
(I mainly would like to understand this better before starting to refactor all show methods, which this issue seems to make necessary. All of them.)

[mateuszbaran]

Well, I think there would have to be one PR that consistently implements the 2-argument and 3-argument show for all types. So states would also need to be handled.

If so, then “where to draw the line” between those that do report a human-readable summary on REPL and those that “have valid repr”?

Ideally all types should have both, and the example in the first post shows that it's possible with a bit of work.

[kellertuer]

That is exactly why I wanted to check back. My problem is that calling

gradient_descent(M, f, grad_f, p0; return_state=true)

like in the above example, printing the state as a summary is nice to have.
If we want to allow for that, we have to break the consistency you want to have at least a bit.

For anything other than the state, I am on your side with your code above.

For the state I would prefer to not break the code from the example or the call above and to print a summary. That would mean that states do not follow the scheme but print their summary instead. Would that be ok?

Otherwise all the summary work that currently exists becomes useless because a user in interactive mode on REPL would never see that – and that is the only reason it exists.
One thing we could do to minimise this is, to do a PrintState or so, where show is the summary, and return_state=true would wrap that? Seems a bit clumsy to me though.

TLDR: I would like to keep the nice summary return_state=true provides on REPL and think for states that breaks with your consistency. What to do?

[mateuszbaran]

I've tried looking into your issue and I don't know what's the problem. In the PR #569 I've implemented a bunch of show methods as suggested here and the state is still printed nicely:

julia> s = gradient_descent(
           M,
           f,
           grad_f,
           data[1];
           stopping_criterion = StopAfterIteration(200) | StopWhenChangeLess(M, 1.0e-16),
           stepsize = ArmijoLinesearch(; contraction_factor = 0.99),
           debug = d,
           record = [:Iteration, :Cost, 1],
           return_state = true,
       )
# Solver state for `Manopt.jl`s Gradient Descent
After 113 iterations

## Parameters
* retraction method: ExponentialRetraction()

## Stepsize
ArmijoLinesearch(;
    initial_stepsize=1.0,
    retraction_method=ExponentialRetraction(),
    contraction_factor=0.99,
    sufficient_decrease=0.1,
)

## Stopping criterion

Stop When _one_ of the following are fulfilled:
  * Max Iteration 200:  not reached
  * |Δp| < 1.0e-16: reached
Overall: reached
This indicates convergence: No

## Debug
    :Iteration = [(:Iterate, "p: %s"), " ", (:Cost, "f(x): %f"), :Stop, "\n", (:Messages, :Once), 500]

## Record
(Iteration = RecordEvery(RecordGroup([RecordIteration(), RecordCost()]), 1, true),)

I haven't fixed all states and test yet.

Could you show me what doesn't work about gradient descent printing in that PR? Or does that work fine?

[kellertuer]

Ah, that looks fine.

I do not follow that PR completely (i.e. in what it does), but you seem to take into account what I was asking for – or I even misunderstood this issue as

All show methods should always only show a valid constructor recreating the thing they show

(which for a state would also not be possible, e.g. a stopping criterion can not directly be created in a state where it indicates to stop).

So what you write here and what you propose in the PR looks fine to me, as long as the print as you have it in this post for the solver call stays in the descriptive form it currently is in :)

[kellertuer]

Ah I have on thing that I do not understand: How does the (seemingly) new print_object differ from the existing status_summary? I think that is the same so we could reuse the old name.

[mateuszbaran]

The three-arg show is free to print whatever we want 🙂 . Only the two-arg one is restricted.

How does the (seemingly) new print_object differ from the existing status_summary? I think that is the same so we could reuse the old name.

print_object can print to an arbitrary IO and supports both repr-like and human-readable outputs (via the multiline kwarg). I think we could extend status_summary instead of introducing a new function.

[kellertuer]

Then I am surprised that the 3-arg one does something specific on REPL, I thought that would always only be the 2-arg one.

Then – sure we should do as you started (and thanks for starting it!) – and yes, if they are similar enough, I think the name status_summary is a bit nicer and I would prefer to have that one extended then.