BUG: Gracefully fail for OOM (Out of Memory) in Presolve #1467
Conversation
No change in C-API, maps to existing errors
There was a problem hiding this comment.
Thanks, but there are a couple of issues here.
Firstly, return Result::kPrimalInfeasible; cannot be returned, as infeasibility hasn't been determined. The postsolve_stack data should be cleared, and presolve should return as if no reductions can be performed.
Secondly, I'm unclear why you're thinking it should somehow trigger HighsPresolveStatus::kReducedToEmpty. It's not reduced to empty if presolve fails. For me, it should trigger HighsPresolveStatus::kNotReduced, in which case there's no call to postsolve, even if the postsolve stack isn't cleared.
It's correct to treat kReducedToEmpty and kReduced the same way. If the model is reduced to empty, then solution_ and basis_ are trivially empty, so postsolve builds on this. If presolve yields kReduced (ie reductions identified, but to a non-empty LP), then the LP solver applied to the reduced LP yields a non-empty solution_ and basis_.
I'm happy to merge this into a development branch for me to fix the issues above, before merging to latest.
Is there an existing Presolving model
ERROR: Problem too large for Presolve, try without
Specific error raised:
Failed to resize the vector. Requested new size: -2142950640. Size to add is 24. Current size: 2152016632.
presolve_.run() returns status: Reduced
Presolve : Reductions: rows 499999(-1); columns 231(-269); elements 115500000(-134500000)
Solving the presolved LP
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
test_oomkill is a Catch v2.13.8 host application.
Run with -? for options
-------------------------------------------------------------------------------
linprog_oom
-------------------------------------------------------------------------------
../check/TestOOMKill.cpp:11
...............................................................................
../check/TestOOMKill.cpp:11: FAILED:
{Unknown expression after the reported line}
due to a fatal error condition:
SIGSEGV - Segmentation violation signal
Makes sense. This was a misunderstanding on my part, on a computer with more memory and with 64-bit integers, the presolve phase reduced to empty, so I thought there might be an optimized path for it, but that would be incorrect here.
That'd be great, I've pushed an test in the C++ code-base as well. To run it: It seems like there should be a new status flag which might be needed, perhaps something like: |
|
Thanks. Overall, this is another very useful contribution to HiGHS from you. I will look at this in a week's time. I'm hard pressed with work this week |
Awesome, thanks so much. For now, I've added a new error code so it goes from Feel free to make changes as needed next week :) |
As noted in the title. Essentially this is a fix for scipy/scipy#15888.
Although this does introduce
HighsExceptionsit requires no changes to theCAPI or any other part of the code, this is just to propagate the error internally, and it maps (within the C++ part itself) to existing error codes, raisingInfeasible.Some more context. For an unreasonably large problem (which none the less has a trivial solution which can be found without Presolve):
This previously triggered the out of memory killer and crashes. Now it will instead report:
This will also save similar crashes across the rest of highs, I used the
pythonexample because that was where it was reported first.FWIW (maybe something for the Highs team) this problem has a trivial solution:
So my understanding is that this should somehow trigger
HighsPresolveStatus::kReducedToEmptyand then bailout (special cased) instead of entering the PostSolve phase (the length error comes from thePostSolveStack). Currently the code treatskReducedToEmptyandkReducedthe same way:However, I think that can be dealt with later (if at all), as that would be an improvement (faster runtime than the current approach), while this PR fixes a bug (the crashing of Highs).