Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add parse_constraint_expr and parse_constraint_head #2228

Merged
merged 2 commits into from
May 4, 2020

Conversation

blegat
Copy link
Member

@blegat blegat commented Apr 22, 2020

This PR continues #2051. We avoid breaking the parse_constraint API. We might consider renaming parse_constraint in parse_call_constraint and parse_constraint_head in parse_constraint but this might be breaking. We can consider this renaming just before we plan to release JuMP v0.22 or v1.0.
Allow parsing of non-:call expressions in @constraint.
@dourouc05 Could you provide an example use case for this ?
This is point 1) of #2051 (comment)

@Wikunia
Copy link
Contributor

Wikunia commented Apr 22, 2020

This would allow to have constraints like:

@constraint(m, b := {x >= y})

which was a possible syntax for the reified constraint.

One can implement:

function JuMP.parse_constraint_head(_error::Function, ::Val{:(:=)}, ls, rs)

in that case which wasn't possible before with the parse_constraint function itself as far as I know.

dourouc05 added a commit to dourouc05/JuMP.jl that referenced this pull request Apr 22, 2020
dourouc05 added a commit to dourouc05/JuMP.jl that referenced this pull request Apr 22, 2020
@codecov
Copy link

codecov bot commented Apr 29, 2020

Codecov Report

Merging #2228 into master will decrease coverage by 0.01%.
The diff coverage is 90.90%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2228      +/-   ##
==========================================
- Coverage   91.17%   91.16%   -0.02%     
==========================================
  Files          42       42              
  Lines        4159     4165       +6     
==========================================
+ Hits         3792     3797       +5     
- Misses        367      368       +1     
Impacted Files Coverage Δ
src/macros.jl 92.52% <90.00%> (-0.14%) ⬇️
src/indicator.jl 96.00% <100.00%> (ø)

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 ef902fe...a6f7948. Read the comment docs.

@blegat blegat merged commit a72e7ac into master May 4, 2020
dourouc05 added a commit to dourouc05/JuMP.jl that referenced this pull request May 5, 2020
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 added a commit to dourouc05/JuMP.jl that referenced this pull request May 5, 2020
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 added a commit to dourouc05/JuMP.jl that referenced this pull request May 5, 2020
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 added a commit to dourouc05/JuMP.jl that referenced this pull request May 5, 2020
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 added a commit to dourouc05/JuMP.jl that referenced this pull request May 5, 2020
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.
@odow odow deleted the bl/constraint_head branch May 10, 2020 19:35
@blegat blegat added this to the 0.21.3 milestone Jun 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants