Check primal feasibility after termination failure

[Robbybp]

Quick patch implementing my proposal in #436

[odow]

I made some changes. We should check this for PrimalStatus, not TerminationStatus since we're checking the primal point.

This would cause is_solved_and_feasible to fail because the termination status is not LOCALLY_SOLVED, but you would see NUMERICAL_ERROR and then FEASIBLE_POINT, which is enough to infer what happened. (The solver did not terminate with a stationary point, but it did terminate with a feasible point.)

[odow]

Do we have a trivial example that terminates with :Restoration_Failure?

[codecov]

Codecov Report

Attention: Patch coverage is 94.44444% with 1 line in your changes missing coverage. Please review.

Project coverage is 94.54%. Comparing base (ffa138d) to head (a5aa7c0).
Report is 1 commits behind head on master.

Files with missing lines	Patch %	Lines
src/MOI_wrapper.jl	94.44%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #435      +/-   ##
==========================================
+ Coverage   94.45%   94.54%   +0.08%     
==========================================
  Files           4        4              
  Lines         974      990      +16     
==========================================
+ Hits          920      936      +16     
  Misses         54       54              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

---

🚨 Try these New Features:

• Flaky Tests Detection - Detect and resolve failed and flaky tests

[odow]

Happy?

[Robbybp]

Yes, checking this for PrimalStatus makes more sense than TerminationStatus. LOCALLY_SOLVED does seem a little optimistic for RestorationFailure, but I'd argue that SLOW_PROGRESS or OTHER_ERROR are better descriptions of what's happening than NUMERICAL_ERROR.

From the LOCALLY_SOLVED documentation, "could not find directions for improvement" seems to be as good a description as any for feasible restoration failure.

I'll leave this up to you. I think my preference is to return OTHER_ERROR for a restoration failure, but I'm not sure how big of a deal this is from a backwards compatibility point of view.

[Robbybp]

Do we have a trivial example that terminates with :Restoration_Failure?

This is hard. I don't have an example that does, and an algebraic example would probably be pretty sensitive to numerics or algorithm changes. If anybody has such a problem, I'd be interested to see it. We might be able to hack together some fake problem with bad derivative information in the C interface that does this, but I don't think this helps us test the MOI interface.

[odow]

I'm okay with OTHER_ERROR. SLOW_PROGRESS is also good to me:
https://github.com/jump-dev/MathOptInterface.jl/blob/02dc52c05c315cb99c6392387dc0ca05d75d484b/src/attributes.jl#L2140-L2141

I don't regard it as a breaking change.

[Robbybp]

LGTM