Skip to content

Properly overload show#569

Merged
kellertuer merged 125 commits intodev-0.6from
mbaran/fix-show-repr
Apr 7, 2026
Merged

Properly overload show#569
kellertuer merged 125 commits intodev-0.6from
mbaran/fix-show-repr

Conversation

@mateuszbaran
Copy link
Copy Markdown
Member

@mateuszbaran mateuszbaran commented Jan 26, 2026

This is a WIP sketch for #560 .

Also fixes a few minor issues ChatGPT noticed.

In general the idea is as follows:

  • introduce a Base.show(io::IO, ::MIME"text/plain", x) for all elements x from Manopt, that generically either prints the status_summary in the multiline case or show(io,x) otherwise
  • implement show(io,x) to print (just) a nice constructor
  • implement status_summary(x; context = :default) to print a human-readable
    • a short form that can be used inside other statusses (context = :inline)
    • for those that may be reproduced by a factory (like debug or record) also the symbol can be printed (context = :short)
    • a (maybe longer) status summary of x should be the context = :default
  • introduce status_summary(io, x; context = ;default) that directly writes onto io and by default calls the normal one and writes that.
    (methods can switch this behaviour that the io generates text and the other materialises of course, then both have to be implemented)

The first has to be introduced for every main point (abstract type) below, and both show and status have to be checked for all concrete subtypes.

📋

For the abstract super types, the check indicates, that a 3-arg show for multiline text/html has been implemented to distinguish the human-readable status_summary and the short 2-arg show, “...” indicates, that the list is not yet complete.

  • Problem(s)
    • ConstrainedManoptProblem
    • DefaultManoptProblem
    • (Manopt.Test.DummyProblem, skipped for now, since this is only for testing and the use to tests its show does not seem necessary)
    • TwoManifoldProblem
    • VectorBundleManoptProblem
  • Objectives (<: AbstractManifoldObjective)
    • ScaledManifoldObjective
    • ManifoldConstrainedSetObjective
    • SymmetricLinearSystemObjective
    • Manopt.ReturnManifoldObjective
    • ScaledManifoldObjective
    • SimpleManifoldCachedObjective
    • ManifoldCostObjective
    • ManifoldProximalGradientObjective
    • ManifoldProximalMapObjective
    • ManifoldSubgradientObjective
    • AdaptiveRegularizationWithCubicsModelObjective
    • TrustRegionModelObjective
    • ManifoldAlternatingGradientObjective
    • ManifoldDifferenceOfConvexObjective
    • ManifoldDifferenceOfConvexProximalObjective
    • ManifoldStochasticGradientObjective
    • PrimalDualManifoldSemismoothNewtonObjective
    • NonlinearLeastSquaresObjective (this one is a bit inconsistent in naming – maybe also the two models above)
    • ConstrainedManifoldObjective
    • EmbeddedManifoldObjective
    • ManifoldCachedObjective
    • ManifoldCountObjective
    • ManifoldFirstOrderObjective
    • ManifoldHessianObjective
    • Manopt.ReturnManifoldObjective
    • PrimalDualManifoldObjective
    • SimpleManifoldCachedObjective
    • (Manopt.Test.DummyDecoratedObjective, skipped for now, since unlikely to require show/repr)
  • solver states (<: AbstractManoptSolverState) - for all states only new show methods and testing missing.
    • InteriorPointNewtonState
    • TruncatedConjugateGradientState
    • AlternatingGradientDescentState
    • ConjugateGradientDescentState
    • FrankWolfeState
    • LevenbergMarquardtState (postponed for now)
    • ProximalPointState
    • QuasiNewtonState
    • StochasticGradientDescentState
    • VectorBundleNewtonState
    • AdaptiveRegularizationState
    • ConjugateResidualState
    • ConvexBundleMethodState
    • CyclicProximalPointState
    • DouglasRachfordState
    • LanczosState
    • AugmentedLagrangianMethodState
    • DifferenceOfConvexProximalState
    • DifferenceOfConvexState
    • ExactPenaltyMethodState
    • TrustRegionsState
    • Manopt.CMAESState
    • Manopt.ClosedFormSubSolverState
    • Manopt.ReturnSolverState
    • Manopt.Test.DummyState
    • MeshAdaptiveDirectSearchState
    • NelderMeadState
    • ParticleSwarmState
    • ProximalBundleMethodState
    • ProximalGradientMethodState
    • StepsizeState
    • SubGradientMethodState
    • DebugSolverState
    • GradientDescentState
    • RecordSolverState
  • DirectionUpdateRule
    • Manopt.AverageGradientRule
    • Manopt.ConjugateDescentCoefficientRule
    • Manopt.ConjugateGradientBealeRestartRule
    • Manopt.DaiYuanCoefficientRule
    • Manopt.DirectionUpdateRuleStorage
    • Manopt.FletcherReevesCoefficientRule
    • Manopt.HagerZhangCoefficientRule
    • Manopt.HestenesStiefelCoefficientRule
    • Manopt.HybridCoefficientRule
    • Manopt.IdentityUpdateRule
    • Manopt.LiuStoreyCoefficientRule
    • Manopt.MomentumGradientRule
    • Manopt.NesterovRule
    • Manopt.PolakRibiereCoefficientRule
    • Manopt.PreconditionedDirectionRule
    • Manopt.SteepestDescentCoefficientRule
    • Manopt.AlternatingGradientRule
    • Manopt.StochasticGradientRule
  • Stepsize (just missing the human readable status_summary)
    • ArmijoLinesearchStepsize
    • AdaptiveWNGradientStepsize
    • AffineCovariantStepsize
    • ConstantStepsize
    • DecreasingStepsize
    • DistanceOverGradientsStepsize
    • DomainBackTrackingStepsize
    • LineSearchesStepsize
    • NullStepBackTrackingStepsize
    • PolyakStepsize
    • ProximalGradientMethodBacktrackingStepsize
    • WolfePowellLinesearchStepsize
    • WolfePowellBinaryLinesearchStepsize
  • Stopping criteria (<: StoppinCriterion)
    • StopWhenBestCostInGenerationConstant
    • StopWhenCovarianceIllConditioned
    • StopWhenCriterionWithIterationCondition
    • StopWhenCurvatureIsNegative
    • StopWhenEvolutionStagnates
    • StopWhenFirstOrderProgress
    • StopWhenModelIncreased
    • StopWhenPopulationCostConcentrated
    • StopWhenPopulationDiverges
    • StopWhenPopulationStronglyConcentrated
    • StopWhenRelativeResidualLess
    • StopWhenResidualIsReducedByFactorOrPower
    • StopWhenSwarmVelocityLess
    • StopWhenTrustRegionIsExceeded
    • StopAfter
    • StopAfterIteration
    • StopWhenAll
    • StopWhenAllLanczosVectorsUsed
    • StopWhenAny
    • StopWhenChangeLess
    • StopWhenCostChangeLess
    • StopWhenCostLess
    • StopWhenCostNaN
    • StopWhenEntryChangeLess
    • StopWhenGradientChangeLess
    • StopWhenGradientMappingNormLess
    • StopWhenGradientNormLess
    • StopWhenIterateNaN
    • StopWhenKKTResidualLess
    • StopWhenLagrangeMultiplierLess
    • StopWhenPollSizeLess
    • StopWhenPopulationConcentrated
    • StopWhenProjectedGradientStationary
    • StopWhenRepeated
    • StopWhenSmallerOrEqual
    • StopWhenStepsizeLess
    • StopWhenSubgradientNormLess
  • Debug/Record Actions (AbstractSolverAction (and DebugActionas well as RecordAction))
    • DebugCallback
    • DebugChange
    • DebugCost
    • DebugDivider
    • DebugDualChange
    • DebugDualResidual
    • DebugEntry
    • DebugEntryChange
    • DebugEvery
    • DebugFeasibility
    • DebugGradient
    • DebugGradientChange
    • DebugGradientNorm
    • DebugGroup
    • DebugIfEntry
    • DebugIterate
    • DebugIteration
    • DebugMessages
    • DebugPrimalDualResidual
    • DebugPrimalResidual
    • DebugProximalParameter
    • DebugStepsize
    • DebugStoppingCriterion
    • DebugTime
    • DebugWarnIfCostIncreases
    • DebugWarnIfCostNotFinite
    • DebugWarnIfFieldNotFinite
    • DebugWarnIfGradientNormTooLarge
    • DebugWarnIfLagrangeMultiplierIncreases
    • DebugWarnIfStepsizeCollapsed
    • DebugWhenActive
    • RecordChange
    • RecordCost
    • RecordEntry
    • RecordEntryChange
    • RecordEvery
    • RecordGradient
    • RecordGradientNorm
    • RecordGroup
    • RecordIterate
    • RecordIteration
    • RecordProximalParameter
    • RecordStepsize
    • RecordStoppingReason
    • RecordSubsolver
    • RecordTime
    • RecordWhenActive
  • Further elements
    • LowerTriangularAdaptivePoll
    • DefaultMeshAdaptiveDirectSearch
    • NelderMeadSimplex

@mateuszbaran mateuszbaran added the WIP Work in Progress (for a pull request) label Jan 26, 2026
@kellertuer
Copy link
Copy Markdown
Member

I thought a bit of a structure and here are the ideas.

1. A new dispatch router

Every abstract type (the “most upper one in the Hierarchy”) implements a 3-arg show (for the text/html) that checks for multiline. If the is present it calls status_summary(io, e) on the thingy e that is of this type, for example

function Base.show(io::IO, ::MIME"text/plain", ams::AbstractManoptSolverState)
multiline = get(io, :multiline, true)
if multiline
return status_summary(io, ams)
else
show(io, ams)
end
end

Does this for all states

2. a new dispatch router for the status summary

status_summary(io, e) by default calls status_summary(e) and puts the resulting string onto io. This way all old summaries can stay and if there is ever a reason to write an status summary that should not materialise the full string, the new io one can be used

3. new 2-arg show methods

What is now needed, besides a few more functions from 1 are the shorter

Example

I did write a first draft for GradientDescentState, but note that for the single fields the print might still be a too-long-one

julia> using Manifolds, Manopt

julia> st = GradientDescentState(Euclidean(3))
# Solver state for `Manopt.jl`s Gradient Descent

## Parameters
* retraction method: ExponentialRetraction()

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

## Stopping criterion
Stop When _one_ of the following are fulfilled:
  * Max Iteration 200:  not reached
  * |grad f| < 1.0e-8: not reached
Overall: not reached
This indicates convergence: No

julia> repr(st)
"GradientDescentState(; direction=Manopt.IdentityUpdateRule(), p=[-1.1757590520224461, 1.4736732113345241, 0.047616008870685164], stepsize=ArmijoLinesearch(;\n    initial_stepsize=1.0,\n    retraction_method=ExponentialRetraction(),\n    contraction_factor=0.95,\n    sufficient_decrease=0.1,\n), stopping_criterion=StopWhenAny{Tuple{StopAfterIteration, StopWhenGradientNormLess{typeof(norm), Float64, Missing}}}((StopAfterIteration(200), StopWhenGradientNormLess{typeof(norm), Float64, Missing}(LinearAlgebra.norm, 1.0e-8, 0.0, -1, missing)), -1), retraction_method=ExponentialRetraction(), X=[0.0, 0.0, 0.0])"

Open points

  • For something like Armijo it's not as easy, because it would only generate a Factory in general and one would have to pass a manifold afterwards to construct it (or actually display the inner ArmijoLinesearch constructor. On the other hand the IndentityUpdateRule is also the internal one, so maybe printing internal ones is fine enough with the prefix.
  • A lot of fields, e.g. in Objectives are functors, there it is unclear to me how to do the short form. Here I am not so sure what to do. Maybe use the default notation and print something like ManifoldCostObjective(f; evaluation=AllocatingEvaluation() ? the repr(obj.f) would probably look strange.

Let me know what you think. If this is reasonable, then I would propose to edit your first post slowly and go through all states, problems, objectives, line searches,... and unify this.

@mateuszbaran
Copy link
Copy Markdown
Member Author

I think it would be more general to have the basic method to implement be the one with io but the "generate a string and write it to the io" is still an improvement over the current approach on the master branch.

  • For something like Armijo it's not as easy, because it would only generate a Factory in general and one would have to pass a manifold afterwards to construct it (or actually display the inner ArmijoLinesearch constructor. On the other hand the IndentityUpdateRule is also the internal one, so maybe printing internal ones is fine enough with the prefix.

ArmijoLinesearchStepsize should print as ArmijoLinesearchStepsize, and ArmijoLinesearch should print as ArmijoLinesearch. Lying about the types can cause difficult to identify issues. I don't want to be lied to. The multi-line message may be less precise but I'd still like to be able to differentiate between ArmijoLinesearchStepsize and ArmijoLinesearch based on the printed output.

  • A lot of fields, e.g. in Objectives are functors, there it is unclear to me how to do the short form. Here I am not so sure what to do. Maybe use the default notation and print something like ManifoldCostObjective(f; evaluation=AllocatingEvaluation() ? the repr(obj.f) would probably look strange.

The short form of states should rarely be displayed to users, to me the important parts are:

  1. Type name is correct.
  2. Key stored values are displayed.
  3. (preferable but not must-have) I can paste the output to REPL and get a similar object.

@kellertuer
Copy link
Copy Markdown
Member

ArmijoLinesearchStepsize should print as ArmijoLinesearchStepsize, and ArmijoLinesearch should print as ArmijoLinesearch. Lying about the types can cause difficult to identify issues. I don't want to be lied to. The multi-line message may be less precise but I'd still like to be able to differentiate between ArmijoLinesearchStepsize and ArmijoLinesearch based on the printed output.

ArmijoLInesearch would then nearly never print, since the factory only exists for a very short time in the construction phase, but yeah, then this (in the state) will print in the future as Manopt.ArmijoLinesearchStepsize

The short form of states should rarely be displayed to users, to me the important parts are:

  1. Type name is correct.
    That's easy
  1. Key stored values are displayed.
    can you elaborate a bit? Fields of the struct? that would include functions like f or grad f that a I can not easily display. But sure kwargs or args of the constructor, those I can display for sure (as in the example above).
  1. (preferable but not must-have) I can paste the output to REPL and get a similar object.

The “get similar object” would not easily work for the functions, since I can either print them nice as f (which is a bit of a lie) or one would usually seee #45#44 for example. So as soon as functions are involved, the pasting would probably not work (and then I prefer f over #45#44#458i6573 anonymous functions).

@kellertuer
Copy link
Copy Markdown
Member

Lying about the types can cause difficult to identify issues. I don't want to be lied to. The multi-line message may be less precise but I'd still like to be able to differentiate between ArmijoLinesearchStepsize and ArmijoLinesearch based on the printed output.

That is actually a bit complicated, since the ArmijoLinesearchStepsize would have a pretty print that says

[Status Summary of an] Armijo Line search

[... details ...]

While the ArmijoLinesearch would more like be. (it does not exist yet but might be worth adding this

Armijo Line search Factory

[... details maybe the same as a or a fraction of them ... ]

(this could end in “call this factory with a manifold and you get an actual Armijo linesearch (stepsize).”

The reason it is this way is ease of use when calling constructors.

@mateuszbaran
Copy link
Copy Markdown
Member Author

ArmijoLInesearch would then nearly never print, since the factory only exists for a very short time in the construction phase, but yeah, then this (in the state) will print in the future as Manopt.ArmijoLinesearchStepsize

Some people might store the factory in their code somewhere, so it can print from there. I sometimes do that myself.

The “get similar object” would not easily work for the functions, since I can either print them nice as f (which is a bit of a lie) or one would usually seee #45#44 for example. So as soon as functions are involved, the pasting would probably not work (and then I prefer f over #45#44#458i6573 anonymous functions).

Yes, that's why I consider it only a nice-to-have. If anonymous functions are stored inside, that's a great example of where we can forget about copy-pasting.

That is actually a bit complicated, since the ArmijoLinesearchStepsize would have a pretty print that says

Yes, these examples are fine.

The reason it is this way is ease of use when calling constructors.

Yes, it's perfectly fine to include factory call corresponding to the line search state but I'd like to see that the object is a state in the description.

@kellertuer
Copy link
Copy Markdown
Member

Then I think we agree. And sure the long forms I am fine with stating a bit more precise that the XStepsize is more like a state (a stepwise “in action”) while the X types are factories for those states.
For these one would print the defaults it does set and add a note that once a manifold provide, a XStepsize is created.

Then I think I have a reasonable overview about the things to do. Once I finish parts, I will slowly populate a list in the initial post to keep track of the progress. Thanks for the idea to unify this and for the discussions.

@kellertuer
Copy link
Copy Markdown
Member

For the human readable variant (status_summary) I introduced inline = true | false to e.g. just print symbols of the debugs when they are listed inline.

I also filled the list above for states / objectives and problems already and will work through them slowly next.

@kellertuer
Copy link
Copy Markdown
Member

kellertuer commented Apr 3, 2026

Status update:

It seems there are only 6 step sizes missing. Step sizes currently take a bit of time, since I unify them in their promotion of real types (we had a few manual fixes for single fields, we will get that for all fields now) – and the last 6 ones are some with several fields. But at least it is slowly closing in going through all show/repr-s

After that it is just quite a few tests for the new show methods and a few status_summaries that have to be written.

edit: I wrote the remaining ones today. The rest of this PR is to get code coverage back up; looks like mostly checking that the show methods all print the right constructor (maybe it is enough to check that the function name is correct).

@mateuszbaran
Copy link
Copy Markdown
Member Author

Nice, hopefully there won't be any more surprises here 🙂

@kellertuer
Copy link
Copy Markdown
Member

Since tests and docs are at least not failing it is hopefully just working through the remaining code cov, yes.

@kellertuer kellertuer added the Ready-for-Review A label for pull requests that are feature-ready label Apr 7, 2026
@mateuszbaran
Copy link
Copy Markdown
Member Author

LGTM 👍 (I can't issue a Github-approval because I've opened this PR, despite you doing vast majority of work here).

@kellertuer kellertuer changed the base branch from master to dev-0.6 April 7, 2026 08:07
@kellertuer
Copy link
Copy Markdown
Member

I diverted the PR to be collected on a dev-0.6 branch, since we plan to collect a few forthcoming breaking changes for that release.

@kellertuer kellertuer merged commit 33c090d into dev-0.6 Apr 7, 2026
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking Ready-for-Review A label for pull requests that are feature-ready

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants