[BUG] Fixed starting variable bounds for diving #474
Conversation
nguidotti
added
bug
|
@nguidotti Is this PR meant for 25.10 ? |
|
Yes. This allows B&B to solve the MIPLIB benchmark more consistently. Akif and Alice noticed that we had a regression in some tests, and this PR should help fix it. |
nguidotti
changed the title
| // Note that we do not know which thread will execute the | ||
| // `exploration_ramp_up` task, so we allow to any thread | ||
| // to repair the heuristic solution. | ||
| repair_heuristic_solutions(); |
There was a problem hiding this comment.
The original intent was just to have a single thread attempt to repair the heuristic solution. We don't want multiple threads to try to repair the same heuristic solution. Is there a way to ensure that only a single thread repairs?
There was a problem hiding this comment.
There is a mutex for retrieving the solutions in the repair queue. Once all solutions are copied, the repair queue is cleared and the mutex is released (line 390 to 396):
std::vector<std::vector<f_t>> to_repair;
mutex_repair_.lock();
if (repair_queue_.size() > 0) {
to_repair = repair_queue_;
repair_queue_.clear();
}
mutex_repair_.unlock();
The other threads will see the repair queue is empty and then move on.
There was a problem hiding this comment.
Note that: the repair will probably be needed much less since now the tolerances are compatible.
There was a problem hiding this comment.
When the heuristic thread injects the solution to the B&B, it would not be easier for it to attempt to repair immediately instead of adding to a queue? The only caveat is the heuristic thread can only repair it after the root node solution.
|
|
||
| // Set the correct bounds for the leaf problem | ||
| leaf_problem.lower = original_lp_.lower; | ||
| leaf_problem.upper = original_lp_.upper; |
There was a problem hiding this comment.
Don't you need these lines to restore the bounds to that of the original problem? Since you may modified the bounds of the leaf_problem previously?
| f_t now = toc(stats_.start_time); | ||
| f_t time_since_last_log = stats_.last_log == 0 ? 1.0 : toc(stats_.last_log); | ||
|
|
||
| if (omp_get_thread_num() == 0) { |
There was a problem hiding this comment.
Don't we only want one thread to print?
There was a problem hiding this comment.
In the tasking model, one thread spawns a task, it goes to a pool and then it is distributed to any available thread. In the ramp-up phase, it will have more threads than tasks, such that some will execute the corresponding task, while others are idling. We do not know which thread will be in each group.
Hence, if we restrict logging to T0 (or any other thread), then the log will only appear when it is active, which may only happen near the end.
There was a problem hiding this comment.
When it is time to report, the code is atomically check if the node is the last one explored. If it is the case, then the thread will print the log. Otherwise, it will skip the report and allow another thread to do it instead.
| return; | ||
| } | ||
|
|
||
| // Set the correct bounds for the leaf problem |
There was a problem hiding this comment.
I see. You are restoring here the bounds here. Is there a reason to move it from the original spot?
There was a problem hiding this comment.
There are two reasons:
- The initial bounds for the best first and diving are different. Best-first uses the bounds of the original problem, while diving uses the bounds of the starting node (we need to keep the bounds separated since the node was detached from the main tree).
- In Propagate the bounds from the parent to the child nodes #473 , we only need to change the bounds after reaching the bottom of the branch.
|
@nguidotti can you walk me through the changes tomorrow before merging? |
| // Note that we do not know which thread will execute the | ||
| // `exploration_ramp_up` task, so we allow to any thread | ||
| // to repair the heuristic solution. | ||
| repair_heuristic_solutions(); |
There was a problem hiding this comment.
Note that: the repair will probably be needed much less since now the tolerances are compatible.
|
/merge |
4 participants
This PR also fixes the starting bounds for the diving subtrees. As these nodes are detached from the main B&B tree, the bounds from the parent were lost. Now, the bounds are also kept when inserting the nodes in the diving queue.
Checklist