New benchmark #339

mateuszbaran · 2023-12-28T12:11:03Z

To compare performance of Manopt.jl and Optim.jl. TODO:

Why does Manopt perform more iteration? I'm trying to match parameters of Optim.jl and Manopt.jl as much as possible. Line search is different but for now StrongWolfe errors for Manopt.
Are we fine with StopWhenGradientInfNormLess?

codecov · 2023-12-28T12:20:51Z

Codecov Report

Attention: 3 lines in your changes are missing coverage. Please review.

Comparison is base (8851619) 99.45% compared to head (649bd55) 99.57%.
Report is 2 commits behind head on master.

❗ Current head 649bd55 differs from pull request most recent head 92db97e. Consider uploading reports for the commit 92db97e to get more accurate results

Files	Patch %	Lines
src/solvers/quasi_Newton.jl	25.00%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #339      +/-   ##
==========================================
+ Coverage   99.45%   99.57%   +0.12%     
==========================================
  Files          69       69              
  Lines        6418     6402      -16     
==========================================
- Hits         6383     6375       -8     
+ Misses         35       27       -8

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

kellertuer · 2023-12-28T12:40:38Z

Instead of StopWhenGradientInfNormLess one could also consider a norm= keyword for StopWhenGradientNormLess – but that should still default to the 2-norm, since

basis would introduce more complexity for users with individual manifolds
we would change the default which I would consider breaking.

I am not sure why strong Wolfe errors, that would be interesting to know.

We could also remove the two existing benchmarks since I never ran them – in the long run we could set up a benchmark CI, but I have not yet fully understood how they work and which packages one would use for that.

kellertuer · 2023-12-28T18:07:11Z

ext/ManoptLineSearchesExt.jl

@@ -59,6 +59,7 @@ function (cs::Manopt.LineSearchesStepsize)(
        return α
    catch ex
        if isa(ex, LineSearches.LineSearchException)
+            println(ex)


I think I would prefer a re-throw of the error here; and maybe we should introduce such errors in our line searches as well.

OK, then the easiest solution is to just not catch the error. We could have that in our line searches too.

src/solvers/quasi_Newton.jl

mateuszbaran · 2023-12-28T19:03:58Z

The test failure seems unrelated to my changes (it's the ALM solver).

kellertuer · 2023-12-28T19:11:41Z

Well, ALM has a subsolver that often is L-BFGS. So if you ALM breaks, Quasi Newton changed (maybe an error, but sometimes ALM is also a bit unstable). But it is an effect of changing L-BFGS.

mateuszbaran · 2023-12-28T21:13:00Z

OK, then I will pay attention to it.

mateuszbaran · 2023-12-30T20:05:01Z

Ref. JuliaNLSolvers/LineSearches.jl#173 .

kellertuer · 2024-01-16T12:30:51Z

I also think it is not the optimal solution, but I also do agree, that thoroughly working through that code, making it (1) more Manopt.jl-like and (b) documenting it thoroughly might take more time indeed.

mateuszbaran · 2024-01-16T20:18:50Z

src/solvers/quasi_Newton.jl

@@ -345,6 +345,11 @@ function step_solver!(mp::AbstractManoptProblem, qns::QuasiNewtonState, iter)
    M = get_manifold(mp)
    get_gradient!(mp, qns.X, qns.p)
    qns.direction_update(qns.η, mp, qns)
+    if real(inner(M, qns.p, qns.η, qns.X)) > 0


So, one of the issues in some cases that LineSearches.jl caught but Manopt didn't is that L-BFGS selected a non-descent direction. IIRC I had a similar issues with CG at some point. This fixes the problem but I don't know if this is the right way to solve it.

How far is that negative? Rounding errors? To me this fix looks more like hard-core reset if error :/ I would prefer an actual fix if this is a bug.

Much more than a rounding error. You can try running the test_case_manopt() function from the PR to observe the issue. The direction gets wrong by the third iteration. I'd prefer a better fix too but it's fairly difficult to figure it out.

It never appears in my tests on flat Euclidean where Manopt's iterations are very close to Optim.jl's iterations. So, I don't know, but maybe it has something to do with vector transports. Both projection transport and parallel transport give this error though.

With this fix, both Manopt and Optim converge well on Rosenbrock with spherical constraints. Sometimes Manopt is faster, and sometimes Optim. For Euclidean Rosenbrock, Manopt is consistently a bit faster.

Well, this “fix” is till of the kind “if it is wrong reset, restart” instead of finding out what is wrong. For projection transport I think this can happen, since that only approximates, it should not happen for parallel transport. But with now 3 master students, I will not have much time beyond my own small PRs I have planned (or want to finish).

But I would still prefer to not have such duck-typing fixes in the code. If it is wrong, let's better find the error than just saying “Error appeared? Restart!”

I'm trying to figure it out but a bug in linesearch doesn't sound like a plausible explanation to me.

VerificationState sounds like something that would be nightmarishly difficult to implement. I believe such optional sanity checks could be useful in the future, even if they are off by default.

Ha, I've found an independent reproducer: #346 .

Oh I did not want to imply all linesearch is wrong but maybe your improvements had a slight flaw in one specific case. But great that you found an example and good that it even works with PT.
Then we could indeed keep the warning here and keep that (and the duct tape fix) until we fix the bug.

I think we discussed this enough in Zulip, and dressed this in tutorial mode. I hope I even added it here? We could add a tutorial debug for this if you feel that helps.

Hm, good that you remind me. I thought we already had this check merged in Manopt.jl? This is 100% not a tutorial-only issue because some combinations of line search/manifold/vector transport will lead to non-descent directions here which we need to catch (or the user gets a very hard to debug problem). Those combinations are still useful because they sometimes converge faster than "always descent direction" alternatives.

mateuszbaran · 2024-02-07T12:33:30Z

I'm trying to slowly wrap this up. How would you organize the hyperparameter tuner? It's a mix of somewhat generic code and example-specific code. ObjectiveData is technically generic but I'd imagine most people using it would have to tweak it anyway -- there is simply too many fine details to cover every possible use case through an API. Probably ManoptExamples.jl would be the right place, at least for now?

kellertuer · 2024-02-07T13:17:20Z

Hi,
I would first have to check what that function does, but will try to find time for that the next few days then, but yeah, ManoptExamples would probably ne a good place. Note that in there we also already have Rosenbrock.

kellertuer

This looks like a very great start to a thorough Benchmark.

My maybe most central remark or question is

How would we add this in a consistent way?
The benchmark_comparison.jl for now is a script;
should that become something we can run every now and then on a CI?
Should the results be part of the docs?
They could then be updated on a branch whenever the benchmark is run (either on CI and committed or when run manually).

then one could for example turn the current benchmark into a Quarto notebook.

Does the benchmark run several solvers and several examples? This could maybe be modularized

kellertuer · 2024-02-17T14:58:30Z

benchmarks/benchmark_comparison.jl

+    return storage
+end
+
+optimize(f_rosenbrock, g_rosenbrock!, [0.0, 0.0], LBFGS())


This is the Optim.jl run? One could mention that in a comment

Yes; this will be removed or commented when preparing final version.

kellertuer · 2024-02-17T14:59:55Z

benchmarks/benchmark_comparison.jl

+
+optimize(f_rosenbrock, g_rosenbrock!, [0.0, 0.0], LBFGS())
+
+function g_rosenbrock!(M::AbstractManifold, storage, x)


This could maybe be called gradient_Rosenbrock? Or is the Optim.jl convention to use g_ for the gradient? Then this could be mentioned here.

benchmarks/benchmark_comparison.jl

kellertuer · 2024-02-17T15:04:48Z

benchmarks/benchmark_comparison.jl

+
+    x0 = zeros(N)
+    x0[1] = 0
+    optim_state = optimize(f_rosenbrock, g_rosenbrock!, x0, method_optim, options_optim)


Similar to the previous all, this could also be one line just the optimize call whose return value is returned?

kellertuer · 2024-02-17T15:05:34Z

benchmarks/manoptuna.jl

+using PythonCall
+include("benchmark_comparison.jl")
+
+# This script requires optuna to be available through PythonCall


I have no experience with optuna, could this be documented a bit what the goal of this is?

kellertuer · 2024-02-17T15:06:48Z

benchmarks/manoptuna.jl

+
+
+"""
+mutable struct ObjectiveData{TObj,TGrad}


This is still undocumented and the functor below quite technical. Could we maybe document that a bit more to also make it understandable for either me or also you in some future time ;) ?

I will document it, I'm just not sure what style of describing to use here. I will add a tutorial-style commentary.

kellertuer · 2024-02-17T15:08:13Z

benchmarks/manoptuna.jl

+or too much pruning (if values here are too low)
+regenerate using `lbfgs_compute_pruning_losses`
+"""
+function lbfgs_study(; pruning_coeff::Float64=0.95)


This also looks quite technical, so.I am not so sure what it does and what its purpose in the end is?
Should this later be a tutorial?

kellertuer · 2024-02-17T15:10:13Z

Changelog.md

@@ -5,6 +5,12 @@ All notable Changes to the Julia package `Manopt.jl` will be documented in this
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

+## [0.4.53] unreleased


This number has to be adapted.

kellertuer · 2024-02-17T15:12:00Z

src/solvers/quasi_Newton.jl

@@ -345,6 +345,11 @@ function step_solver!(mp::AbstractManoptProblem, qns::QuasiNewtonState, iter)
    M = get_manifold(mp)
    get_gradient!(mp, qns.X, qns.p)
    qns.direction_update(qns.η, mp, qns)
+    if real(inner(M, qns.p, qns.η, qns.X)) > 0


I think we discussed this enough in Zulip, and dressed this in tutorial mode. I hope I even added it here? We could add a tutorial debug for this if you feel that helps.

kellertuer · 2024-02-17T15:16:09Z

If you would prefer having scripts that can be run easier (than Quarto Notebooks) we could also look into the new Quarto Scripts which I wanted to try on something anyways.

mateuszbaran · 2024-02-17T15:51:40Z

This looks like a very great start to a thorough Benchmark.

My maybe most central remark or question is

How would we add this in a consistent way? The benchmark_comparison.jl for now is a script; should that become something we can run every now and then on a CI? Should the results be part of the docs? They could then be updated on a branch whenever the benchmark is run (either on CI and committed or when run manually).

Currently I'm leaning towards turning it into a "how to choose the right solver (and its options) for your problem" tutorial. I'm not sure how (and for what problems) run it on CI. Note that the optimization script is quite demanding computationally despite all that advanced machinery.

Does the benchmark run several solvers and several examples? This could maybe be modularized

I've experimented with several examples but I haven't decided which one to use for an example. Most likely not Rosenbrock on sphere but I will decide when we select the right format.

If you would prefer having scripts that can be run easier (than Quarto Notebooks) we could also look into the new Quarto Scripts which I wanted to try on something anyways.

I really liked Quarto notebooks until updating Julia broke my settings and I could not get it back to work for a couple of hours (it still doesn't work, I just gave up; it makes the Jupyter notebook but the Julia kernel refuses to run). That might be the main problem with turning it into a tutorial.

kellertuer · 2024-02-17T16:33:37Z

I really liked Quarto notebooks until updating Julia broke my settings and I could not get it back to work for a couple of hours (it still doesn't work, I just gave up; it makes the Jupyter notebook but the Julia kernel refuses to run). That might be the main problem with turning it into a tutorial.

Remember to recompile IJulia. That is one of the main reasons I did so much Pkg.activate()... stuff in the documentation. If it happens that a new Julia version comes along, at least recompiling IJulia is crucial.

I would also be fine with these being benchmarks and a bit less tutorial focussed.

mateuszbaran · 2024-02-17T16:39:43Z

Remember to recompile IJulia. That is one of the main reasons I did so much Pkg.activate()... stuff in the documentation. If it happens that a new Julia version comes along, at least recompiling IJulia is crucial.

I did recompile IJulia. This script actually needs to run with the Conda.jl Python but I'm not sure how to run the notebook in its Jupyter. Or I did something wrong trying.

mateuszbaran · 2024-02-17T16:47:38Z

I just tried again, activating the Conda.jl environment. quarto render fails throwing "No module named yaml" despite me being able to import it when I run python REPL. Directly running the browser notebook complains about IJulia not being installed despite me just doing build IJulia a moment ago.

kellertuer · 2024-02-17T17:11:00Z

Oh Python depedencies – I nearly never manage to get that right. But from Julia with CondaPkg.jl (and their config file) I got it consistent.

mateuszbaran · 2024-03-01T12:15:22Z

I've moved the interesting parts of this PR to separate PRs so this one can be closed I think.

mateuszbaran · 2024-03-01T12:15:44Z

BTW, I tried to address some of your comments in the version submitted to ManoptExamples.

New benchmark

5512168

mateuszbaran added the WIP Work in Progress (for a pull request) label Dec 28, 2023

mateuszbaran added 5 commits December 28, 2023 15:29

use HZ line search

9d7e479

Fix LineSearchesExt bug

a23838c

minor updates

cecc2ca

changelog

a4bf77e

for testing

99cb471

kellertuer reviewed Dec 28, 2023

View reviewed changes

somewhat fix L-BFGS memory

4ef1dd7

mateuszbaran commented Dec 28, 2023

View reviewed changes

src/solvers/quasi_Newton.jl Outdated Show resolved Hide resolved

fix L-BFGS memory

7fcf166

mateuszbaran added 7 commits December 29, 2023 12:25

minor updates

9f26d83

fix test, minor benchmark updates

ee551c2

would this help?

d90fed0

add debug to ALM

3091c08

fix ALM; test on Julia 1.10

546be06

tweaking stopping criteria of ALM

5060c84

Rayleigh quotient benchmarking

0925a2b

mateuszbaran mentioned this pull request Jan 1, 2024

Fixes from benchmark #341

Merged

mateuszbaran added 5 commits January 1, 2024 16:53

Merge branch 'master' into mbaran/new-benchmark

7d7acea

bump version

2a55909

manoptuna

f7e1435

VT for manoptuna

6e334bc

some work on manoptuna

77893eb

not a descent direction?!?

377d242

mateuszbaran commented Jan 16, 2024

View reviewed changes

mateuszbaran added 11 commits January 17, 2024 12:40

Merge branch 'master' into mbaran/new-benchmark

a0f6396

fix test, add a warning

07529fa

Merge branch 'master' into mbaran/new-benchmark

1d8bb8e

use the new generalized gradient norm stopping criterion

4db49ce

Merge branch 'master' into mbaran/new-benchmark

649bd55

some work on manoptuna

02764d5

Merge branch 'master' into mbaran/new-benchmark

93528a4

generalize manoptuna tuner

96fb14f

selecting manifold using Manoptuna

fc9b060

suggesting float values, linesearch selection, conditional parameters

dbf0bb2

pick sigma for HZ

565a9fb

use simple linear combination of time and objective losses

92db97e

kellertuer reviewed Feb 17, 2024

View reviewed changes

This was referenced Mar 1, 2024

Hyperparameter optimization JuliaManifolds/ManoptExamples.jl#15

Merged

Checking for non-descent direction in qN #361

Merged

mateuszbaran closed this Mar 1, 2024

kellertuer deleted the mbaran/new-benchmark branch May 4, 2024 17:32


		optimize(f_rosenbrock, g_rosenbrock!, [0.0, 0.0], LBFGS())

		function g_rosenbrock!(M::AbstractManifold, storage, x)



		"""
		mutable struct ObjectiveData{TObj,TGrad}

New benchmark #339

New benchmark #339

Conversation

mateuszbaran commented Dec 28, 2023 • edited Loading

codecov bot commented Dec 28, 2023 • edited Loading

Codecov Report

kellertuer commented Dec 28, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mateuszbaran commented Dec 28, 2023

kellertuer commented Dec 28, 2023

mateuszbaran commented Dec 28, 2023

mateuszbaran commented Dec 30, 2023

kellertuer commented Jan 16, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mateuszbaran commented Feb 7, 2024

kellertuer commented Feb 7, 2024

kellertuer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kellertuer commented Feb 17, 2024

mateuszbaran commented Feb 17, 2024

kellertuer commented Feb 17, 2024

mateuszbaran commented Feb 17, 2024

mateuszbaran commented Feb 17, 2024

kellertuer commented Feb 17, 2024

mateuszbaran commented Mar 1, 2024

mateuszbaran commented Mar 1, 2024

mateuszbaran commented Dec 28, 2023 •

edited

Loading

codecov bot commented Dec 28, 2023 •

edited

Loading