Skip to content

Conversation

@ericphanson
Copy link
Collaborator

Builds off of #467

I think we can support both MOI v0.9 and MOI v0.10 pretty easily, since there are so few changes we need to make. It's a bit hacky, but I think it is worth it, since there might be a long tail of solvers that aren't updated. E.g. dropping MBP support in Convex was pretty annoying for me personally (even though I was the one who did it) since I ended up wanting to use Pajarito, and this requires much less compat shimming than that would have.

I've updated CI here to test with both versions.

@codecov
Copy link

codecov bot commented Nov 14, 2021

Codecov Report

Merging #468 (f0eac7a) into master (c10461b) will increase coverage by 0.27%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #468      +/-   ##
==========================================
+ Coverage   91.82%   92.09%   +0.27%     
==========================================
  Files          83       83              
  Lines        5137     5137              
==========================================
+ Hits         4717     4731      +14     
+ Misses        420      406      -14     
Impacted Files Coverage Δ
src/solution.jl 92.09% <100.00%> (+7.90%) ⬆️

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 c10461b...f0eac7a. Read the comment docs.

Base automatically changed from eph/moi_update to master November 14, 2021 14:29
@ericphanson ericphanson requested a review from odow November 14, 2021 15:13
Copy link
Member

@odow odow left a comment

Choose a reason for hiding this comment

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

Nice. If you want something that doesn't use try-catch, I think you should be able to use

if !isdefined(MOI, :SingleVariable) || MOI.SingleVariable isa Function
    const SV_OR_VI, SV_OR_IDENTITY = MOI.VariableIndex, identity
else
    const SV_OR_VI, SV_OR_IDENTITY = MOI.SingleVariable, MOI.VariableIndex
end

Co-authored-by: Oscar Dowson <[email protected]>
@ericphanson
Copy link
Collaborator Author

Thanks, that's nicer! (The second branch should have MOI.SingleVariable, MOI.SingleVariable, by the way). Made that change in f0eac7a; if CI passes I'll merge+tag.

@ericphanson ericphanson merged commit b092a93 into master Nov 14, 2021
@ericphanson ericphanson deleted the eph/moi_compat branch November 14, 2021 20:56
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.

3 participants