Skip to content

Conversation

shadiakiki1986
Copy link
Contributor

@shadiakiki1986 shadiakiki1986 commented Nov 6, 2020

Tests for input nan, for lower-bound already set, and for upper-bound already set. This was prompted by PR 230 in Ipopt (link): Fix error in add_constraint NaN handling for EqualTo

I figured I'd add a unit test for the checks here, and they'd be useful for other solvers as well. I checked ECOS and SCS, but they don't implement add_constraint. OTOH, GLPK.jl does implement it (ref), but it needs this new test to be added to the unit test array (ref).

…nd already set, and for upper-bound already set
@odow
Copy link
Member

odow commented Nov 6, 2020

So we already check for finiteness at the JuMP level: jump-dev/JuMP.jl#2272

I don't know if we want to make every solver check every coefficient for a NaN. That seems like a lot of overhead.

@odow
Copy link
Member

odow commented Dec 4, 2020

Closing since there are already tests for LowerBoundAlreadySet:

function set_lower_bound_twice(model::MOI.ModelLike, T::Type)
MOI.empty!(model)
@test MOI.is_empty(model)
x = MOI.add_variable(model)
f = MOI.SingleVariable(x)
lb = zero(T)
@test MOI.supports_constraint(model, MOI.SingleVariable, MOI.GreaterThan{T})
sets = [MOI.EqualTo(lb), MOI.Interval(lb, lb),
MOI.Semicontinuous(lb, lb), MOI.Semiinteger(lb, lb)]
set2 = MOI.GreaterThan(lb)
for set1 in sets
if !MOI.supports_constraint(model, MOI.SingleVariable, typeof(set1))
continue
end
ci = MOI.add_constraint(model, f, set1)
err = MOI.LowerBoundAlreadySet{typeof(set1), typeof(set2)}(x)
@test_throws err MOI.add_constraint(model, f, set2)
MOI.delete(model, ci)
ci = MOI.add_constraint(model, f, set2)
err = MOI.LowerBoundAlreadySet{typeof(set2), typeof(set1)}(x)
@test_throws err MOI.add_constraint(model, f, set1)
MOI.delete(model, ci)
end
end

and I don't think we need all solvers checking for isnan on every element. JuMP already does this, and either the lower-level solver should do it, or the person choosing to bypass JuMP and use MOI should do it.

@odow odow closed this Dec 4, 2020
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.

2 participants