-
Notifications
You must be signed in to change notification settings - Fork 93
Add missing PSDCS in bridge cache #524
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
Conversation
Codecov Report
@@ Coverage Diff @@
## master #524 +/- ##
==========================================
+ Coverage 95.57% 95.65% +0.07%
==========================================
Files 46 46
Lines 4678 4692 +14
==========================================
+ Hits 4471 4488 +17
+ Misses 207 204 -3
Continue to review full report at Codecov.
|
test/bridge.jl
Outdated
@@ -162,6 +162,7 @@ MOIU.@model(NoRSOCModel, | |||
|
|||
@testset "Combining two briges" begin | |||
fullbridgedmock = MOIB.fullbridgeoptimizer(mock, Float64) | |||
@test MOI.supports_constraint(fullbridgedmock, MOI.VectorAffineFunction{Float64}, MOI.PositiveSemidefiniteConeSquare) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test doesn't fall under the "combining two bridges" header. Consider using a different testset.
test/bridge.jl
Outdated
end | ||
for F in [MOI.VectorOfVariables, MOI.VectorAffineFunction{Float64}, | ||
MOI.VectorQuadraticFunction{Float64}] | ||
@show F |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Debug output?
test/bridge.jl
Outdated
MOI.GeometricMeanCone) | ||
end | ||
for F in [MOI.VectorOfVariables, MOI.VectorAffineFunction{Float64}] | ||
# Missing vcat for quadratic for supporting quadratic |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand these two comments. Are they TODOs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes :)
test/bridge.jl
Outdated
MOI.GeometricMeanCone) | ||
end | ||
for F in [MOI.VectorOfVariables, MOI.VectorAffineFunction{Float64}] | ||
# TODO Missing vcat for quadratic for supporting quadratic |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I still don't understand these TODOs and how they relate to the surrounding code. Are they implying that certain constraints are not yet supported? Which?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These tests are not in the previous for
because they do not support VectorQuadraticFunction yet
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you clarify that on the comments?
Let's tag MOI v0.6.1 once this is merged since this is a critical fix now that jump-dev/JuMP.jl#1468 is merged
Closes jump-dev/JuMP.jl#1476