best dive lower bound#58
Conversation
Josh Robbins (jrobbins11)
commented
Apr 13, 2026
Josh Robbins (jrobbins11)
commented
- added bugfix for computing lower bound during best dive in B&B
- fixed pruning bug
…fixed pruning bug
There was a problem hiding this comment.
Pull request overview
This PR refactors and fixes branch-and-bound bound/pruning behavior, specifically to compute a correct global lower bound during “best dive” search and to improve queue pruning correctness.
Changes:
- Replaces inline lower-bound computation with a dedicated
BranchAndBound::get_lower_bound()helper. - Implements best-dive lower-bound computation by taking the minimum bound over all queued nodes (not just the queue top).
- Updates
PriorityQueuePrunable::prune()to remove all prunable elements and rebuild the heap; adds an accessor for the underlying container to support best-dive lower-bound scanning.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src/BranchAndBound.cpp |
Introduces get_lower_bound() and uses it in the main convergence loop; adds best-dive LB logic. |
include/zonoopt/BranchAndBound.hpp |
Declares the new get_lower_bound() helper. |
include/zonoopt/BnbDataStructures.hpp |
Fixes pruning to remove multiple elements and re-heapifies; adds get_container() accessor. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| else if (this->data.admm_data->settings.search_mode == 0) // best first | ||
| { | ||
| J_min = std::min(this->node_queue.top()->solution.J, min_val_pair.second); | ||
| } | ||
| else if (this->data.admm_data->settings.search_mode == 1) // best dive | ||
| { | ||
| J_min = min_val_pair.second; | ||
| for (const auto& n : this->node_queue.get_container()) | ||
| J_min = std::min(n->solution.J, J_min); | ||
| } |
There was a problem hiding this comment.
In best-first / best-dive branches, min_val_pair.second is used even when valid is false (i.e., no active thread tags). ThreadSafeSet::get_min() returns a default-constructed value when empty, so this can produce an incorrect lower bound (and potentially premature convergence) when J_threads is empty but node_queue is not. Consider handling !valid by computing the bound from node_queue only (or treating the thread bound as +inf) before taking mins.
| // prune queue | ||
| void prune(const T& t) | ||
| { | ||
| auto it_prune = std::find_if(this->c.begin(), this->c.end(), [&](const T& item) | ||
| auto it_prune = std::remove_if(this->c.begin(), this->c.end(), [&](const T& item) | ||
| { | ||
| return this->comp(item, t); | ||
| }); | ||
| if (it_prune != this->c.end()) | ||
| { | ||
| this->c.erase(it_prune, this->c.end()); | ||
| std::make_heap(this->c.begin(), this->c.end(), this->comp); | ||
| } |
There was a problem hiding this comment.
PriorityQueuePrunable::prune() removes items using this->comp(item, t). With NodeCompare, any item with is_priority()==true will never satisfy comp(item, t) when t is non-priority, so priority nodes will not be pruned even if their objective is worse than the incumbent. If pruning is intended to discard all nodes whose lower bound cannot beat the incumbent (regardless of priority), the prune predicate should compare the objective bound directly (or use a comparator that ignores the priority flag for pruning).
| else if (this->data.admm_data->settings.search_mode == 1) // best dive | ||
| { | ||
| J_min = min_val_pair.second; | ||
| for (const auto& n : this->node_queue.get_container()) | ||
| J_min = std::min(n->solution.J, J_min); | ||
| } |
There was a problem hiding this comment.
get_lower_bound() computes the best-dive lower bound by scanning the entire underlying priority queue container while holding pq_mtx. Since this runs in the main monitoring loop, it can block worker threads from pushing/popping and can become a bottleneck as the queue grows. Consider maintaining a separate min-bound (updated on push/pop/prune) or copying out the needed J values under lock and computing the min outside the critical section.
…ottleneck for computing lower bound
