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 support for constraint dual #1129

Merged
merged 3 commits into from Nov 23, 2017
Merged

Add support for constraint dual #1129

merged 3 commits into from Nov 23, 2017

Conversation

blegat
Copy link
Member

@blegat blegat commented Nov 21, 2017

Closes #1127

@codecov
Copy link

codecov bot commented Nov 22, 2017

Codecov Report

Merging #1129 into jump/moi will increase coverage by 0.18%.
The diff coverage is 91.66%.

Impacted file tree graph

@@             Coverage Diff              @@
##           jump/moi    #1129      +/-   ##
============================================
+ Coverage     28.72%   28.91%   +0.18%     
============================================
  Files            20       20              
  Lines          3648     3659      +11     
============================================
+ Hits           1048     1058      +10     
- Misses         2600     2601       +1
Impacted Files Coverage Δ
src/solverinterface.jl 83.78% <100%> (+4.47%) ⬆️
src/JuMP.jl 59.77% <66.66%> (+0.24%) ⬆️

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 db33c82...13bb1e6. Read the comment docs.

src/JuMP.jl Outdated
@@ -100,6 +100,9 @@ mutable struct Model <: AbstractModel
# obj#::QuadExpr
# objSense::Symbol

# We use the constraint reference value to have a concrete type
constrainttosolverconstraint::Dict{UInt64,UInt64}
Copy link
Member

Choose a reason for hiding this comment

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

  1. I don't like how this is untyped.
  2. I think it's incorrect, because constraint reference values are allowed to overlap if they're of different types.

Copy link
Member Author

Choose a reason for hiding this comment

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

I knew you were not going to like it :D

src/JuMP.jl Outdated
@@ -100,6 +100,9 @@ mutable struct Model <: AbstractModel
# obj#::QuadExpr
# objSense::Symbol

# We use the constraint reference value to have a concrete type
Copy link
Member

Choose a reason for hiding this comment

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

Comment should be updated.

src/JuMP.jl Outdated
@@ -100,6 +100,9 @@ mutable struct Model <: AbstractModel
# obj#::QuadExpr
# objSense::Symbol

# We use the constraint reference value to have a concrete type
constrainttosolverconstraint::Dict{MOICON,MOICON}
Copy link
Member

Choose a reason for hiding this comment

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

Doesn't this field need to be initialized in the Model() constructor? Why doesn't this code crash?

Copy link
Member Author

Choose a reason for hiding this comment

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

I am doing the same than what you did with variabletosolvervariable. It is initialized in every call of attach. If the variable is accessed before attach is ever called, there will be an Undef... error but this does not happen since the dictionary only have sense once a solver is attached.

@mlubin mlubin merged commit d3e40d9 into jump/moi Nov 23, 2017
@mlubin mlubin deleted the bl/resultdual branch November 25, 2017 21:22
@mlubin mlubin mentioned this pull request Nov 25, 2017
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.

None yet

2 participants