JuliaManifolds / Manopt.jl

🏔️Manopt. jl – Optimization on Manifolds in Julia
http://manoptjl.org
Other
314 stars 40 forks source link

Introduce support of subsolver records. #377

Closed kellertuer closed 4 months ago

kellertuer commented 4 months ago

This is a PR to resolve #371.

ToDo

Open Points

Currently the symbol :Subsolver is used in subsolvers to declare them beding dependent (of the parent), while it might be breaking :WhenActive might be a nicer symbol. The :Subsolver could be used to indicate subsolver recordings. I have to think about this a bit and whether this is seriously breaking, since for now :Subsolver. On the other hand it is not breaking code to the extend it does now work, just that since the last release to this then some might get a bit more subsolver debug.

kellertuer commented 4 months ago

The design is nice and seems to work in general. I went for :WhenActive that is much nicer to use in writing

The main thing that seems to bother a bit still is, that though there is a lot of resets implemented, it seems that the recording of subsolver recordings is still incremental, this needs to be fixed still. So though recorded values are reset to the empty array the next push still pushes to the previous recordings array – no clue yet why.

kellertuer commented 4 months ago

Got it to work, even with a small add-on such that with the keywords

        record = [ :Iteration, :Cost, (:Subsolver, :Stop)],
        sub_kwargs = [
            :debug => [:Stop, :WhenActive],
            :record => [:Stop => :Stop]
        ],

The subsolver records its stopping reason at the end, and the “Subsolver recorder” takes over everything recorded at the stop. (by default that second element is :Iteration, which takes over all recordings from there iterations of the subsolver).

One can of course also do :record => [:Stop => [:Stop, :Cost]], for the subsolver, than at every stop, the stopping criterion and the cost are recorded, and taken over by the subsolver recorder.

So with that this is feature complete (but not yet doc / test complete).

codecov[bot] commented 4 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 99.76%. Comparing base (0d899bf) to head (610a873). Report is 1 commits behind head on master.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #377 +/- ## ======================================= Coverage 99.76% 99.76% ======================================= Files 74 74 Lines 7167 7241 +74 ======================================= + Hits 7150 7224 +74 Misses 17 17 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.