Skip to content

Conversation

@blegat
Copy link
Member

@blegat blegat commented Oct 25, 2018

This error makes LQOI fails with MOI v0.6.2 (see JuliaOpt/LinQuadOptInterface.jl#67). The PR adds a test to catch the error and fixes it.

The error comes from the fact that UnknownSet is neither an AbstractScalarSet nor an AbstractVectorSet hence a method error is thrown, see:
https://github.com/JuliaOpt/MathOptInterface.jl/blob/00b5965ea1491a893af01c0bf8b5501a0f11c4bb/src/constraints.jl#L65-L88

@codecov-io
Copy link

codecov-io commented Oct 25, 2018

Codecov Report

Merging #543 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #543   +/-   ##
=======================================
  Coverage   95.48%   95.48%           
=======================================
  Files          47       47           
  Lines        4783     4783           
=======================================
  Hits         4567     4567           
  Misses        216      216
Impacted Files Coverage Δ
src/Test/modellike.jl 98.07% <100%> (ø) ⬆️

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 00b5965...8e59d3a. Read the comment docs.

test/dummy.jl Outdated
@@ -1,9 +1,19 @@
struct DummyModel <: MOI.ModelLike
# DummyModel{true} implements add_variable and add_constraint
struct DummyModel{A} <: MOI.ModelLike
Copy link
Member

@mlubin mlubin Oct 25, 2018

Choose a reason for hiding this comment

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

What about an abstract dummy model and two concrete subclasses to distinguish between the versions that implement add_variable or not. A boolean type parameter is hard to interpret.

@blegat blegat merged commit c7bd022 into master Oct 26, 2018
@odow odow deleted the bl/unknownset branch March 6, 2019 15:33
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.

4 participants