Allow to pass Optim.Options to bat_findmode

[Cornelius-G]

Allows to pass all the Optim.Options to bat_findmode.

Example:

using BAT
using Optim

posterior = BAT.example_posterior()

optalg = OptimAlg(;optalg = Optim.NelderMead(), options=Optim.Options(iterations=200))

bat_findmode(posterior, optalg)

[Cornelius-G]

Ah, I just realize this solution might be a problem since we declared OptimAlg part of the stable API 😖
Any ideas how to handle this?

[codecov]

Codecov Report

Attention: Patch coverage is 31.57895% with 26 lines in your changes are missing coverage. Please review.

Project coverage is 54.86%. Comparing base (7c3a123) to head (fb70273).
Report is 6 commits behind head on main.

❗ Current head fb70273 differs from pull request most recent head 86b07b4. Consider uploading reports for the commit 86b07b4 to get more accurate results

Files	Patch %	Lines
ext/BATOptimizationExt.jl	0.00%	26 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #423      +/-   ##
==========================================
- Coverage   55.03%   54.86%   -0.17%     
==========================================
  Files         116      117       +1     
  Lines        5624     5652      +28     
==========================================
+ Hits         3095     3101       +6     
- Misses       2529     2551      +22     

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

[oschulz]

How about algopts instead of just options, to be a bit more specific?

[oschulz]

Which options would the user set, typically (since BAT will also set some, we need to ensure they don't clash)?

[Cornelius-G]

Which options would the user set, typically (since BAT will also set some, we need to ensure they don't clash)?

I was mostly thinking about things like: Iterations and the values for determining convergence, like x_tol, f_tol etc.

[oschulz]

I was mostly thinking about things like: Iterations and the values for determining convergence, like x_tol, f_tol etc.

Hm, since those are pretty fundamental, maybe we should offer (some of those) as explicit fields of OptimAlg directly, so the user can use them across optimization backends in a consistent fashion?

[Cornelius-G]

Hm, since those are pretty fundamental, maybe we should offer (some of those) as explicit fields of OptimAlg directly, so the user can use them across optimization backends in a consistent fashion?

Hm, I'm not quite sure. It sounds a bit unnecessary to wrap all the options (and would again not give full control to the user).
I understand that we currently always require Optim.Options(store_trace = true, extended_trace=true), but we could just enforce this by setting these two options explicitly.

On the other hand, I just had a quick look at Optimization.jl. (I'm not sure, do we have any plans to support this in the future instead of only relying on Optim.jl?) But they seem to actually have a unified interface for common options like the iterations and tolerances: https://docs.sciml.ai/Optimization/stable/API/solve/#CommonSolve.solve-Tuple{OptimizationProblem,%20Any}

[oschulz]

I understand that we currently always require Optim.Options(store_trace = true, extended_trace=true), but we could just enforce this by setting these two options explicitly.

No, BAT will need control over these, for example for pathfinder (which requires a trace).

In principle, it should be easy for the user to switch between algorithms/backends, so options that most of them will offer we should provide in a generic fashion under common names.

[Cornelius-G]

No, BAT will need control over these, for example for pathfinder (which requires a trace).

Yeah, I just mean that in BAT we can overwrite the user input for these two options to always be true.

[Cornelius-G]

So I'm also fine to have these options as individual keywords. But feels a bit like reinventing what Optimization.jl is already offering.

[oschulz]

reinventing what Optimization.jl is already offering

To certain degree, yes. Optimization.jl used to be an extremely heavy dependency, but it's load time has gone down recently. I would still prefer not to tie BAT to a single optimization backend, even if it's a "meta-backend". But we should definitely add support for Optimization.jl as well. It may not be usable in all cases, though, at least in the past it was tricky to make it store gradient traces, which pathfinder will need.

[Cornelius-G]

Hi @oschulz, here is another proposal.
I now also added a small wrapper for using Optimization.jl with bat_findmode. (This wrapper is very basic and there is no deeper handling of the AD as is currently the case for the Optim.jl interface.)
But with this we can now see a bit better how to make the bat_findmode interface work for different optimizer packages.
The proposal is now that we have the keywords maxiters, maxtime, abstol, reltol as direct keyword arguments for the OptimAlg.
(These are the names used by Optimization.jl as the common options for all included optimization algs.)
In addition there is now the option to pass further kwargs which will just be passed to the optimizer and that can contain all the algorithm-specific options.
For the case of Optim.jl, we currently then just overwrite the (potential) user input for store_trace = true, extended_trace=true to ensure that BAT has access to the trace.

Example:

using BAT

posterior = BAT.example_posterior()

using Optim
alg = OptimAlg(;
    optalg = Optim.NelderMead(parameters=Optim.FixedParameters()),
    maxiters=200,
    kwargs = (f_calls_limit=100,),
)
my_mode = bat_findmode(posterior, alg)

using OptimizationOptimJL
alg = OptimizationAlg(; 
    optalg = OptimizationOptimJL.ParticleSwarm(n_particles=10), 
    maxiters=200, 
    kwargs=(f_calls_limit=50,)
)
my_mode = bat_findmode(posterior, alg)

We can discuss the naming of the algorithms and keywords. But I wanted to check first whether this is the right way at all.

[oschulz]

Sorry for the delay - that approach looks good to me!

[Cornelius-G]

Hi @oschulz, I just realized this PR is still open.
Do you have any more comments on this?

[oschulz]

Let's merge it. :-)

Can you just fix the Docs build failure?

[Cornelius-G]

Let's merge it. :-)

Can you just fix the Docs build failure?

Ok, Docs build is fixed. Should be ready to merge then 🙂

[oschulz]

Replaced by #439

[oschulz]

Closing in favor of #442