Splits MOI solve into init and solve #400

ValentinKaisermayer · 2022-10-20T13:55:11Z

using ModelingToolkit
import Optimization
import OptimizationMOI
import Ipopt

@variables x[1:2] = [0.0, 0.0] [bounds = (0.0, Inf)]
@parameters a = 1.0
@parameters b = 1.0
@named sys = OptimizationSystem(
    x[1] + x[2], [x...], [a, b]; 
    constraints=[1.0 ≲ a * x[1]^2 * b + x[2]^2, x[1]^2 + x[2]^2 ≲ 2.0]
)
prob = OptimizationProblem(sys, [x[1] => 2.0, x[2] => 0.0], []; grad=true, hess=true)
cache = Optimization.init(prob)
cache = OptimizationMOI.set_p(cache, [1.0, 100.0])
Optimization.solve!(cache, Ipopt.Optimizer(); print_level=0)

ValentinKaisermayer · 2022-10-20T13:56:27Z

lib/OptimizationMOI/src/OptimizationMOI.jl

    J::JT
    H::HT
    cons_H::Vector{CHT}
    lcons::Vector{T}
    ucons::Vector{T}
+    sense::S
+    prob::PR # TODO: this is dangerous since the cache and problem might get out of sync after reinit


I think it is not good to have the problem in here but it is needed for the construction of the solution.

Let's store the cache in the solution object as you suggested below, to make full use of the memory efficiency the abstraction offers? (again this would have to be done in SciMLBase)

ValentinKaisermayer · 2022-10-20T13:57:55Z

lib/OptimizationMOI/src/OptimizationMOI.jl

@@ -336,12 +372,19 @@ function SciMLBase.__solve(prob::OptimizationProblem,
        minimum = NaN
        opt_ret = :Default
    end
-    return SciMLBase.build_solution(prob,
+    return SciMLBase.build_solution(cache.prob,


I think a specific interface for optimization solutions is needed. From the problem only p and observed will be used at any point, I think.

What would be even better is to avoid all those nested field access in the solution interfaces, like A.prob.p and use something like get_p(A), so that anything that implements this interface can be used, e.g. a cache.

codecov · 2022-10-20T14:08:05Z

Codecov Report

Merging #400 (9b669f7) into master (11f16bc) will decrease coverage by 0.37%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #400      +/-   ##
==========================================
- Coverage   82.02%   81.64%   -0.38%     
==========================================
  Files           9        9              
  Lines         267      267              
==========================================
- Hits          219      218       -1     
- Misses         48       49       +1

Impacted Files	Coverage Δ
src/function/finitediff.jl	`90.69% <0.00%> (-2.33%)`	⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

ValentinKaisermayer · 2022-10-20T14:30:21Z

#352

Vaibhavdixit02 · 2022-10-21T12:55:44Z

lib/OptimizationMOI/src/OptimizationMOI.jl

-struct MOIOptimizationProblem{T, F <: OptimizationFunction, uType, P,
-                              JT <: DenseOrSparse{T}, HT <: DenseOrSparse{T},
-                              CHT <: DenseOrSparse{T}} <:
+struct MOIOptimizationCache{T, F <: OptimizationFunction, uType, P, LB, UB, I,


I think this can be a more general, OptimizationCache right? (and moved to SciMLBase) and then, exactly like how you have done here, we'd define init for it for each backend.

What would such an OptimizationCache hold? Right now it is kind of big. It holds the problem, which holds the function which hold the system. Convinient but a nightmare in terms of interface.

I think all the fields here, except remove the prob field for OptimizationProblem from this (since as I commented below we can also make the build_solution work directly with OptimizationCache then as you had suggested)

I think this can be a more general, OptimizationCache right? (and moved to SciMLBase) and then, exactly like how you have done here, we'd define init for it for each backend.

Why not stick the OptimizationCache into Optimization.jl?

@ChrisRackauckas pointed out that a per-solver cache maybe needed in #352. I'm not quite sure yet.

Since reinit(cache; p) or something like set_p(cache, p) is the only real reason for a cache, it has to be possible to change the parameters. All nonlinear sovers use prob.f.f and prob.f.cons which simply take p as an argument - easy! But what if the actual box constraints bounds are defined via parameters?
I would like to implement an interface for linear solvers as well. The only way I see to implement this, is directly via the objective and constraints expressions and using Symbolics.jl.

For now the OptimizationSolution does not support access of parameters, however it would be nice to provide this. Than we need paramsyms.

For the MOI case I think you added the expr and cons_expr to the OptimizationFunction, at least if the parameters are not replaced it might be usefull for the linear solver interface.

Why not stick the OptimizationCache into Optimization.jl?

Because afaik we keep all such structs in SciMLBase, so it avoids any recursive dependencies for these. For example in https://github.com/SciML/SciMLBase.jl/pull/286/files if instead of C you want to use OptimizationCache then it would have to be there since SciMLBase can't take Optimization as a dependency.

@ChrisRackauckas pointed out that a per-solver cache may be needed in #352. I'm not quite sure yet.

Yeah me too, from what I can see, OptimizationCache seems like a union of OptimizationFunction and OptimizationProblem, such that it can store the complete state of the problem, hence it would end up needing all of the possible fields and we just initialize everything to nothing and fill it up as relevant for the backend.

Since reinit(cache; p) or something like set_p(cache, p) is the only real reason for a cache, it has to be possible to change the parameters. All nonlinear sovers use prob.f.f and prob.f.cons which simply take p as an argument - easy! But what if the actual box constraints bounds are defined via parameters?

Sorry, not sure I understood this. Why would that not be possible? 🤔

Regarding the expressions, yeah that would go into it as well, and replace any constant values interpolated in there if needed (but this can be punted for later depending on how much work it ends up being)

Because afaik we keep all such structs in SciMLBase, so it avoids any recursive dependencies for these.

LinearCache would be a counterexample but i can see that it might be beneficial.

If ub or lb are defined in terms of parameters of an MTK model, only the system knows how to compute this.

@parameters c @variables x [bounds = (c, Inf)]

It is stored in the bounds metadata of each variable. But accessing the metadata is only possible easily via MTK. We can of course forbid this in MTK.

Vaibhavdixit02 · 2022-10-21T13:41:13Z

lib/OptimizationMOI/src/OptimizationMOI.jl

    J::JT
    H::HT
    cons_H::Vector{CHT}
    lcons::Vector{T}
    ucons::Vector{T}
+    sense::S
+    prob::PR # TODO: this is dangerous since the cache and problem might get out of sync after reinit


Let's store the cache in the solution object as you suggested below, to make full use of the memory efficiency the abstraction offers? (again this would have to be done in SciMLBase)

Vaibhavdixit02 · 2022-10-21T13:45:24Z

This PR looks good, but I got a feeling that a lot of it can be just copy pasted into SciMLBase and then used here. I would be okay with merging it here and taking up the move if that's what you'd prefer @ValentinKaisermayer. But I think it might be more impactful to move the changes into SciMLBase and then follow up here?

lib/OptimizationMOI/src/OptimizationMOI.jl

ValentinKaisermayer · 2022-10-22T16:50:41Z

closing this in favor of #381

split into init and solve

4f64316

ValentinKaisermayer commented Oct 20, 2022

View reviewed changes

ValentinKaisermayer added 2 commits October 20, 2022 16:05

rename

4ef028a

formatting

5cda7fc

changes method definition

9b669f7

ValentinKaisermayer marked this pull request as ready for review October 20, 2022 15:10

ValentinKaisermayer mentioned this pull request Oct 20, 2022

Adds support for MOI.AbstractOptimizer that do not support MOI.NLPBlock #381

Merged

Vaibhavdixit02 reviewed Oct 21, 2022

View reviewed changes

ChrisRackauckas reviewed Oct 22, 2022

View reviewed changes

lib/OptimizationMOI/src/OptimizationMOI.jl Show resolved Hide resolved

ChrisRackauckas reviewed Oct 22, 2022

View reviewed changes

lib/OptimizationMOI/src/OptimizationMOI.jl Show resolved Hide resolved

ValentinKaisermayer closed this Oct 22, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Splits MOI solve into init and solve #400

Splits MOI solve into init and solve #400

ValentinKaisermayer commented Oct 20, 2022 •

edited

Loading

ValentinKaisermayer Oct 20, 2022

Vaibhavdixit02 Oct 21, 2022

ValentinKaisermayer Oct 20, 2022

ValentinKaisermayer Oct 20, 2022

codecov bot commented Oct 20, 2022 •

edited

Loading

ValentinKaisermayer commented Oct 20, 2022

Vaibhavdixit02 Oct 21, 2022

ValentinKaisermayer Oct 21, 2022

Vaibhavdixit02 Oct 21, 2022

ValentinKaisermayer Oct 21, 2022

ValentinKaisermayer Oct 21, 2022

Vaibhavdixit02 Oct 21, 2022

ValentinKaisermayer Oct 21, 2022

Vaibhavdixit02 Oct 21, 2022

Vaibhavdixit02 commented Oct 21, 2022

ValentinKaisermayer commented Oct 22, 2022

Splits MOI solve into init and solve #400

Splits MOI solve into init and solve #400

Conversation

ValentinKaisermayer commented Oct 20, 2022 • edited Loading

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

codecov bot commented Oct 20, 2022 • edited Loading

Codecov Report

ValentinKaisermayer commented Oct 20, 2022

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

Vaibhavdixit02 commented Oct 21, 2022

ValentinKaisermayer commented Oct 22, 2022

ValentinKaisermayer commented Oct 20, 2022 •

edited

Loading

codecov bot commented Oct 20, 2022 •

edited

Loading