lanl-ansi / Juniper.jl

A JuMP-based Nonlinear Integer Program Solver
https://lanl-ansi.github.io/Juniper.jl/stable/
MIT License
179 stars 22 forks source link

Incorrect PrimalStatus if termination is not optimal #249

Closed odow closed 2 years ago

odow commented 2 years ago

Juniper reports the PrimalStatus as INFEASIBLE_POINT when the problem is not optimal: https://github.com/lanl-ansi/Juniper.jl/blob/053661ebc023ff4534210207f9ca40c0ff42fca0/src/MOI_wrapper/results.jl#L29-L39

This is incorrect, because sometimes the point is actually feasible. If we can prove feasibility, it should be FEASIBLE_POINT, otherwise it should be something like UNKNOWN_RESULT_STATUS

First reported on Discourse: https://discourse.julialang.org/t/obtaining-sub-optimal-results-from-the-model-when-the-time-limit-is-reached/85635

ccoffrin commented 2 years ago

A very good find! Kind of shocking this has been in the wild for so long without noticing.

I don't know MOI well enough to suggest the best fix, but as long as the inner NLP solver returns a suitable "solved" status and all discrete variables are integral within the tolerance, we should mark the current incumbent as FEASIBLE_POINT.

odow commented 2 years ago

Taking a look now.

Wikunia commented 2 years ago

I'm a bit confused by this to be honest. The fix in #250 doesn't change when FEASIBLE_POINT is returned, correct? (Just talking about primal status) It changes the infeasible one to unknown. Isn't it an infeasible point if it's not a feasible point by the definition of feasibility that we are using? If it's actually feasible and the check at state_is_optimal returns false isn't the check wrong?

odow commented 2 years ago

Yeah, ideally the check in PrimalStatus would be about feasibility, not whether the optimal status has been found. But it's also okay to report feasible when you find the optimal solution and "I have no idea" if something else happened. That might be because the problem is infeasible, or it might be because of a time limit, and some other numerical issue.

odow commented 2 years ago

The DualStatus return was just always wrong in every situation.

Wikunia commented 2 years ago

Understood thanks for the explanation.