Add extension points for more generic `@constraint` syntax #2051

dourouc05 · 2019-08-29T15:26:24Z

Prototype for #2014. Comments are welcome! This PR is mostly to enable more things from extensions.

src/macros.jl

dourouc05 · 2019-09-07T02:52:06Z

@blegat I did as you suggested (at least, I hope so…), which yielded a simpler implementation. Not all tests still pass, but quite a few of them do (issues with bridging).

dourouc05 · 2019-09-08T00:25:08Z

Most of the tests pass now, except one for the @constraint(model, [x == 1-x, 2x == 3]) syntax, but I've been unable to understand why it worked previously…

So, for now, this PR allows to write JuMP extensions to allow global constraints like alldifferent and reification (binary variable indicating if a condition is met). I think the implementation is rather clean in its principles, only added overloads to parse_constraint (and changing its interface to allow much finer dispatch). What's missing right now for full constraint-programming modelling: logical operator parsing (including implications), allowing operators almost everywhere (with constraints like x >= min{y, z*z} or having IloSizeOf with CPLEX), maybe allow other types of variables (again, with CPLEX, you can have intervals as variables).

src/macros.jl

Wikunia · 2019-12-20T01:59:52Z

As I'll not be able to attend the developers call and currently can't check this as I'm on my phone: is it already possible to write a not equal constraint with this/JuMP?

blegat · 2020-01-02T15:27:18Z

@dourouc05 In #2132, I was able to make @constraint(model, complements(..., ...)) work without needing to modify the parse_constraint API, couldn't this also work for your use cases ?

dourouc05 · 2020-02-10T14:51:07Z

@blegat thanks for your ping. However, this case is slightly different: for many constraints, you have notions similar to a left-hand and a right-hand sides.

for complementarity constraints, F(x) ⟂ x
for indicator constraints, condition ⇒ { constraints }

Here, there is just one set of expressions, not two.

An easy modification would be to force the user to just have alldifferent([x, y]), with a vector of expressions as an argument. This way, I think that it would be sufficient to have a definition like this to extend the parse_constraint API:

function parse_constraint(_error::Function, sense::Symbol, F)
    (sense, vectorized) = _check_vectorized(sense)
    vectorized, parse_one_operator_constraint(_error, vectorized, Val(sense), F)...
end

I made a new patch to still allow syntax like alldifferent(x, y). I did not see any other way than to slightly modify _constraint_macro to allow passing all the arguments as one array. This is much cleaner than previously, at least!

Here are some MOI sets that would be needed in complement to this PR: dourouc05/MathOptInterface.jl@df85074

codecov · 2020-02-10T17:17:59Z

Codecov Report

Merging #2051 into master will increase coverage by 0.13%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #2051      +/-   ##
==========================================
+ Coverage   91.16%   91.30%   +0.13%     
==========================================
  Files          42       42              
  Lines        4165     4186      +21     
==========================================
+ Hits         3797     3822      +25     
+ Misses        368      364       -4

Impacted Files	Coverage Δ
src/macros.jl	`92.79% <100.00%> (+0.26%)`	⬆️
src/Containers/SparseAxisArray.jl	`78.57% <0.00%> (-1.95%)`	⬇️
src/nlp.jl	`92.82% <0.00%> (+0.55%)`	⬆️
src/Containers/DenseAxisArray.jl	`84.46% <0.00%> (+1.13%)`	⬆️
src/Containers/Containers.jl	`88.88% <0.00%> (+38.88%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a72e7ac...228618a. Read the comment docs.

src/macros.jl

jump-dev/JuMP.jl#2051

dourouc05 · 2020-02-24T17:58:08Z

@blegat I made your solution work, it is in the current version of this PR. I'm not sure that parse_call_constraint is strictly necessary, it would be enough to have a ::Val{:call} on all these methods, but I find this solution cleaner.

With this, I have the majority of the extension points I need, with very little code in JuMP. It would be nice to allow a syntax like @constraint(m, x == count(y, 1)) or @constraint(m, x == count(y .== 1)), a much more natural syntax, but it would require deeper changes. I'm having a look at it.

blegat · 2020-02-25T08:33:45Z

src/macros.jl

+end
+
+function parse_constraint(_error::Function, ::Val{:call}, args...)
+    return parse_call_constraint(_error, Val(args[1]), args[2:end]...)


I would rather have this call parse_constraint(_error, args...) to avoid breaking existing code

How can you then dispatch on the function being called (i.e. args[1])?

You can just keep the existing parse_constraint(_error::Function, sense::Symbol, lhs, rhs), the call will be dispatched to it, isn't it ?

Sorry, I skipped your comment… I don't see how Julia could dispatch on a symbol without Val. Or would you rather have this function called parse_constraint? Wouldn't this be enough to preserve existing code?

It is dispatched in parse_one_operator_constraint as it is currently.

Is the new version what you meant?

(The tests don't pass locally with this commit, although all errors are related to MutableArithmetics.jl… I guess this is alright?)

dourouc05 · 2020-04-24T03:05:52Z

Actually, I'm starting to get lost in the implications of various parts of this PR… For now, I think that #2228 and #2229 would cover all this original PR was about.

The next step would be to allow to rewrite more expressions, to allow things like @constraint(m, x == y[z]) where at least one of y and z is a JuMP variable (either an integer index or a vector). However, to do this, you need information which does not seem to be available to a macro… (Maybe with generated functions? They work like macros, but have dispatch on the type of the arguments.)

@blegat

Add support for reification. Things moved to https://github.com/dourouc05/JuCP.jl Update macros.jl Pass the same type of argument to the different calls to parsefun. Restore two definitions of parse_constraint to avoid ambiguity. LoadError: LoadError: MethodError: parse_constraint(::JuMP.var"#_error#70"{Symbol}, ::Symbol, ::Symbol, ::Expr, ::Symbol, ::Symbol) is ambiguous. Candidates: parse_constraint(_error::Function, sense::Symbol, args...) in JuMP at C:\Users\Thibaut\.julia\dev\JuMP\src\macros.jl:180 parse_constraint(_error::Function, lb, lsign::Symbol, aff, rsign::Symbol, ub) in JuMP at C:\Users\Thibaut\.julia\dev\JuMP\src\macros.jl:205 Implement more closely @blegat's suggestion. Also adapt build_constraint. Also adapt indicator constraints. Also adapt SD constraint. For :call constraint, dispatch on the first argument. Clean up. Make tests pass. Useless extension point. Allow rewriting the rhs of a comparison constraint. Bug fixing (due to MA?). Don't break user code. If the constraint looks like `== f(x)`, whatever f or x, the previous code would always think the function f must be known by JuMP. Reduce diff size. @blegat's comments. Simplify PR wrt jump-dev#2228. Simplify PR wrt jump-dev#2228. Clean PR wrt jump-dev#2229.

dourouc05 · 2020-05-05T03:28:14Z

I've just rewritten the PR to bring back the functionality of having constraints with just a function (i.e. point 2 for @blegat's message), now with a test case.

dourouc05 · 2020-05-05T03:34:14Z

All PRs for new syntax (this one and #2229) have been merged in: https://github.com/dourouc05/JuMP.jl/tree/constraints

blegat · 2020-05-05T12:52:18Z

test/macros.jl

@@ -176,6 +176,20 @@ function custom_expression_test(ModelType::Type{<:JuMP.AbstractModel})
    end
 end

+function JuMP.parse_constraint_call(_error::Function, ::Val{:f}, x)


Why not JuMP.parse_constraint_head(_error::Function, ::Val{:call}, ::Val{:f}, x) ?

If I remove my changes in src/macros.jl and adapt the test as you suggest, this is what I get:

ERROR: LoadError: LoadError: LoadError: In `@constraint(model, con_ref, f(x))`: Constraints must be in one of the following forms: expr1 <= expr2 expr1 >= expr2 expr1 == expr2 lb <= expr <= ub Stacktrace: [1] error(::String, ::String) at .\error.jl:42 [2] _macro_error(::Symbol, ::Array{Any,1}, ::String) at C:\Users\Thibaut\.julia\dev\JuMP\src\macros.jl:891 [3] (::JuMP.var"#_error#68"{Symbol})(::String) at C:\Users\Thibaut\.julia\dev\JuMP\src\macros.jl:363 [4] _unknown_constraint_expr(::JuMP.var"#_error#68"{Symbol}) at C:\Users\Thibaut\.julia\dev\JuMP\src\macros.jl:240 [5] parse_constraint(::Function, ::Symbol, ::Symbol) at C:\Users\Thibaut\.julia\dev\JuMP\src\macros.jl:249 [6] parse_constraint_head(::Function, ::Val{:call}, ::Symbol, ::Symbol) at C:\Users\Thibaut\.julia\dev\JuMP\src\macros.jl:184 [7] parse_constraint_expr at C:\Users\Thibaut\.julia\dev\JuMP\src\macros.jl:181 [inlined] [8] _constraint_macro(::Tuple{Symbol,Symbol,Expr}, ::Symbol, ::typeof(parse_constraint_expr)) at C:\Users\Thibaut\.julia\dev\JuMP\src\macros.jl:404 [9] @constraint(::LineNumberNode, ::Module, ::Vararg{Any,N} where N) at C:\Users\Thibaut\.julia\dev\JuMP\src\macros.jl:487 [10] include(::String) at .\client.jl:439 [11] top-level scope at C:\Users\Thibaut\.julia\dev\JuMP\test\runtests.jl:23 [12] include(::String) at .\client.jl:439 [13] top-level scope at none:6 in expression starting at C:\Users\Thibaut\.julia\dev\JuMP\test\macros.jl:186 in expression starting at C:\Users\Thibaut\.julia\dev\JuMP\test\macros.jl:182 in expression starting at C:\Users\Thibaut\.julia\dev\JuMP\test\runtests.jl:23

blegat

What do you think of the second suggestion in #2236 ?
With this you can add the test with the modification I suggested and we don't need to modify src/macros.jl (BTW what you are suggesting is a breaking change).
I would suggest using parse_constraint_head in JuCP, you will just have to rename it to parse_constraint once we make the breaking change but that shouldn't break JuCP if you specifiy JuMP = "0.21" in your Project.toml.

src/macros.jl

blegat · 2020-05-07T07:17:52Z

src/macros.jl

+    return parse_constraint(_error, Val(args[1]), args[2:end]...)
+end
+
+function parse_constraint(_error::Function, sense::Val, F...)


Why are these two methods needed ?

Indeed, they are not needed, I've just removed them.

src/macros.jl

test/macros.jl

blegat · 2020-05-11T16:09:57Z

test/macros.jl

+        @test jump_function(con) == x
+        @test moi_set(con) isa CustomSet
+
+        @test_macro_throws LoadError @constraint(model, g(x))


Replace LoadError by ErrorException. This is why travis is failing.

I didn't expect the macro to be so smart! Done in the next commit.

(By the way, why isn't it upstreamed? I guess many Julia projects could benefit from it!)

blegat reviewed Aug 29, 2019

View reviewed changes

src/macros.jl Outdated Show resolved Hide resolved

blegat reviewed Aug 29, 2019

View reviewed changes

src/macros.jl Outdated Show resolved Hide resolved

dourouc05 force-pushed the dourouc05-constraint-macro branch from f994a60 to c3099f9 Compare September 7, 2019 02:48

dourouc05 force-pushed the dourouc05-constraint-macro branch from 9e31bc9 to 9fc76c4 Compare September 7, 2019 14:41

dourouc05 changed the title ~~[WIP] Allow @constraint without relational operator~~ [WIP] Allow constraint programming within JuMP Sep 8, 2019

blegat reviewed Sep 15, 2019

View reviewed changes

src/macros.jl Outdated Show resolved Hide resolved

blegat reviewed Sep 15, 2019

View reviewed changes

src/macros.jl Outdated Show resolved Hide resolved

blegat reviewed Sep 15, 2019

View reviewed changes

src/macros.jl Outdated Show resolved Hide resolved

blegat reviewed Sep 15, 2019

View reviewed changes

src/macros.jl Outdated Show resolved Hide resolved

blegat reviewed Sep 15, 2019

View reviewed changes

src/macros.jl Outdated Show resolved Hide resolved

blegat reviewed Sep 24, 2019

View reviewed changes

src/macros.jl Outdated Show resolved Hide resolved

blegat reviewed Sep 24, 2019

View reviewed changes

src/macros.jl Outdated Show resolved Hide resolved

blegat reviewed Sep 24, 2019

View reviewed changes

src/macros.jl Outdated Show resolved Hide resolved

odow mentioned this pull request Oct 3, 2019

Complementarity constraint jump-dev/MathOptInterface.jl#771

Closed

dourouc05 force-pushed the dourouc05-constraint-macro branch 2 times, most recently from 054067d to 29a47c2 Compare February 10, 2020 15:16

dourouc05 mentioned this pull request Feb 11, 2020

Typo in sets jump-dev/MathOptInterface.jl#1024

Merged

blegat reviewed Feb 13, 2020

View reviewed changes

src/macros.jl Outdated Show resolved Hide resolved

dourouc05 added a commit to JuliaConstraints/JuCP.jl that referenced this pull request Feb 13, 2020

Move things from PR.

84c5ee0

jump-dev/JuMP.jl#2051

dourouc05 force-pushed the dourouc05-constraint-macro branch from 931340a to 281cdf7 Compare February 19, 2020 17:15

blegat reviewed Feb 25, 2020

View reviewed changes

dourouc05 changed the title ~~[WIP] Allow constraint programming within JuMP~~ Allow constraint programming within JuMP Mar 30, 2020

dourouc05 mentioned this pull request Mar 30, 2020

Feature Request: Reified constraint #2215

Closed

dourouc05 force-pushed the dourouc05-constraint-macro branch 2 times, most recently from e481b15 to dc77f32 Compare April 24, 2020 02:49

dourouc05 force-pushed the dourouc05-constraint-macro branch 3 times, most recently from 9fd9877 to 597a02c Compare May 5, 2020 01:34

dourouc05 force-pushed the dourouc05-constraint-macro branch from 597a02c to 1e07538 Compare May 5, 2020 01:35

Rewrite the dispatch code, add a test.

cc54d89

blegat reviewed May 5, 2020

View reviewed changes

As @blegat suggested.

763eddb

blegat reviewed May 6, 2020

View reviewed changes

src/macros.jl Outdated Show resolved Hide resolved

As @blegat suggested.

67d8fc0

blegat reviewed May 7, 2020

View reviewed changes

src/macros.jl Show resolved Hide resolved

dourouc05 added 3 commits May 7, 2020 16:36

Add error as suggested by @blegat.

97bbe22

Remove useless definition.

61e69bc

Remove useless function.

481f596

blegat reviewed May 11, 2020

View reviewed changes

test/macros.jl Outdated Show resolved Hide resolved

Use @test_macro_throws as suggested by @blegat

155c577

blegat reviewed May 11, 2020

View reviewed changes

Update macros.jl

228618a

blegat approved these changes May 12, 2020

View reviewed changes

blegat added this to the 0.21.3 milestone May 12, 2020

blegat merged commit 13032eb into jump-dev:master May 12, 2020

dourouc05 mentioned this pull request May 12, 2020

Add rewrite_call_expression through _rewrite_expr. #2241

Closed

dourouc05 mentioned this pull request Jun 4, 2020

parse_one_operator_constraint: clash with parse_ternary_constraint when there are four arguments #2246

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add extension points for more generic `@constraint` syntax #2051

Add extension points for more generic `@constraint` syntax #2051

dourouc05 commented Aug 29, 2019

dourouc05 commented Sep 7, 2019

dourouc05 commented Sep 8, 2019

Wikunia commented Dec 20, 2019

blegat commented Jan 2, 2020 •

edited

Loading

dourouc05 commented Feb 10, 2020 •

edited

Loading

codecov bot commented Feb 10, 2020 •

edited

Loading

dourouc05 commented Feb 24, 2020

blegat Feb 25, 2020

dourouc05 Feb 26, 2020

blegat Feb 26, 2020

dourouc05 Mar 26, 2020 •

edited

Loading

blegat Mar 26, 2020

dourouc05 Mar 26, 2020 •

edited

Loading

dourouc05 commented Apr 24, 2020

dourouc05 commented May 5, 2020

dourouc05 commented May 5, 2020

blegat May 5, 2020

dourouc05 May 5, 2020

blegat left a comment

blegat May 7, 2020

dourouc05 May 7, 2020

blegat May 11, 2020

dourouc05 May 11, 2020 •

edited

Loading

Add extension points for more generic @constraint syntax #2051

Add extension points for more generic @constraint syntax #2051

Conversation

dourouc05 commented Aug 29, 2019

dourouc05 commented Sep 7, 2019

dourouc05 commented Sep 8, 2019

Wikunia commented Dec 20, 2019

blegat commented Jan 2, 2020 • edited Loading

dourouc05 commented Feb 10, 2020 • edited Loading

codecov bot commented Feb 10, 2020 • edited Loading

Codecov Report

dourouc05 commented Feb 24, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dourouc05 Mar 26, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dourouc05 Mar 26, 2020 • edited Loading

Choose a reason for hiding this comment

dourouc05 commented Apr 24, 2020

dourouc05 commented May 5, 2020

dourouc05 commented May 5, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

blegat 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

dourouc05 May 11, 2020 • edited Loading

Choose a reason for hiding this comment

Add extension points for more generic `@constraint` syntax #2051

Add extension points for more generic `@constraint` syntax #2051

blegat commented Jan 2, 2020 •

edited

Loading

dourouc05 commented Feb 10, 2020 •

edited

Loading

codecov bot commented Feb 10, 2020 •

edited

Loading

dourouc05 Mar 26, 2020 •

edited

Loading

dourouc05 Mar 26, 2020 •

edited

Loading

dourouc05 May 11, 2020 •

edited

Loading