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 JuMP-level Nonnegatives, Nonpositives, and Zeros sets #3273

Merged
merged 2 commits into from Mar 10, 2023

Conversation

odow
Copy link
Member

@odow odow commented Mar 8, 2023

This change enables Ax >= b to be lowered to Ax - b in Nonnegatives(), and removes the confusing Vector-in-GreaterThan(0) method in build_constraint that asserted the right-hand side was zero.

Closes #3265

I'm not sure why it didn't occur to me to do this ages ago.

This change enables `Ax >= b` to be lowered to
`Ax - b in Nonnegatives()`, and removes the confusing
Vector-in-GreaterThan(0) method in build_constraint that asserted the
right-hand side was zero.
@codecov
Copy link

codecov bot commented Mar 8, 2023

Codecov Report

Patch coverage: 100.00% and no project coverage change

Comparison is base (63f92c2) 98.10% compared to head (45d5345) 98.11%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #3273   +/-   ##
=======================================
  Coverage   98.10%   98.11%           
=======================================
  Files          34       34           
  Lines        4704     4714   +10     
=======================================
+ Hits         4615     4625   +10     
  Misses         89       89           
Impacted Files Coverage Δ
src/sd.jl 100.00% <ø> (ø)
src/sets.jl 100.00% <ø> (ø)
src/macros.jl 98.60% <100.00%> (+0.02%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Co-authored-by: James Foster <38274066+jd-foster@users.noreply.github.com>
f::AbstractVector,
set::Nonnegatives,
)
return build_constraint(_error, f, MOI.Nonnegatives(length(f)))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is equivalent to assuming that @constraint(model, a >= b) is equivalent to @constraint(model, a >= b, Nonnegatives()) right ?
I think we explicitly decided not to have these cones by default but I would be ok to change it but I think we need to make sure everybody agrees on it.
It also kind of makes sense since we assumes Nonnegatives by default to compare vector of objective in multiobjective optimization.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is equivalent to assuming that @constraint(model, a >= b) is equivalent to @constraint(model, a >= b, Nonnegatives()) right?

Yes, for vector-valued a - b. I think this is pretty standard interpretation. Do you remember why we decided not to?

It makes it a lot easier to write models that conic solvers support without bridges. Currently they'll bridge every scalar linear constraint to a vector-valued one of dimension 1.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't remember, @mlubin do you ?
It's also the convention Boyd's uses in his book.

@mlubin
Copy link
Member

mlubin commented Mar 9, 2023

The trade-off is that the solvers where we care about an efficient direct mode are unlikely to natively support Nonnegatives. Isn't this a potentially breaking change for anyone using direct mode?

@blegat
Copy link
Member

blegat commented Mar 9, 2023

Isn't this a potentially breaking change for anyone using direct mode?

No because the only cases that will change are cases that were previously erroring. The scalar >= and .>= still create GreaterThan constraints but the vector >= used to error and it now creates a Nonnegatives constraint.

)
@test_throws_strip err @constraint(model, [x, 2x] == [1 - x, 3])
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's an example of code that previously errored

@mlubin
Copy link
Member

mlubin commented Mar 10, 2023

I guess it's ok then. It's a bit confusing that we'll allow both A*x >= b and A*x .>= b which are the same constraints with subtle differences.

Make sure to use [Julia's dot syntax](https://docs.julialang.org/en/v1/manual/functions/index.html#man-vectorized-1)
in front of the comparison operators (for example, `.==`, `.>=`, and `.<=`). If you
use a comparison without the dot, an error will be thrown.
The two constraints, `==` and `.==` are similar, but subtly different. The first
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this bit of the documentation enough to describe the differences?

@odow odow merged commit f38bfce into master Mar 10, 2023
11 checks passed
@odow odow deleted the od/quadrant-sets branch March 10, 2023 23:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

Add JuMP.Zeros set to replace Function-in-EqualTo(0)
4 participants