Closed pulsipher closed 10 months ago
We can use operator_to_set
which gets called by one of the fallbacks fot
parse_constraint_call
.
On Wed, Aug 16, 2023, 5:30 PM Hector Perez @.***> wrote:
@.**** commented on this pull request.
In src/constraints.jl https://github.com/hdavid16/DisjunctiveProgramming.jl/pull/43#discussion_r1296449296 :
- rhs0, set0 = rhs.args[2:3]
- if rhs.args[1] in (:in, :∈) && set0.head == :call && set0.args[2] isa Bool
- set = set0
- elseif rhs.args[1] == :(==) && set0 isa Bool
- set = _MOI.EqualTo(set0)
- else
- _error(error_msg)
- end
Ok. Sounds good to me. We should still give a meaningful error telling the user to use parenthesis. This would require extending the parse constraint call, right?
— Reply to this email directly, view it on GitHub https://github.com/hdavid16/DisjunctiveProgramming.jl/pull/43#discussion_r1296449296, or unsubscribe https://github.com/notifications/unsubscribe-auth/AICHX5ZU7LCTPDSRUNIH663XVU3WZANCNFSM5V2ALCOQ . You are receiving this because you were mentioned.Message ID: @.***>
I'll also leave a note that the expression creation in the reformulation steps can definitely be improved. Simply using @expression
where possible would make a difference.
My last suggestion would be to move all the big-m specific methods to bigm.jl and all the hull specific methods to hull.jl
That’s a good point. There is no guarantee that they will be created from the inside out since all you need is to specify the logical variables for each disjunction.
On Mon, Sep 18, 2023 at 11:27 PM Joshua Pulsipher @.***> wrote:
@.**** commented on this pull request.
In src/constraints.jl https://github.com/hdavid16/DisjunctiveProgramming.jl/pull/43#discussion_r1329534851 :
- dref = _disjunction(_error, model, structure, name)
- obj = JuMP.constraint_object(dref)
- return JuMP.add_constraint(model, _DisjunctConstraint(obj, tag.indicator), name)
Ya this is going to need some careful thought. I would be hesitant to add to disjunct constraints while reformulating since that will make it tricky to change to anothet reformulation. We can maybe make some buffer field to store them as we build the reformulation. Also note that th e disjunctions are stored in creation order and the inner most must be created first so they will be reformulated first.
— Reply to this email directly, view it on GitHub https://github.com/hdavid16/DisjunctiveProgramming.jl/pull/43#discussion_r1329534851, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACF3MIP35JK5YXHFX5WND63X3EGJFANCNFSM5V2ALCOQ . You are receiving this because you commented.Message ID: @.***>
Rather than comment on every function, we can use append!
instead of push!
to avoid splatting
Also, let's remove the extra spaces above most of the return
s
Note that test should not have its own Project.toml
. It should use the main one. Any additional dependences can be added with extra
and targets
field. See https://github.com/infiniteopt/InfiniteOpt.jl/blob/master/Project.toml
I think I figured out how to represent disjunctions at the MOI level, see https://github.com/pulsipher/DisjunctiveProgramming.jl/pull/1
Before we merge this, I think it would make sense to unify some of our logical operators with JuMP. Specifically, I think we should support:
&&
for logical and||
for logical or==
for logical equivalence
And adjust the underlying symbols in the NonlinearExprs accordingly. Our current operators would just be another way to define things. We might also consider supporting this addition to JuMP: https://github.com/jump-dev/JuMP.jl/pull/3530. This would allow users to avoid wrapping the expression in parentheses and explicitly give the set.
Let me know what you think.
I think this is ready to be merged, but it might be worth waiting to see what develops with https://github.com/jump-dev/JuMP.jl/pull/3530 so can we can adjust the logical constraint structure accordingly. Alternatively, we can merge this and then address any adjustments in a future PR before releasing a new version of DisjunctiveProgramming.
Let's merge and add support for :=
in a later release. Thanks!
Copied over to #68
This pull request will take some time and seeks to completely refactor DisjunctiveProgramming.jl to better extend JuMP in a more modular and performant way.
TODO
Add printing(save for a future PR)Add documentation via Documenter.jl(save for a future PR)