Add logging and time budget for probing in presolve, and increase visibility of presolve rule switch-off#2865
Conversation
…e time statements
…dev_level is nonzero
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## latest #2865 +/- ##
==========================================
+ Coverage 80.35% 80.37% +0.02%
==========================================
Files 348 348
Lines 86522 86600 +78
==========================================
+ Hits 69527 69609 +82
+ Misses 16995 16991 -4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
jajhall
changed the title
|
Closes #2860 |
fwesselm
left a comment
fwesselm
left a comment
There was a problem hiding this comment.
I would like to understand why this particular model is so slow in probing. I think @Opt-Mucca has made some improvements to make probing stop earlier recently. Also, I think I should run the MIP suite to see how this affects performance.
highs/presolve/HPresolve.cpp
Outdated
|
|
||
| iBin_probed++; | ||
| double tt = this->timer->read(); | ||
| if (tt > log_tt + log_tt_interval && iBin_probed > log_iBin_probed) { |
There was a problem hiding this comment.
It seems that nothing in this if block is done when silent = true, so maybe the outer if-check could look like this:
if (!silent && tt > log_tt + log_tt_interval && iBin_probed > log_iBin_probed)
and then the other checks for !silent could be removed.
There was a problem hiding this comment.
No, the block contains the return with Result::kStopped; if the probing time limit is reached. Since that is "lost" in what appears to be just logging tells me that I should refactor this section!
highs/presolve/HPresolve.cpp
Outdated
| double log_tt = tt0; | ||
| HighsInt log_iBin_probed = iBin_probed; | ||
| double time_remaining = options->time_limit - tt0; | ||
| double probing_time_limit = 0.1 * time_remaining; |
There was a problem hiding this comment.
I understand that a time limit may be needed, but I also do not really like hard-coding things. However, having an option may also be too much.
There was a problem hiding this comment.
Agreed, but I felt that something was needed so that users for whom probing is very expensive could get some advantage from it.
Maybe it's better to let probing run to options->time_limit and print a message suggesting that probing in presolve is switched off.
| } | ||
|
|
||
| const bool silent = mipsolver && mipsolver->mipdata_->numRestarts > 0; | ||
| if (options->presolve != kHighsOffString) { |
There was a problem hiding this comment.
if (!silent && options->presolve != kHighsOffString) ?
| if (cliquetable.getSubstitution(i) != nullptr || !domain.isBinary(i)) | ||
| continue; | ||
|
|
||
| iBin_probed++; |
There was a problem hiding this comment.
Would it be worth putting the new code that checks the time limit into a lambda? This could also be done later - it would make reading the probing code easier for me.
highs/presolve/HPresolve.cpp
Outdated
| highsLogUser(options->log_options, HighsLogType::kInfo, | ||
| "%" HIGHSINT_FORMAT " rows, %" HIGHSINT_FORMAT | ||
| " cols, %" HIGHSINT_FORMAT " nonzeros %s\n", | ||
| " cols, %" HIGHSINT_FORMAT " nonzeros %s\n", |
There was a problem hiding this comment.
Is the extra space needed?
There was a problem hiding this comment.
No, I don't know how that's crept in
highs/presolve/HPresolve.cpp
Outdated
| model->sense_ = ObjSense::kMinimize; | ||
| } | ||
|
|
||
| const bool silent = mipsolver && mipsolver->mipdata_->numRestarts > 0; |
There was a problem hiding this comment.
Would it be worth making this a method in HPresolve.h? Maybe not, but anyway:
bool isLogSilent() const {
return mipsolver && mipsolver->mipdata_->numRestarts > 0;
}
|
Thanks @fwesselm ! |
…ts > 0; by silentLog()
… comments; updated FEATURES.md
2 participants
The potentially prohibitive high cost of probing means that there should be some logging so that HiGHS doesn't go "quiet", and this has been added.
The high cost is mitigated by giving probing a budget of 10% of
time_limit. It may make the MIP solver non-deterministic iftime_limitis finite, but that's better than it "never" returning.Since this may lead users to want to switch off probing, the ability to switch off more advanced/expensive presolve rules is now visible.