adds Inequality #717

ValentinKaisermayer · 2022-08-27T15:07:03Z

follow up on #630

Indented use-case:

using ModelingToolkit
using Symbolics

@variables x y
@parameters a b
objective_func = (a - x)^2 + b * (y - x^2)^2
cons = [
    x^2 + y^2 ≲ 0.8
    0 ≲ x * y
    x * y ≲ 5 
]
sys = OptimizationSystem(objective_func, [x, y], [a, b], name = :sys, constraints = cons)

julia> cons
3-element Vector{Inequality}:
 x^2 + y^2 ≲ 0.8
 0 ≲ x*y
 x*y ≲ 5

codecov-commenter · 2022-08-29T18:20:19Z

Codecov Report

Merging #717 (8d3eb33) into master (ab7e1aa) will decrease coverage by 72.79%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master    #717       +/-   ##
==========================================
- Coverage   75.82%   3.03%   -72.80%     
==========================================
  Files          23      24        +1     
  Lines        2792    2737       -55     
==========================================
- Hits         2117      83     -2034     
- Misses        675    2654     +1979

Impacted Files	Coverage Δ
src/Symbolics.jl	`0.00% <ø> (-71.43%)`	⬇️
src/equations.jl	`0.00% <0.00%> (-51.03%)`	⬇️
src/inequality.jl	`0.00% <0.00%> (ø)`
src/groebner_basis.jl	`0.00% <0.00%> (-100.00%)`	⬇️
src/semipoly.jl	`0.00% <0.00%> (-91.31%)`	⬇️
src/linear_algebra.jl	`0.00% <0.00%> (-89.91%)`	⬇️
src/array-lib.jl	`0.00% <0.00%> (-80.56%)`	⬇️
src/diff.jl	`0.00% <0.00%> (-80.29%)`	⬇️
src/arrays.jl	`0.00% <0.00%> (-78.27%)`	⬇️
src/complex.jl	`0.00% <0.00%> (-75.00%)`	⬇️
... and 14 more

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

ValentinKaisermayer · 2022-08-29T19:34:53Z

Should we add a compound inequality in order to represent a <= x <= b?

shashi · 2022-08-31T04:12:32Z

Can't you use an & term?

ValentinKaisermayer · 2022-08-31T05:35:40Z

a <= x constructs an inequality. Would a <= x & x <= b construct simply two inequalities or would it be a separate type? It seems that the Optimization.jl interface expects this form.

BTW the & is implicit already. All constraints have to hold simultaneously.

shashi · 2022-09-02T17:25:11Z

Would a <= x & x <= b construct simply two inequalities or would it be a separate type?

If Inequality behaved like an istree (see https://symbolicutils.juliasymbolics.org/interface/) and is subtype of Symbolic then it should automatically construct a Term with & as the operation and the two inequalities as the arguments. Feel free to add a new type if that is too generic!

ValentinKaisermayer · 2022-09-02T19:08:23Z

I have followed the implementation of Equation. Is there any reason why it is not an isterm?

ValentinKaisermayer · 2022-09-03T08:04:55Z

I guess it would be somewhat important to be able to simplify inequalities into this form. Otherwise for e.g. Ipopt we would have double the amount of inequality constraints.

dpsanders · 2022-09-04T04:58:57Z

I believe that you can't write a <= x <= b due to how Julia parses this. I think there may be an issue about that?

ValentinKaisermayer · 2022-09-04T10:58:15Z

However, this could be handled via a the constructor, doesn't it?

<=(a,b) = Inequality(a, b, leq)
<=(a,b::Inequality) = IntervalInequality(a, b.lhs, b.rhs, leq)

dpsanders · 2022-09-10T22:03:42Z

Yes but only if you write a <= (b <= c) with parentheses.

The problem is with the way Julia parses a <= b <= c:

julia> Meta.@lower a <= b <= c
:($(Expr(:thunk, CodeInfo(
    @ none within `top-level scope`
1 ─ %1 = a <= b
└──      goto #3 if not %1
2 ─ %3 = b <= c
└──      return %3
3 ─      return false
))))

ValentinKaisermayer · 2022-09-14T19:44:49Z

@shashi If you are fine with the interface right now, can we merge this PR and leave a term interface or separate type as a follow-up?

shashi · 2022-09-16T18:12:19Z

Sure we can merge this :)

ValentinKaisermayer · 2022-10-09T14:39:01Z

should fix #740 as well

…entinKaisermayer/Symbolics.jl into vk-inequality

isaacsas · 2022-11-10T22:11:19Z

src/equations.jl

@@ -126,6 +126,10 @@ for T in [:Num, :Complex, :Number], S in [:Num, :Complex, :Number]
    end
 end

+canonical_form(eq::Equation) = eq.lhs - eq.rhs ~ 0
+
+get_variables(eq::Equation) = unique(vcat(get_variables(eq.lhs), get_variables(eq.rhs)))


@ValentinKaisermayer, @YingboMa, @shashi Just note this is a breaking change as previously get_variables only applied to the rhs. It also means get_variables is no longer consistent with get_variables!. Please be aware of this in making any new Symbolics releases. (It may not have any practical effect -- so far I haven't found anything that really relied on that behavior but I could have missed something in ModelingToolkit or Catalyst...)

I guess there has been a release already and this was included, so it's a bit late to make sure the release is flagged as breaking...

How can it be breaking if get_variables(eq::Equation) is added and not altered?

It was previously (intentionally) defined with a different behavior:

Symbolics.jl/src/utils.jl

Line 74 in 0026df9

function get_variables!(vars, e::Equation, varlist=nothing)

Symbolics.jl/src/utils.jl

Line 78 in 0026df9

get_variables(e, varlist=nothing) = get_variables!([], e, varlist)

The new dispatch changes this behavior.

I see. We can remove it, it is after all just for convenience and not used in MTK. Only get_variables!: https://github.com/SciML/ModelingToolkit.jl/blob/28b36c8f9932acfc6c54737c51437d35b192b0d7/src/systems/dependency_graphs.jl#L43
However, I must say, that without proper documentation it is not clear why only the rhs is considered. I guess it assumes 0~f(x) or D(x)~f(x), but just by convention.
https://symbolics.juliasymbolics.org/stable/manual/expression_manipulation/#Symbolics.get_variables

I think the change makes sense given ModelingToolkit is now allowing more flexibility in equation structure, but the in-place version should probably get updated for consistency. I agree the behavior should get documented in either case though!

bowenszhu · 2022-11-11T18:48:29Z

src/inequality.jl

+x - y ≲ 0
+```
+"""
+function ≲(lhs, rhs) 


Could you add a one-line description for how to type ≲? For example, Base.MathConstants.pi.

Symbolics.jl is not in SciML, but SciMLStyle doesn't allow Unicode to be used in public APIs.
See SciMLStyle: General Naming Principles.
So it is probably better to change the function name.

See #787

So it is probably better to change the function name.

Yeah, but to what?

adds Inequality

c9d9c8a

ChrisRackauckas requested a review from shashi August 27, 2022 16:06

shashi approved these changes Aug 29, 2022

View reviewed changes

ValentinKaisermayer mentioned this pull request Aug 31, 2022

Improves OptimizationSystem SciML/ModelingToolkit.jl#1787

Merged

adds test and support for array inequalities

482eeec

ValentinKaisermayer marked this pull request as ready for review August 31, 2022 18:36

ValentinKaisermayer and others added 2 commits September 1, 2022 15:27

Merge branch 'JuliaSymbolics:master' into vk-inequality

bfb62e6

fix

6cd222c

ValentinKaisermayer mentioned this pull request Sep 1, 2022

Adds support for inequalities SciML/ModelingToolkit.jl#1799

Merged

ValentinKaisermayer requested a review from shashi September 2, 2022 14:17

shashi approved these changes Sep 2, 2022

View reviewed changes

Update inequality.jl

b4e1a84

ValentinKaisermayer and others added 6 commits September 17, 2022 14:48

fixes test

4646970

Update inequality.jl

60614eb

fix scalarize

3d419ec

adds test

649de41

adds subs

d743f36

adds get_variables for Eqation and Inequality

49517ec

wrap in unique

8d3eb33

ValentinKaisermayer mentioned this pull request Oct 9, 2022

Loose ends from OptimizationSystem SciML/ModelingToolkit.jl#1786

Closed

6 tasks

Merge branches 'vk-inequality' and 'master' of https://github.com/Val…

2932afc

…entinKaisermayer/Symbolics.jl into vk-inequality

shashi merged commit 61bf8a9 into JuliaSymbolics:master Oct 10, 2022

ValentinKaisermayer deleted the vk-inequality branch October 11, 2022 11:17

isaacsas reviewed Nov 10, 2022

View reviewed changes

bowenszhu reviewed Nov 11, 2022

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

adds Inequality #717

adds Inequality #717

ValentinKaisermayer commented Aug 27, 2022

codecov-commenter commented Aug 29, 2022 •

edited

Loading

ValentinKaisermayer commented Aug 29, 2022

shashi commented Aug 31, 2022

ValentinKaisermayer commented Aug 31, 2022 •

edited

Loading

shashi commented Sep 2, 2022

ValentinKaisermayer commented Sep 2, 2022

ValentinKaisermayer commented Sep 3, 2022

dpsanders commented Sep 4, 2022

ValentinKaisermayer commented Sep 4, 2022

dpsanders commented Sep 10, 2022

ValentinKaisermayer commented Sep 14, 2022

shashi commented Sep 16, 2022

ValentinKaisermayer commented Oct 9, 2022

isaacsas Nov 10, 2022

isaacsas Nov 10, 2022

ValentinKaisermayer Nov 11, 2022

isaacsas Nov 11, 2022

ValentinKaisermayer Nov 11, 2022

isaacsas Nov 11, 2022

bowenszhu Nov 11, 2022

ValentinKaisermayer Nov 11, 2022

adds Inequality #717

adds Inequality #717

Conversation

ValentinKaisermayer commented Aug 27, 2022

codecov-commenter commented Aug 29, 2022 • edited Loading

Codecov Report

ValentinKaisermayer commented Aug 29, 2022

shashi commented Aug 31, 2022

ValentinKaisermayer commented Aug 31, 2022 • edited Loading

shashi commented Sep 2, 2022

ValentinKaisermayer commented Sep 2, 2022

ValentinKaisermayer commented Sep 3, 2022

dpsanders commented Sep 4, 2022

ValentinKaisermayer commented Sep 4, 2022

dpsanders commented Sep 10, 2022

ValentinKaisermayer commented Sep 14, 2022

shashi commented Sep 16, 2022

ValentinKaisermayer commented Oct 9, 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

codecov-commenter commented Aug 29, 2022 •

edited

Loading

ValentinKaisermayer commented Aug 31, 2022 •

edited

Loading