Introduced an entry change stopping criterion and generalise the gradient norm SC.

JuliaManifolds / Manopt.jl

🏔️Manopt. jl – Optimization on Manifolds in Julia

http://manoptjl.org

Other

314 stars 40 forks source link

Introduced an entry change stopping criterion and generalise the gradient norm SC. #345

Closed kellertuer closed 8 months ago

kellertuer commented 8 months ago

This PR

[x] introduces a StopWhenEntryChangeLess stopping criterion
[x] generalises the StopWhenGradientNormLess to accept arbitrary norm functions. Can you check, @mateuszbaran, whether that includes your InfNorm one? I think this way it is much nicer to introduce different Norm-possibilities.
[x] the entry change should be used in the PSO to check the population check (instead of “misusing” the iterate one)
[x] needs test coverage after clarification of functionality.

codecov[bot] commented 8 months ago

Codecov Report

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

Comparison is base (86a2866) 99.78% compared to head (747efe6) 99.62%.

:exclamation: Current head 747efe6 differs from pull request most recent head f4e830b. Consider uploading reports for the commit f4e830b to get more accurate results

Additional details and impacted files

```diff @@ Coverage Diff @@ ## master #345 +/- ## ========================================== - Coverage 99.78% 99.62% -0.16% ========================================== Files 69 69 Lines 6380 6398 +18 ========================================== + Hits 6366 6374 +8 - Misses 14 24 +10 ```

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

kellertuer commented 8 months ago

The decrease per patch is just the 10 lines of Strong Wolfe again that depending on the randomness in the tests sometimes are not reached. So this is ready to go.

mateuszbaran commented 8 months ago

BTW, yes, this is enough to represent the inf norm criterion :+1:

kellertuer commented 8 months ago

BTW, yes, this is enough to represent the inf norm criterion 👍

Nice! I had hoped so – and feel this is nicer than coming up with a new criterion (which might mean coming up with new crieria for other norms later as well, and then it gets too crowded). Just took me a while to see this relatively simple solution.

kellertuer commented 8 months ago

HM I just saw that you had one fix to just compute some norm only once, but it seems to have broken formatting and tests. Will check this only after my lecture and some meetings somewhen in the late afternoon.

mateuszbaran commented 8 months ago

I've fixed tests and formatting.

kellertuer commented 8 months ago

Great thanks. Is the discussion above clarified and with that this PR ready to merge?