Skip to content

Conversation

@odow
Copy link
Member

@odow odow commented Oct 1, 2020

Closes #311

@codecov
Copy link

codecov bot commented Oct 1, 2020

Codecov Report

Merging #318 into master will increase coverage by 0.32%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #318      +/-   ##
==========================================
+ Coverage   54.08%   54.40%   +0.32%     
==========================================
  Files           7        7              
  Lines        2703     2722      +19     
==========================================
+ Hits         1462     1481      +19     
  Misses       1241     1241              
Impacted Files Coverage Δ
src/MOI/MOI_wrapper.jl 89.35% <100.00%> (+0.15%) ⬆️

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 8bf4a84...86e67c2. Read the comment docs.

@odow
Copy link
Member Author

odow commented Oct 4, 2020

Barring objections, I'll leave this open for another 48 hours, then merge.

@blegat
Copy link
Member

blegat commented Oct 4, 2020

Not sure this is relevant but it seems you are handling cases where both bounds and the ZeroOne constraint are added.
These cases should be an error, see https://github.com/jump-dev/MathOptInterface.jl/blob/0423a6c61c6f47e3baf81642dd60e6c7a3c9e405/src/Utilities/model.jl#L482-L496

@odow
Copy link
Member Author

odow commented Oct 4, 2020

These cases should be an error

Most definitely not. Where did we discuss this??? People add binary variables with bounds all the time.

@blegat
Copy link
Member

blegat commented Oct 5, 2020

It seems it was discussed in
jump-dev/MathOptInterface.jl#570 (comment)
and then implemented in
jump-dev/MathOptInterface.jl#732
Looking at the LOWER_BOUND_MASK and UPPER_BOUND_MASK in more details, it seems that we consider that ZeroOne don't set bound (the bounds are 0 and 1, we know it from the existence of the ZeroOne flag in the mask so we don't need to store the bounds anywhere so it does not conflict with other set that need their bounds to be stored.

@odow
Copy link
Member Author

odow commented Oct 5, 2020

Correct. ZeroOne does not set bounds.

@odow odow merged commit 46e107a into master Oct 5, 2020
@odow odow deleted the od/binary branch October 5, 2020 20:49
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.

CPLEX---Warning: Non-integral bounds for integer variables rounded.

3 participants