-
Notifications
You must be signed in to change notification settings - Fork 92
Add concurrent lp solve and crossover at root node #602
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: release/25.12
Are you sure you want to change the base?
Changes from all commits
6f88f67
94307c9
c0f5886
3cb93e7
3eb950b
4a1cf94
3229b0c
49c97ba
d333846
0351a83
00a89fe
ed71447
bfe9be9
2a9c994
32c6edb
4764d51
b6aad5d
1c0d139
a198738
031b7de
55de62d
63d8b87
68361a5
dcca90b
4a10885
32d984a
94182cc
bc002c4
26ccae5
093cbcb
ca7c9d3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,6 +8,7 @@ | |
| #include <omp.h> | ||
| #include <algorithm> | ||
| #include <dual_simplex/branch_and_bound.hpp> | ||
| #include <dual_simplex/crossover.hpp> | ||
| #include <dual_simplex/initial_basis.hpp> | ||
| #include <dual_simplex/logger.hpp> | ||
| #include <dual_simplex/mip_node.hpp> | ||
|
|
@@ -1059,34 +1060,113 @@ mip_status_t branch_and_bound_t<i_t, f_t>::solve(mip_solution_t<i_t, f_t>& solut | |
|
|
||
| root_relax_soln_.resize(original_lp_.num_rows, original_lp_.num_cols); | ||
|
|
||
| settings_.log.printf("Solving LP root relaxation\n"); | ||
| settings_.log.printf("Waiting for PDLP root relaxation\n"); | ||
|
|
||
| simplex_solver_settings_t lp_settings = settings_; | ||
| lp_settings.inside_mip = 1; | ||
| lp_status_t root_status = solve_linear_program_advanced( | ||
| original_lp_, stats_.start_time, lp_settings, root_relax_soln_, root_vstatus_, edge_norms_); | ||
| stats_.total_lp_iters = root_relax_soln_.iterations; | ||
| stats_.total_lp_solve_time = toc(stats_.start_time); | ||
| if (root_status == lp_status_t::INFEASIBLE) { | ||
| settings_.log.printf("MIP Infeasible\n"); | ||
| // FIXME: rarely dual simplex detects infeasible whereas it is feasible. | ||
| // to add a small safety net, check if there is a primal solution already. | ||
| // Uncomment this if the issue with cost266-UUE is resolved | ||
| // if (settings.heuristic_preemption_callback != nullptr) { | ||
| // settings.heuristic_preemption_callback(); | ||
| // } | ||
| return mip_status_t::INFEASIBLE; | ||
| } | ||
| if (root_status == lp_status_t::UNBOUNDED) { | ||
| settings_.log.printf("MIP Unbounded\n"); | ||
| if (settings_.heuristic_preemption_callback != nullptr) { | ||
| settings_.heuristic_preemption_callback(); | ||
| if (!is_main_thread()) { | ||
| lp_status_t root_status = solve_linear_program_advanced( | ||
| original_lp_, stats_.start_time, lp_settings, root_relax_soln_, root_vstatus_, edge_norms_); | ||
| stats_.total_lp_iters = root_relax_soln_.iterations; | ||
| stats_.total_lp_solve_time = toc(stats_.start_time); | ||
|
|
||
|
Comment on lines
1060
to
+1072
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chainAvoid unbounded busy-spin on The main-thread path now does: settings_.log.printf("Waiting for PDLP root relaxation\n");
// ...
while (!root_relaxation_solution_set_.load(std::memory_order_acquire)) {
continue;
}This pure spin loop will:
For robustness, consider at least:
Also, make the contract explicit (in the header or nearby comment) that all PDLP paths must eventually call the setter that flips To validate the wiring, please grep for all writes to Also applies to: 1111-1136 🏁 Script executed: #!/bin/bash
rg -n "root_relaxation_solution_set_" cppLength of output: 347 I need to see more context to verify the reviewer's concerns about the deadlock risk. Let me examine the spin loop and callback invocation sites. Length of output: 520 🏁 Script executed: #!/bin/bash
# Show the callback setter context (lines 135-145)
sed -n '135,145p' cpp/src/dual_simplex/branch_and_bound.hppLength of output: 449 🏁 Script executed: #!/bin/bash
# Search for PDLP solver invocation to understand when callback is called
rg -n "pdlp|PDLP" cpp/src/dual_simplex/branch_and_bound.cpp -A 2 -B 2 | head -100Length of output: 214 🏁 Script executed: #!/bin/bash
# Look for error handling or time-limit paths that might involve root_relaxation
rg -n "time.limit|error|timeout|fail" cpp/src/dual_simplex/branch_and_bound.cpp -C 2 | head -80Length of output: 3311 🏁 Script executed: #!/bin/bash
# Check if there's a destructor or cleanup code
rg -n "~|destructor" cpp/src/dual_simplex/branch_and_bound.hppLength of output: 0 🏁 Script executed: #!/bin/bash
# Check for any thread join or wait logic in main thread
rg -n "join|detach|thread" cpp/src/dual_simplex/branch_and_bound.cpp -C 2 | head -100Length of output: 4639 🏁 Script executed: #!/bin/bash
# Look at the section mentioned in "Also applies to: 1111-1136"
sed -n '1111,1136p' cpp/src/dual_simplex/branch_and_bound.cppLength of output: 1206 🏁 Script executed: #!/bin/bash
# Search for where the callback/lambda capturing root_relaxation_solution_set_ is registered
rg -n "root_relaxation_solution_set_|set_root_relax" cpp/src/dual_simplex/branch_and_bound.cpp -B 5 -A 5 | head -150Length of output: 592 I need to find where the callback method is defined and all sites that invoke it. Length of output: 1431 🏁 Script executed: #!/bin/bash
# Search for all invocations of the root relaxation setter callback
rg -n "set_root_relax|set_root_relax_soln" cpp/src/dual_simplex/ -B 3 -A 1Length of output: 491 🏁 Script executed: #!/bin/bash
# Search for PDLP configuration, callback setup, or lambda capture
rg -n "pdlp_solver|pdlp_settings|AddCallback|callback|set_root" cpp/src/dual_simplex/branch_and_bound.cpp -B 2 -A 2 | head -150Length of output: 3680 🏁 Script executed: #!/bin/bash
# Check if there's error handling in the non-main thread path (lines 1067-1110)
sed -n '1067,1110p' cpp/src/dual_simplex/branch_and_bound.cppLength of output: 1969 🏁 Script executed: #!/bin/bash
# Search for where PDLP solver is invoked and where callbacks might be registered
rg -n "solve_linear_program_advanced|pdlp" cpp/src/dual_simplex/ --type=hpp --type=cpp -B 5 -A 5 | head -200Length of output: 27 I need to find where Length of output: 312 🏁 Script executed: #!/bin/bash
# Correct syntax and search for PDLP solver setup
rg -n "solve_linear_program_advanced" cpp/src/dual_simplex/Length of output: 727 🏁 Script executed: #!/bin/bash
# Look for callback mechanism or PDLP configuration
rg -n "callback|Callback" cpp/src/dual_simplex/branch_and_bound.hpp | head -30Length of output: 46 🏁 Script executed: #!/bin/bash
# Check main thread detection and thread spawning
sed -n '1030,1080p' cpp/src/dual_simplex/branch_and_bound.cppLength of output: 1993 🏁 Script executed: #!/bin/bash
# Search for where PDLP might be set up or where branch_and_bound is used as callback
rg -n "branch_and_bound" cpp/src/dual_simplex/ --type cpp -lLength of output: 114 🏁 Script executed: #!/bin/bash
# Look for files that might setup PDLP with callback
find cpp/src -name "*.cpp" -o -name "*.h" | xargs grep -l "set_root_relaxation_solution" 2>/dev/nullLength of output: 0 The setter is never called within the B&B code itself—it must be invoked from external PDLP worker. Let me trace the callback mechanism. Length of output: 3592 🏁 Script executed: #!/bin/bash
# Search for branch_and_bound usage in solve.cpp
rg -n "branch_and_bound|set_root_relaxation" cpp/src/dual_simplex/solve.cpp -B 3 -A 3Length of output: 949 🏁 Script executed: #!/bin/bash
# Look for PDLP or callback context setup
rg -n "pdlp|PDLP|context|Context|callback" cpp/src/dual_simplex/solve.cpp | head -50Length of output: 0 🏁 Script executed: #!/bin/bash
# Check if branch_and_bound is passed to solve functions or if there's threading
rg -n "thread|Thread|PDLP|pdlp|worker" cpp/src/dual_simplex/branch_and_bound.cpp | head -40Length of output: 1930 🏁 Script executed: #!/bin/bash
# Look for any MIP root solve entry point that might interface with external PDLP
rg -n "root.*relax|mip_status_t.*solve|solve.*mip" cpp/src/dual_simplex/branch_and_bound.cpp | head -30Length of output: 2162 Based on my verification, the reviewer's concern is confirmed as a legitimate major issue. Here's what I found: Key findings:
Avoid unbounded busy-spin on The main-thread path (line 1112) enters a pure spin loop with no safeguards: while (!root_relaxation_solution_set_.load(std::memory_order_acquire)) {
continue;
}This will deadlock indefinitely if the external PDLP worker never calls Required fixes:
Also verify that all external PDLP error handlers guarantee invoking the setter before returning control to B&B. 🤖 Prompt for AI Agents |
||
| if (root_status == lp_status_t::INFEASIBLE) { | ||
| settings_.log.printf("MIP Infeasible\n"); | ||
| // FIXME: rarely dual simplex detects infeasible whereas it is feasible. | ||
| // to add a small safety net, check if there is a primal solution already. | ||
| // Uncomment this if the issue with cost266-UUE is resolved | ||
| // if (settings.heuristic_preemption_callback != nullptr) { | ||
| // settings.heuristic_preemption_callback(); | ||
| // } | ||
| return mip_status_t::INFEASIBLE; | ||
| } | ||
| if (root_status == lp_status_t::UNBOUNDED) { | ||
| settings_.log.printf("MIP Unbounded\n"); | ||
| if (settings_.heuristic_preemption_callback != nullptr) { | ||
| settings_.heuristic_preemption_callback(); | ||
| } | ||
| return mip_status_t::UNBOUNDED; | ||
| } | ||
| return mip_status_t::UNBOUNDED; | ||
| } | ||
|
|
||
| if (root_status == lp_status_t::TIME_LIMIT) { | ||
| status_ = mip_exploration_status_t::TIME_LIMIT; | ||
| return set_final_solution(solution, -inf); | ||
| if (root_status == lp_status_t::TIME_LIMIT) { | ||
| status_ = mip_exploration_status_t::TIME_LIMIT; | ||
| return set_final_solution(solution, -inf); | ||
| } | ||
| } else { | ||
| i_t validation_iters = 0; | ||
| // edge_norms_.clear(); | ||
| // settings_.set_log(true); | ||
| // dual::status_t lp_status = dual_phase2(2, | ||
| // 0, | ||
| // stats_.start_time, | ||
| // original_lp_, | ||
| // settings_, | ||
| // root_vstatus_, | ||
| // root_relax_soln_, | ||
| // validation_iters, | ||
| // edge_norms_); | ||
| // settings_.log.printf("Validation iterations root relaxation: %d\n", validation_iters); | ||
| // exit(0); | ||
|
|
||
| // Wait for the root relaxation solution to be sent by the diversity manager | ||
| while (!root_relaxation_solution_set_.load(std::memory_order_acquire)) { | ||
| continue; | ||
| } | ||
| stats_.total_lp_iters = root_relax_soln_.iterations; | ||
| stats_.total_lp_solve_time = toc(stats_.start_time); | ||
| // Crush the root relaxation solution on converted user problem | ||
| std::vector<f_t> crushed_root_x; | ||
| crush_primal_solution( | ||
| original_problem_, original_lp_, root_relax_soln_.x, new_slacks_, crushed_root_x); | ||
| std::vector<f_t> crushed_root_y; | ||
| std::vector<f_t> crushed_root_z; | ||
|
|
||
| f_t dual_res_inf = crush_dual_solution(original_problem_, | ||
| original_lp_, | ||
| new_slacks_, | ||
| root_relax_soln_.y, | ||
| root_relax_soln_.z, | ||
| crushed_root_y, | ||
| crushed_root_z); | ||
| settings_.log.printf("Dual residual inf: %e\n", dual_res_inf); | ||
|
|
||
| root_relax_soln_.x = crushed_root_x; | ||
| root_relax_soln_.y = crushed_root_y; | ||
| root_relax_soln_.z = crushed_root_z; | ||
|
|
||
| // Call crossover on the crushed solution | ||
| lp_solution_t<i_t, f_t> crossover_solution(original_lp_.num_rows, original_lp_.num_cols); | ||
| crossover_status_t crossover_status = crossover(original_lp_, | ||
| settings_, | ||
| root_relax_soln_, | ||
| stats_.start_time, | ||
| crossover_solution, | ||
| root_vstatus_); | ||
| settings_.log.printf("Crossover status: %d\n", crossover_status); | ||
|
|
||
| if (crossover_status == crossover_status_t::NUMERICAL_ISSUES) { | ||
| settings_.log.printf("MIP Infeasible\n"); | ||
| // FIXME: rarely dual simplex detects infeasible whereas it is feasible. | ||
| // to add a small safety net, check if there is a primal solution already. | ||
| // Uncomment this if the issue with cost266-UUE is resolved | ||
| // if (settings.heuristic_preemption_callback != nullptr) { | ||
| // settings.heuristic_preemption_callback(); | ||
| // } | ||
| return mip_status_t::INFEASIBLE; | ||
| } | ||
| if (crossover_status == crossover_status_t::NUMERICAL_ISSUES) { | ||
| settings_.log.printf("MIP Unbounded\n"); | ||
| if (settings_.heuristic_preemption_callback != nullptr) { | ||
| settings_.heuristic_preemption_callback(); | ||
| } | ||
| return mip_status_t::UNBOUNDED; | ||
| } | ||
|
|
||
| if (crossover_status == crossover_status_t::TIME_LIMIT || | ||
| crossover_status == crossover_status_t::CONCURRENT_LIMIT) { | ||
| status_ = mip_exploration_status_t::TIME_LIMIT; | ||
| return set_final_solution(solution, -inf); | ||
| } | ||
| } | ||
|
|
||
| assert(root_vstatus_.size() == original_lp_.num_cols); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix duplicated
NUMERICAL_ISSUEScondition and unreachable “Unbounded” branch.In the main-thread root path, the crossover status handling has a clear copy‑paste bug:
The second
ifis unreachable and the intended status for the “MIP Unbounded” path is almost certainly something else (e.g., a distinct crossover enum for unboundedness or a feasibility variant). As written, you will never hit the “MIP Unbounded” path, and allNUMERICAL_ISSUESare treated as infeasible.Please correct the second condition to the appropriate status (and/or use
else if) so that infeasibility vs. unboundedness is distinguished as intended, and add a brief comment documenting the mapping betweencrossover_status_tand MIP status here.Also applies to: 1061-1095, 1111-1169
🧰 Tools
🪛 Clang (14.0.6)
[error] 8-8: 'omp.h' file not found
(clang-diagnostic-error)