Repository navigation
Support MOI.ObjectiveBound() - #117
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #117 +/- ##
==========================================
+ Coverage 98.31% 98.43% +0.12%
==========================================
Files 5 5
Lines 652 703 +51
==========================================
+ Hits 641 692 +51
Misses 11 11 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| ) where {T<:Integer} | ||
| # all variables and coefficients are integer, so the objective is integer, | ||
| # and the bound can be rounded towards it | ||
| return T(sense == MOI.MAX_SENSE ? floor(bound) : ceil(bound)) |
There was a problem hiding this comment.
I don’t think this is safe. HiGHS might return 0.9999999999 as the bound when the true value is 1.0
There was a problem hiding this comment.
Probably we can check if isapprox(round(T, x), x) is true and only round it if it's the case. Note that isapprox(0, x) is false for all x so we need to use a nonzero atol to make it work. If MiniZinc returns 0.5, we just want to leave it as is
There was a problem hiding this comment.
Hmm yes I hadn't considered that.
If
MiniZincreturns 0.5, we just want to leave it as is
I think this wouldn't work for GenericModel{Int}, since in JuMP, the return value of objective_bound(model) would be an Int (in this case)
Would changing objective_bound(model::GenericModel{T})::Union{T,Vector{T}} where {T} to remove its return type be acceptable?
Also, what would a good choice of atol be?
There was a problem hiding this comment.
Use the same as the default rtol, Base.rtoldefault(T)
There was a problem hiding this comment.
Since T<:Integer and Base.roldefault(Integer)=0, I assume you meant Base.rtoldefault(typeof(bound)), which I have implemented
9a30247 to
506decb
Compare
| # a bound within tolerance of an integer is snapped to it. | ||
| # otherwise it is rounded towards the objective. | ||
| # since isapprox(0, x) is always false, an atol must be provided | ||
| if isapprox(round(bound), bound; atol = Base.rtoldefault(typeof(bound))) |
There was a problem hiding this comment.
| if isapprox(round(bound), bound; atol = Base.rtoldefault(typeof(bound))) | |
| if isapprox(round(T, bound), bound; atol = Base.rtoldefault(typeof(bound))) |
| if isapprox(round(bound), bound; atol = Base.rtoldefault(typeof(bound))) | ||
| return round(T, bound) | ||
| end | ||
| return sense == MOI.MAX_SENSE ? floor(T, bound) : ceil(T, bound) |
There was a problem hiding this comment.
That's only true if the objective is affine right ? Are we sure there are no nonlinear function that can produce non-integer values ?
There was a problem hiding this comment.
Maybe we should just through an InexactError here
|
I'm not convinced with the rounding here. I think we might just be better off reporting what the solver tells us. |
|
Yes, I am less convinced now as well. I hadn't even considered nonlinear expressions. If we report straight from MiniZinc, then it may return a I'm not quite sure the solution here, happy to take suggestions. Maybe just throw |
Yes, we should start with the most conservative approach and then see if there are use case for rounding. So we can start with a simple |
|
I've dropped the rounding, stuck with |
| function MOI.get(model::Optimizer{T}, attr::MOI.ObjectiveBound) where {T} | ||
| if model.solver_status == "OPTIMAL" | ||
| # completed the search, so the bound is the objective value | ||
| return MOI.get(model, MOI.ObjectiveValue()) |
There was a problem hiding this comment.
We should return a value here only if the solver actually found something called "objective bound". It's okay to throw an error when the solver said "optimal" but didn't provide a bound.
Adds support for
MOI.ObjectiveBound.I've made this parametric, so it should come out the same type as
primal_objective.To achieve this, we have to add the flag
-s, which ask MiniZinc to report statistics. This goes to the output file, so users don't see it anyways.If a solver can report a bound (cpsat, highs), then it gets reported there.
We then regex capture it, convert to float, then convert to
T, rounding it in the direction of the objective if we need to.Thanks!