-
-
Notifications
You must be signed in to change notification settings - Fork 398
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 := operator for Boolean satisfiability problems #3530
Conversation
Codecov ReportAll modified lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3530 +/- ##
=======================================
Coverage 98.25% 98.26%
=======================================
Files 37 37
Lines 5563 5580 +17
=======================================
+ Hits 5466 5483 +17
Misses 97 97
☔ View full report in Codecov by Sentry. |
Looks ok! No strong opinions from me. Thanks for experimenting. |
So this minizinc
makes the flatzinc
So the minizinc compiler:
I guess any arithmetic between I don't know what that means for us. But perhaps we don't need this special syntax. |
Seems possibly inefficient... what if you make it a feasibility problem with no objective? I would expect a boolean interpretation to make more sense if the sub-solver is a SAT solver at least |
This is with
|
There's also a general problem with julia> model = GenericModel{Bool}()
A JuMP Model
Feasibility problem with:
Variables: 0
Model mode: AUTOMATIC
CachingOptimizer state: NO_OPTIMIZER
Solver name: No optimizer attached.
julia> @variable(model, x[1:2])
2-element Vector{GenericVariableRef{Bool}}:
x[1]
x[2]
julia> x[1] - x[2]
ERROR: MethodError: no method matching _build_aff_expr(::Bool, ::Bool, ::GenericVariableRef{Bool}, ::Int64, ::GenericVariableRef{Bool})
Closest candidates are:
_build_aff_expr(::V, ::V, ::K, ::V, ::K) where {V, K}
@ JuMP ~/.julia/dev/JuMP/src/aff_expr.jl:87
_build_aff_expr(::V, ::V, ::K) where {V, K}
@ JuMP ~/.julia/dev/JuMP/src/aff_expr.jl:81
Stacktrace:
[1] -(lhs::GenericVariableRef{Bool}, rhs::GenericVariableRef{Bool})
@ JuMP ~/.julia/dev/JuMP/src/operators.jl:120
[2] top-level scope
@ REPL[14]:1 I think this just means that you can't write You have to write |
src/constraints.jl
Outdated
$parse_code_rhs | ||
end | ||
build_code = quote | ||
if $new_rhs isa Bool |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we do it at the parsing level, it won't work if rhs
is a Julia variable (not a JuMP decision variable) for which the value is a Bool
right ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This part is at runtime. See the tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah indeed. I guess it's best to add code inside the build_constraint
that add code that is generated by the macro if possible
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But in this case, it might be difficult
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We would need a new JuMP set Equal
with and do build_constraint(error, (a, b), JuMP.Equal())
The asymmetry of the symbol
and
should be equivalent, but superficially they don't seem to be. |
We could use Ideally, people would only write |
I had a play to try and come up with a better syntax, but I couldn't really think of anything |
This would be helpful for defining logical constraints in DisjunctiveProgramming.jl. |
Any syntax suggestions? |
In our use case, it would seem unlikely to have anything beside a We would also prefer for logic constraints to have a distinct set type (not |
But it'd be |
Here are our options for operators with a higher precedence than
For ASCII operators, we really only have
|
julia> dump(:(x == y === true))
Expr
head: Symbol comparison
args: Array{Any}((5,))
1: Symbol x
2: Symbol ==
3: Symbol y
4: Symbol ===
5: Bool true |
Ya the automated conversion is what led to us to avoid using
Ya |
I've changed it so that I wonder if we should enforce that the |
I think restricting the RHS to be a |
Yeah, I've changed to only rhs Bool and it makes more sense. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great.
We may want to expose the |
x-ref #2227
This is one improvement suggested by today's call, motivated by the fact that
==
has lower precedence than&&
and||
, sox || y == true
parses asx || (y == true)
. The new syntax,x || y := true
parses as(x || y) := true
.The key difference of
(lhs == rhs) in EqualTo(true)`. Thus, it should pretty much only be used for Boolean problems that support ScalarNonlinearFunction.
:=
compared with==
is that==
useslhs - rhs in EqualTo(false)
, whereas:=
lowers to the NonlinearExprcc @chriscoey thoughts?
See the examples in
test/test_constraint.jl
TODO: