Handle constraints inside `modelingtoolkitize` of `OptimizationSystem` #1983

Vaibhavdixit02 · 2022-11-30T15:40:53Z

This will enable the AutoModelingToolkit in Optimization.jl to switch over to using modelingtoolkitize. SciML/Optimization.jl#432

src/systems/optimization/optimizationsystem.jl

ChrisRackauckas · 2022-11-30T15:43:13Z

Needs tests

ValentinKaisermayer · 2022-12-02T20:21:33Z

closes #1902

ValentinKaisermayer · 2022-12-08T07:26:19Z

src/systems/optimization/modelingtoolkitize.jl

+            end
+        end
+
+        if (isnothing(prob.lcons) || all(isinf.(prob.lcons))) && (isnothing(prob.ucons) || all(isinf.(prob.ucons)))


Why is this the behaviour for equality contraints? It seems wrong.

Why? Btw did you see line 33 as well?

I meant that -inf <= cons <= inf yields cons ~ 0

I see what you are saying, it might be better to no assume that if user is not passing equal constraints they don't intend it to be an equality constraint. I was assuming that case to be equivalent to having a equality to 0, it might not always be true

I think if a user supplies such a pair of (-inf,inf) it is either a modelling error or the intension is to deactivate the constraint. So in any case no constraint should be added. We might even cry. I wonder how JuMP handles such a case.

Added an error in this case

ValentinKaisermayer · 2022-12-08T07:31:49Z

src/systems/optimization/modelingtoolkitize.jl

-        if !isnothing(prob.lcons) && !isnothing(prob.ucons)
-            cons = prob.lcons .≲ lhs .≲ prob.ucons
-        else
+        if !isnothing(prob.lcons)


I think this can be refactored, since ucons and lcons are either both nothing or both not.

That holds true for ub and lb but not for these. We could change it in SciMLBase, but I have left it as it is for correct behavior now and will make the change there subsequently.

ValentinKaisermayer · 2022-12-08T07:33:53Z

src/systems/optimization/modelingtoolkitize.jl

+        if !isnothing(prob.ucons)
+            for i in 1:num_cons
+                if !isinf(prob.ucons[i])
+                    prob.lcons[i] !== prob.ucons[i] ? push!(cons, lhs[i] ~ prob.ucons[i]) : push!(cons, lhs[i] ≲ prob.ucons[i])


Isn't it the other way around? If ucons and lcons are equal, an equality has to be added.

Oh shit. Yes it's flipped, thanks for catching that, how did this not get caught by the tests though I'll check that

ValentinKaisermayer · 2022-12-08T11:01:38Z

src/systems/optimization/modelingtoolkitize.jl

+        cons = []
+    end
+
+    de = OptimizationSystem(eqs, vec(vars), vec(params);


I am not sure I understood what you meant here

You are generating the parameters via variable(:α, i)

ValentinKaisermayer · 2022-12-08T11:03:15Z

src/systems/optimization/modelingtoolkitize.jl

+
+Generate `OptimizationSystem`, dependent variables, and parameters from an `OptimizationProblem`.
+"""
+function modelingtoolkitize(prob::DiffEqBase.OptimizationProblem; num_cons = 0, kwargs...)


Isn't num_cons simply isnothing(prob.lcons) ? 0 : length(prob.lcons) ?

…cation

src/systems/optimization/modelingtoolkitize.jl

Vaibhavdixit02 · 2023-01-11T12:48:41Z

Should be ready now

ChrisRackauckas · 2023-01-11T15:16:06Z

src/systems/optimization/modelingtoolkitize.jl

+        p = prob.p
+    end
+
+    vars = reshape([variable(:x, i) for i in eachindex(prob.u0)], size(prob.u0))


This should be a restructure. Use the ODE one as a template, i.e. https://github.com/SciML/ModelingToolkit.jl/pull/1983/files#diff-5c721fb74d2a713d8da5efa082f9b6927958cbedf859232a859c252869193196R20-R21

ChrisRackauckas · 2023-01-11T17:06:37Z

test/optimizationsystem.jl

    generate_hessian(combinedsys)
    hess_sparsity = ModelingToolkit.hessian_sparsity(sys1)
-    sparse_prob = OptimizationProblem(sys1, [x, y], [a, b], grad = true, sparse = true)
+    sparse_prob = OptimizationProblem(sys1, [x, y], [a, b], grad = true, obj_sparse = true)


what's obj_sparse and how is it different from sparse?

In this PR I have removed sparse in favor of obj_sparse (objective's sparse) and cons_sparse (constraints' sparse). This is similar to the AutoModelingToolkit struct. A single kwarg is not sufficient since either could be sparse

I'd keep obj_sparse as sparse. Consistency matters.

We can keep it and deprecate and interpret it as obj_sparse, but also keep these two for consistency with Optimization.jl?

No, not deprecate it. Why would an ODE use obj_sparse instead of sparse?

Handle constraints inside modelingtoolkitize of OptimizationSystem

1342a50

ChrisRackauckas reviewed Nov 30, 2022

View reviewed changes

src/systems/optimization/optimizationsystem.jl Outdated Show resolved Hide resolved

Handle constraint bounds and add test

4b10b72

ValentinKaisermayer reviewed Dec 8, 2022

View reviewed changes

Vaibhavdixit02 mentioned this pull request Jan 8, 2023

Add an FAQ page SciML/Optimization.jl#451

Merged

Address review comments and substitute in inequalities while simplifi…

4f5349d

…cation

Vaibhavdixit02 force-pushed the osmtkizecons branch from de3bde9 to 4f5349d Compare January 10, 2023 09:21

Use obj_sparse kwarg and remove num_cons in tests

2f2e9c4

Vaibhavdixit02 commented Jan 11, 2023

View reviewed changes

src/systems/optimization/modelingtoolkitize.jl Outdated Show resolved Hide resolved

Update src/systems/optimization/modelingtoolkitize.jl

0d1ce4f

ChrisRackauckas reviewed Jan 11, 2023

View reviewed changes

ChrisRackauckas approved these changes Jan 11, 2023

View reviewed changes

Use restructure instead of reshape

3c4d820

Vaibhavdixit02 force-pushed the osmtkizecons branch from b87e974 to 3c4d820 Compare January 11, 2023 16:33

ChrisRackauckas reviewed Jan 11, 2023

View reviewed changes

Keep sparse

a501bcf

ChrisRackauckas merged commit 5659fae into master Jan 13, 2023

ChrisRackauckas deleted the osmtkizecons branch January 13, 2023 19:13

Uh oh!

Handle constraints inside modelingtoolkitize of OptimizationSystem #1983

Handle constraints inside modelingtoolkitize of OptimizationSystem #1983

Uh oh!

Conversation

Vaibhavdixit02 commented Nov 30, 2022

Uh oh!

Uh oh!

ChrisRackauckas commented Nov 30, 2022

Uh oh!

ValentinKaisermayer commented Dec 2, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Vaibhavdixit02 commented Jan 11, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Handle constraints inside `modelingtoolkitize` of `OptimizationSystem` #1983

Handle constraints inside `modelingtoolkitize` of `OptimizationSystem` #1983