Skip to content

Commit b26e1ed

Browse files
authored
Refactor Logical Constraints to use := (#91)
* refactor for := * Improve coverage and fix macro bug * fix macro bug * another bug fix * fix bug * doc and coverage fixes * improve coverage * add test
1 parent 04334c9 commit b26e1ed

File tree

15 files changed

+169
-163
lines changed

15 files changed

+169
-163
lines changed

Project.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ JuMP = "4076af6c-e467-56ae-b986-b466b2749572"
88
Reexport = "189a3867-3050-52da-a836-e630ba90ab69"
99

1010
[compat]
11-
JuMP = "1.15"
11+
JuMP = "1.16"
1212
Reexport = "1"
1313
julia = "1.6"
1414

README.md

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -67,16 +67,16 @@ Two types of logical constraints are supported:
6767

6868
2. `Proposition` or Boolean constraints: These describe the relationships between Logical variables via Boolean algebra. Supported logical operators include:
6969

70-
- `` or `logical_or` (OR, typed with `\vee + tab`).
71-
- `` or `logical_and` (AND, typed with `\wedge + tab`).
70+
- `` or `logical_or` or `||` (OR, typed with `\vee + tab`).
71+
- `` or `logical_and` or `&&` (AND, typed with `\wedge + tab`).
7272
- `¬` or `logical_not` (NOT, typed with `\neg + tab`).
7373
- `` of `implies` (Implication, typed with `\Longrightarrow + tab`).
74-
- `` or `iff` (double implication or equivalence, typed with `\Leftrightarrow + tab`).
74+
- `` or `iff` or `==` (double implication or equivalence, typed with `\Leftrightarrow + tab`).
7575

76-
The `@constraint` JuMP macro is used to create these constraints with the `IsTrue` set:
76+
The `@constraint` JuMP macro is used to create these constraints using `:=`:
7777

7878
```julia
79-
@constraint(model, (Y[1] ⟹ Y[2]) in IsTrue())
79+
@constraint(model, Y[1] ⟹ Y[2] := true)
8080
```
8181

8282
_Note_: The parenthesis in the example above around the implication clause are only required when the parent logical operator is `` or `` to avoid parsing errors.

docs/src/index.md

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -67,16 +67,16 @@ Two types of logical constraints are supported:
6767

6868
2. `Proposition` or Boolean constraints: These describe the relationships between Logical variables via Boolean algebra. Supported logical operators include:
6969

70-
- `` or `logical_or` (OR, typed with `\vee + tab`).
71-
- `` or `logical_and` (AND, typed with `\wedge + tab`).
70+
- `` or `logical_or` or `||` (OR, typed with `\vee + tab`).
71+
- `` or `logical_and` or `&&` (AND, typed with `\wedge + tab`).
7272
- `¬` or `logical_not` (NOT, typed with `\neg + tab`).
7373
- `` of `implies` (Implication, typed with `\Longrightarrow + tab`).
74-
- `` or `iff` (double implication or equivalence, typed with `\Leftrightarrow + tab`).
74+
- `` or `iff` or `==` (double implication or equivalence, typed with `\Leftrightarrow + tab`).
7575

76-
The `@constraint` JuMP macro is used to create these constraints with the `IsTrue` set:
76+
The `@constraint` JuMP macro is used to create these constraints with `:=`:
7777

7878
```julia
79-
@constraint(model, (Y[1] ⟹ Y[2]) in IsTrue())
79+
@constraint(model, Y[1] ⟹ Y[2] := true)
8080
```
8181

8282
_Note_: The parenthesis in the example above around the implication clause are only required when the parent logical operator is `` or `` to avoid parsing errors.

examples/ex4.jl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ using DisjunctiveProgramming
66

77
m = GDPModel()
88
@variable(m, Y[1:4], Logical)
9-
@constraint(m, ¬((Y[1] ¬Y[2]) (Y[3] Y[4])) IsTrue())
9+
@constraint(m, ¬((Y[1] ¬Y[2]) (Y[3] Y[4])) := true)
1010
reformulate_model(m, BigM())
1111
print(m)
1212
# Feasibility

src/constraints.jl

Lines changed: 81 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -93,14 +93,14 @@ for (RefType, loc) in ((:DisjunctConstraintRef, :disjunct_constraints),
9393
return cref1.model === cref2.model && cref1.index == cref2.index
9494
end
9595
Base.copy(cref::$RefType) = cref
96-
@doc """
97-
Base.getindex(map::GenericReferenceMap, cref::$($RefType))
98-
99-
...
100-
"""
101-
function Base.getindex(map::ReferenceMap, cref::$RefType)
102-
$RefType(map.model, index(cref))
103-
end
96+
# @doc """
97+
# Base.getindex(map::GenericReferenceMap, cref::$($RefType))
98+
99+
# ...
100+
# """
101+
# function Base.getindex(map::ReferenceMap, cref::$RefType)
102+
# $RefType(map.model, index(cref))
103+
# end
104104
end
105105
end
106106

@@ -432,19 +432,6 @@ end
432432
################################################################################
433433
# LOGICAL CONSTRAINTS
434434
################################################################################
435-
JuMP.operator_to_set(_error::Function, ::Val{:⟹}) = _error(
436-
"Cannot use ⟹ in a MOI set (invalid right-hand side). If you are seeing this error, " *
437-
"you likely added a logical constraint of the form A ⟹ B ∈ IsTrue(). " *
438-
"Instead, you should enclose the constraint function in parenthesis: " *
439-
"(A ⟹ B) ∈ IsTrue()."
440-
)
441-
JuMP.operator_to_set(_error::Function, ::Val{:⇔}) = _error(
442-
"Cannot use ⇔ in a MOI set (invalid right-hand side). If you are seeing this error, " *
443-
"you likely added a logical constraint of the form A ⇔ B ∈ IsTrue(). " *
444-
"Instead, you should enclose the constraint function in parenthesis: " *
445-
"(A ⇔ B) ∈ IsTrue()."
446-
)
447-
448435
"""
449436
function JuMP.build_constraint(
450437
_error::Function,
@@ -468,17 +455,6 @@ model = GDPModel();
468455
@variable(model, Y[i = 1:2], LogicalVariable);
469456
@constraint(model, [Y[1], Y[2]] in Exactly(1));
470457
```
471-
472-
JuMP.build_constraint(
473-
_error::Function,
474-
func::_LogicalExpr,
475-
set::IsTrue
476-
)
477-
478-
Extend `JuMP.build_constraint` to add logical propositional constraints to a [`GDPModel`](@ref).
479-
This in combination with `JuMP.add_constraint` enables the use of
480-
`@constraint(model, [name], logical_expr in IsTrue())` to define a Boolean expression that must
481-
either be true or false.
482458
"""
483459
function JuMP.build_constraint( # Cardinality logical constraint
484460
_error::Function,
@@ -497,31 +473,6 @@ function JuMP.build_constraint( # Cardinality logical constraint
497473
_error("Selector constraints can only be applied to a Vector or Container of LogicalVariableRefs.")
498474
end
499475

500-
# Proposition logical constraint: _LogicalExpr
501-
function JuMP.build_constraint(
502-
_error::Function,
503-
func::_LogicalExpr,
504-
set::IsTrue
505-
)
506-
if !(func.head in _LogicalOperatorHeads)
507-
_error("Unrecognized logical operator `$(func.head)`.")
508-
else
509-
return ScalarConstraint(func, set)
510-
end
511-
end
512-
513-
# Fallback for LogicalVariableRef in IsTrue
514-
function JuMP.build_constraint(
515-
_error::Function,
516-
func::LogicalVariableRef,
517-
set::IsTrue
518-
)
519-
_error(
520-
"Logical propositions must be of the form `logical_expr in IsTrue()`. " *
521-
"If you are trying to fix a logical variable, use `fix(logical_var, true)` instead."
522-
)
523-
end
524-
525476
# Fallback for Affine/Quad expressions
526477
function JuMP.build_constraint(
527478
_error::Function,
@@ -531,7 +482,7 @@ function JuMP.build_constraint(
531482
_error("Cannot add, subtract, or multiply with logical variables.")
532483
end
533484

534-
# Fallback for other set types (TODO: we could relax this later if needed)
485+
# Fallback for other set types
535486
function JuMP.build_constraint(
536487
_error::Function,
537488
expr::Union{LogicalVariableRef, _LogicalExpr},
@@ -540,16 +491,78 @@ function JuMP.build_constraint(
540491
_error("Invalid set `$set` for logical constraint.")
541492
end
542493

494+
# Check that logical expression is valid
495+
function _check_logical_expression(ex)
496+
return
497+
end
498+
function _check_logical_expression(ex::_LogicalExpr)
499+
if !(ex.head in _LogicalOperatorHeads)
500+
error("Unrecognized logical operator `$(ex.head)`.")
501+
end
502+
for a in ex.args
503+
_check_logical_expression(a)
504+
end
505+
return
506+
end
507+
543508
"""
544509
function JuMP.add_constraint(
545-
model::Model,
546-
c::ScalarConstraint{<:F, S},
510+
model::JuMP.Model,
511+
c::JuMP.ScalarConstraint{_LogicalExpr, MOI.EqualTo{Bool}},
547512
name::String = ""
548-
) where {F <: Union{LogicalVariableRef, _LogicalExpr}, S}
513+
)
549514
550515
Extend `JuMP.add_constraint` to allow creating logical proposition constraints
551-
for a [`GDPModel`](@ref) with the `@constraint` macro.
516+
for a [`GDPModel`](@ref) with the `@constraint` macro. Users should define
517+
logical constraints via the syntax `@constraint(model, logical_expr := true)`.
518+
"""
519+
function JuMP.add_constraint(
520+
model::JuMP.Model,
521+
c::JuMP.ScalarConstraint{_LogicalExpr, S},
522+
name::String = ""
523+
) where {S} # S <: JuMP._DoNotConvertSet{MOI.EqualTo{Bool}} or MOI.EqualTo{Bool}
524+
# check the constraint out
525+
is_gdp_model(model) || error("Can only add logical constraints to `GDPModel`s.")
526+
set = JuMP.moi_set(c)
527+
@assert set isa MOI.EqualTo{Bool} "Unexpected set `$set` for logical constraint."
528+
func = JuMP.jump_function(c)
529+
JuMP.check_belongs_to_model(func, model)
530+
_check_logical_expression(func)
531+
# add negation if needed
532+
if !set.value
533+
func = _LogicalExpr(:!, func)
534+
set = MOI.EqualTo{Bool}(true)
535+
end
536+
# add the constraint
537+
new_c = JuMP.ScalarConstraint(func, set) # we have guarranteed that set.value = true
538+
constr_data = ConstraintData(new_c, name)
539+
idx = _MOIUC.add_item(_logical_constraints(model), constr_data)
540+
_set_ready_to_optimize(model, false)
541+
return LogicalConstraintRef(model, idx)
542+
end
543+
function JuMP.add_constraint(
544+
model::JuMP.Model,
545+
c::JuMP.ScalarConstraint{LogicalVariableRef, S},
546+
name::String = ""
547+
) where {S} # S <: JuMP._DoNotConvertSet{MOI.EqualTo{Bool}} or MOI.EqualTo{Bool}
548+
error("Cannot define constraint on single logical variable, use `fix` instead.")
549+
end
550+
function JuMP.add_constraint(
551+
model::JuMP.Model,
552+
c::JuMP.ScalarConstraint{GenericAffExpr{C, LogicalVariableRef}, S},
553+
name::String = ""
554+
) where {S, C}
555+
error("Cannot add, subtract, or multiply with logical variables.")
556+
end
557+
function JuMP.add_constraint(
558+
model::JuMP.Model,
559+
c::JuMP.ScalarConstraint{GenericQuadExpr{C, LogicalVariableRef}, S},
560+
name::String = ""
561+
) where {S, C}
562+
error("Cannot add, subtract, or multiply with logical variables.")
563+
end
552564

565+
"""
553566
function JuMP.add_constraint(
554567
model::Model,
555568
c::VectorConstraint{<:F, S, Shape},
@@ -560,24 +573,14 @@ Extend `JuMP.add_constraint` to allow creating logical cardinality constraints
560573
for a [`GDPModel`](@ref) with the `@constraint` macro.
561574
"""
562575
function JuMP.add_constraint(
563-
model::Model,
564-
c::ScalarConstraint{F, S},
565-
name::String = ""
566-
) where {F <: Union{LogicalVariableRef, _LogicalExpr}, S <: IsTrue}
567-
is_gdp_model(model) || error("Can only add logical constraints to `GDPModel`s.")
568-
@assert all(is_valid.(model, _get_constraint_variables(model, c))) "Constraint variables do not belong to model."
569-
constr_data = ConstraintData(c, name)
570-
idx = _MOIUC.add_item(_logical_constraints(model), constr_data)
571-
_set_ready_to_optimize(model, false)
572-
return LogicalConstraintRef(model, idx)
573-
end
574-
function JuMP.add_constraint(
575-
model::Model,
576-
c::VectorConstraint{F, S, Shape},
576+
model::JuMP.Model,
577+
c::JuMP.VectorConstraint{F, S, Shape},
577578
name::String = ""
578-
) where {F, S <: Union{_MOIAtLeast, _MOIAtMost, _MOIExactly}, Shape}
579+
) where {F, S <: Union{_MOIAtLeast, _MOIAtMost, _MOIExactly}, Shape}
579580
is_gdp_model(model) || error("Can only add logical constraints to `GDPModel`s.")
580-
@assert all(is_valid.(model, _get_constraint_variables(model, c))) "Constraint variables do not belong to model."
581+
func = JuMP.jump_function(c)
582+
JuMP.check_belongs_to_model.(filter(Base.Fix2(isa, JuMP.AbstractJuMPScalar), func), model)
583+
# TODO maybe do some formatting on `c` to ensure the types are what we expect later
581584
constr_data = ConstraintData(c, name)
582585
idx = _MOIUC.add_item(_logical_constraints(model), constr_data)
583586
_set_ready_to_optimize(model, false)

src/datatypes.jl

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -55,11 +55,6 @@ struct LogicalVariableRef <: AbstractVariableRef
5555
index::LogicalVariableIndex
5656
end
5757

58-
################################################################################
59-
# LOGICAL PROPOSITION SETS
60-
################################################################################
61-
struct IsTrue <: _MOI.AbstractScalarSet end
62-
6358
################################################################################
6459
# LOGICAL SELECTOR (CARDINALITY) SETS
6560
################################################################################

src/macros.jl

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ function _extract_kwargs(args)
5151
arg_list = collect(args)
5252
if !isempty(args) && isexpr(args[1], :parameters)
5353
p = popfirst!(arg_list)
54-
append!(arg_list, p.args)
54+
append!(arg_list, (Expr(:(=), a.args...) for a in p.args))
5555
end
5656
extra_kwargs = filter(x -> isexpr(x, :(=)) && x.args[1] != :container &&
5757
x.args[1] != :base_name, arg_list)
@@ -160,22 +160,22 @@ The recognized keyword arguments in `kw_args` are the following:
160160
161161
To create disjunctions without macros, see [`disjunction`](@ref).
162162
"""
163-
macro disjunction(model, args...)
164-
# prepare the model
165-
esc_model = esc(model)
166-
163+
macro disjunction(args...)
167164
# define error message function
168-
_error(str...) = _macro_error(:disjunction, (model, args...),
165+
166+
_error(str...) = _macro_error(:disjunction, (args...,),
169167
__source__, str...)
170168

171169
# parse the arguments
172170
pos_args, extra_kwargs, container_type, base_name = _extract_kwargs(args)
173171

174172
# initial processing of positional arguments
175-
length(pos_args) >= 1 || _error("Not enough arguments, please see docs for accepted `@disjunction` syntax..")
173+
length(pos_args) >= 2 || _error("Not enough arguments, please see docs for accepted `@disjunction` syntax..")
174+
model = popfirst!(pos_args)
175+
esc_model = esc(model)
176176
y = first(pos_args)
177177
extra = pos_args[2:end]
178-
if isexpr(args[1], :block)
178+
if isexpr(args[2], :block)
179179
_error("Invalid syntax. Did you mean to use `@disjunctions`?")
180180
end
181181

0 commit comments

Comments
 (0)