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

Better support of single variable constraints #590

Merged
merged 22 commits into from
Oct 13, 2021
Merged

Conversation

guimarqu
Copy link
Contributor

@guimarqu guimarqu commented Aug 14, 2021

Introduce a new structure to store single variable constraints.

task for #583, #436.

A lot of work in this PR, because this change has an impact on almost everything in the MathProg modules.
I did this PR because we were not supporting multiple single var constraints and the dual value associated with single var constraints was not in the Dual Solution.

Contributions were highly interdependent.

Contribution 1

Creation of a new data structure SingleVarConstraint to store the representation of a single var constraint : 1*var <> rhs

This constraint is not stored in the coefficient matrix nor transmitted to the subsolver for the sake of performance.
If one creates such a constraint, changes its sense or its rhs, one must ensure that one calls bounds_propagation! before calling the subsolver.

Contribution 2

bounds_propagation! iterates over the single var constraints and updates the lower and upper bounds of the variables.
These changes are propagated to the subsolver.

Contribution 3

New data structure for PrimalSolution and DualSolution (first task of #593) because the DualSolution now stores the reduced cost and the active bound of each variable (if it has a non-zero reduced cost)

@guimarqu guimarqu added the bug Something isn't working label Aug 14, 2021
@codecov
Copy link

codecov bot commented Aug 14, 2021

Codecov Report

Merging #590 (4a23484) into master (85c9481) will increase coverage by 0.41%.
The diff coverage is 86.11%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #590      +/-   ##
==========================================
+ Coverage   86.64%   87.06%   +0.41%     
==========================================
  Files          47       47              
  Lines        4792     4978     +186     
==========================================
+ Hits         4152     4334     +182     
- Misses        640      644       +4     
Impacted Files Coverage Δ
src/Algorithm/node.jl 77.77% <ø> (ø)
src/MathProg/duties.jl 66.66% <ø> (ø)
src/MathProg/types.jl 100.00% <ø> (ø)
src/MathProg/formulation.jl 78.19% <69.44%> (+0.54%) ⬆️
src/MOIwrapper.jl 85.02% <81.35%> (+1.04%) ⬆️
src/MathProg/manager.jl 61.76% <83.33%> (+5.76%) ⬆️
src/MathProg/vcids.jl 79.16% <85.71%> (+6.75%) ⬆️
src/MathProg/MOIinterface.jl 90.51% <90.62%> (-1.19%) ⬇️
src/MathProg/varconstr.jl 92.82% <91.54%> (+5.93%) ⬆️
src/MathProg/solutions.jl 83.82% <95.34%> (+14.25%) ⬆️
... and 22 more

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 85c9481...4a23484. Read the comment docs.

@guimarqu guimarqu marked this pull request as ready for review September 4, 2021 14:25
@guimarqu
Copy link
Contributor Author

guimarqu commented Sep 4, 2021

I think it's time to merge this PR into master... I think the review will be a very hard job.

This PR does not fix any issue but is a prerequisite for issues #583 #593 #436 #591 #531.

Moreover, it turns out that SingleVariableConstraint disappears in the next release of MathOptInterface.
Without this PR, this change in MOI would dramatically decrease the performance of Coluna. I'll update the MOI wrapper & the decomposition in a future PR.

Next tasks (I'll open a follow-up issue) :

  • get rid of MOI.SingleVarConstraint and update MOI wrapper and use single var constraints when length(func.terms) == 1 where func is a scalar affine function (fix Constraint on variable overwritten #583)
  • use redcost_vars of a dual solution in algorithm (it seems to work for now because @constraint(m, c, x >= 1) is considered as a ScalarAffineFunction - Set constraint by MOI
  • create single var constraint in subproblems when Coluna branches on a single variable (will fix Variable bounds not updated in branching #531)
  • fix MOI tests with TODO comment (fix MOI tests #436)

@guimarqu guimarqu added the waiting Waiting for another package version release or patch label Sep 4, 2021
Copy link
Collaborator

@rrsadykov rrsadykov left a comment

Choose a reason for hiding this comment

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

Yes, I think that it is not possible to really review this pull request. You may just explain us (at a high level) what did you do on our next meeting.

@@ -8,7 +8,7 @@ mutable struct Node
depth::Int
parent::Union{Nothing, Node}
optstate::OptimizationState
#branch::Union{Nothing, Branch} # branch::Id{Constraint}
#branch::Union{Nothing, Branch} # branch::ConstrId
Copy link
Collaborator

Choose a reason for hiding this comment

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

This can be deleted

@guimarqu
Copy link
Contributor Author

guimarqu commented Sep 6, 2021

@rrsadykov Yes it's planned ! I don't merge it right now because I think it's better to release a 0.3.10 before. More work for 0.4.0 than I initially thought.

@guimarqu guimarqu removed the waiting Waiting for another package version release or patch label Sep 8, 2021
@guimarqu guimarqu added the waiting Waiting for another package version release or patch label Sep 28, 2021
@guimarqu guimarqu mentioned this pull request Oct 9, 2021
5 tasks
@guimarqu guimarqu removed the waiting Waiting for another package version release or patch label Oct 13, 2021
@guimarqu guimarqu merged commit f68c49c into master Oct 13, 2021
@guimarqu guimarqu deleted the single_var_bound branch October 13, 2021 10:19
@guimarqu guimarqu restored the single_var_bound branch October 13, 2021 13:17
@guimarqu guimarqu deleted the single_var_bound branch March 19, 2022 13:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants