Improve error message when querying a variable bound that does not exist #1587
Conversation
| @@ -257,7 +257,9 @@ function has_lower_bound(v::VariableRef) | |||
| end | |||
|
|
|||
| function lower_bound_index(v::VariableRef) | |||
| @assert has_lower_bound(v) # TODO error message | |||
| if !has_lower_bound(v) | |||
| error("Variable $(v) does not have a lower bound.") | |||
There was a problem hiding this comment.
This might be misleading. Consider a polyhedral feasible set, if you eliminate all variables except v then it might be a set that is bounded from below.
Maybe lower bound constraint ?
There was a problem hiding this comment.
This error message will almost always be called when users run JuMP.lower_bound(x) if x is missing a lower bound.
There might be other constraints that enforce a lower bound on the variable, but that is a different kettle of fish.
There was a problem hiding this comment.
that is a different kettle of fish.
That is clear for us, but is that clear for a user that has just started using JuMP and has no idea what JuMP.lower_bound is ?
There was a problem hiding this comment.
There was a problem hiding this comment.
Who reads the doc if it is not thrown on your face :-P
There was a problem hiding this comment.
Also a user that has done @constraint model x >= 0 will not understand why there is no lower bound.
There was a problem hiding this comment.
This relates to #1586. I feel that if the user writes @constraint, we should add a constraint (e.g., a row) to the problem.
Then we make "variable bound" mean a bound that was introduced in @variable or via JuMP.set_lower_bound and JuMP.set_upper_bound.
There was a problem hiding this comment.
I think the error message is sufficiently clear given that has_lower_bound is a documented concept in JuMP.
Codecov Report
@@ Coverage Diff @@
## master #1587 +/- ##
=========================================
+ Coverage 90.8% 90.8% +<.01%
=========================================
Files 28 28
Lines 3696 3698 +2
=========================================
+ Hits 3356 3358 +2
Misses 340 340
Continue to review full report at Codecov.
|
Closes #1509