Heuristic Improvements: balance between generation and improvement heuristics #382
Conversation
| context(context_) | ||
| { | ||
| setup(problem_); | ||
| // setup(problem_); |
There was a problem hiding this comment.
Should we remove setup completely?
There was a problem hiding this comment.
There is a bug in load balanced setup, @kaatish is fixing that. This should normally be enabled, I will try if it is resolved with Aatish's recommendation.
| if (settings.heuristic_preemption_callback != nullptr) { | ||
| settings.heuristic_preemption_callback(); | ||
| } | ||
| // FIXME: rarely dual simplex detects infeasible whereas it is feasible. |
There was a problem hiding this comment.
So if heuristics has a feasible solution, but branch and bound says it is infeasible, you take the feasible solution?
There was a problem hiding this comment.
Yes, that's what i wanted to do for now.
cpp/src/dual_simplex/phase2.cpp
Outdated
| for (i_t k = 0; k < delta_z_nz; ++k) { | ||
| const i_t j = delta_z_indices[k]; | ||
| z[j] += step_length * delta_z[j]; | ||
| if (std::isnan(z[j]) || std::isinf(z[j])) { return -1; } |
There was a problem hiding this comment.
This is a performance critical loop. Hopefully the compiler is smart about this, but would you mind doing something like:
f_t zj = z[j] + step_length * delta_z[j];
z[j] = zj;
if (zj != zj || std::isinf(zj)) {
return -1;
}
Also, can you file an issue so that we can track that this should be removed?
There was a problem hiding this comment.
I think isfinite is probably better suited as a single instruction, let me know if it is better.
There was a problem hiding this comment.
Oh you are worried about double access, I am 90% sure compiler will optimize this but sure i can fix this to have single access.
There was a problem hiding this comment.
We might not need this at all because the bound_flipping_ratio test has solved the numerical issue.
There was a problem hiding this comment.
Indeed, we don't need this. Removed.
There was a problem hiding this comment.
Is there a reason why we are limiting the number of threads to 8? It is not better to let be a user controller setting?
There was a problem hiding this comment.
Actually yes, but since we don't have a thread pool now, it is separate for now. Once we have a thread pool we should enable more dynamic strategy.
| #endif | ||
| logger_.set_level(default_level()); | ||
| logger_.flush_on(rapids_logger::level_enum::info); | ||
| logger_.flush_on(rapids_logger::level_enum::debug); |
There was a problem hiding this comment.
Maybe we should keep the flush in the info option unless there is a performance penalty.
There was a problem hiding this comment.
Debug is disabled in production, so there will be no performance penalty.
aliceb-nv
left a comment
aliceb-nv
left a comment
There was a problem hiding this comment.
LGTM engine side, approving; thanks a lot for the great work Akif!
cpp/CMakeLists.txt
Outdated
|
|
||
|
|
||
| option(BUILD_MIP_BENCHMARKS "Build MIP benchmarks" OFF) | ||
| set(BUILD_MIP_BENCHMARKS ON) |
There was a problem hiding this comment.
Debugging leftover I assume :)
On my setup I add --cmake-args=\"-DBUILD_MIP_BENCHMARKS=ON\" to my build.sh command which does the trick
There was a problem hiding this comment.
Makes sense, thanks will do that. I think we should add a build.sh command for this.
| if (nonbasic_entering == -1) { | ||
| // -1,-2 and -3 are reserved for other things | ||
| return -4; | ||
| } |
There was a problem hiding this comment.
This issue is tracked, right?
There was a problem hiding this comment.
I will create an issue thanks!
|
/merge |
…uristics (#382) This PR changes the heuristic structure by creating a natural balance between generation and improvement. The FP/FJ loop now adds solution to the population and only if we have enough diverse solutions we exit the loop and execute the population improvement. The diversity is increased to `sqrt(n_integers)`. The recombiners are run between the current best and all other solutions in the current population, if stagnation is detected in FP/FJ loop and then the loop continues. The bounds prop rounding in the context of FP is also improved. When the dual simplex solution is set, the pdlp is warm started now with both primal and dual solutions. The default tolerance is now 1e-6 absolute tolerance and 1e-12 relative tolerance. This PR includes bug fixes on: - Apperance of inf/nan on `z` vector dual simplex phase2. - Invalid launch dimensions on FJ and hash kernels. - Timer diff and function time limit issues when the solver is run with unlimited time limit. Benchmark results in 10 mins run on H100: - Main branch: 207 feasible solutions and average gap: '28.54', 3 unfinished/crashed - This PR: 213 feasible and average gap: '23.11', 1 unfinished/crushed. (The PR didn't have any crash before merge with main branch) closes #142 closes #374 closes #218 Authors: - Akif ÇÖRDÜK (https://github.com/akifcorduk) Approvers: - Ramakrishnap (https://github.com/rgsl888prabhu) - Alice Boucher (https://github.com/aliceb-nv) URL: #382
6 participants
Description
This PR changes the heuristic structure by creating a natural balance between generation and improvement.
The FP/FJ loop now adds solution to the population and only if we have enough diverse solutions we exit the loop and execute the population improvement. The diversity is increased to
sqrt(n_integers). The recombiners are run between the current best and all other solutions in the current population, if stagnation is detected in FP/FJ loop and then the loop continues. The bounds prop rounding in the context of FP is also improved. When the dual simplex solution is set, the pdlp is warm started now with both primal and dual solutions.The default tolerance is now 1e-6 absolute tolerance and 1e-12 relative tolerance.
This PR includes bug fixes on:
zvector dual simplex phase2.Benchmark results in 10 mins run on H100:
closes #142
closes #374
closes #218