-
Notifications
You must be signed in to change notification settings - Fork 84
Reuse of the basis factorization from the parent node #492
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: branch-25.12
Are you sure you want to change the base?
Reuse of the basis factorization from the parent node #492
Conversation
…ing basis_update_mpf_t.
Remove unused dependency raft-dask Authors: - Ramakrishnap (https://github.com/rgsl888prabhu) Approvers: - Ishika Roy (https://github.com/Iroy30) - Trevor McKay (https://github.com/tmckayus) URL: NVIDIA#475
…blem (NVIDIA#477) Fix an issue where the free variable info from the original problem was being used on the folded problem. This was leading to undefined memory access. Thanks to Burcin for reporting the bug and Hugo for determining the issue. Authors: - Chris Maes (https://github.com/chris-maes) Approvers: - Hugo Linsenmaier (https://github.com/hlinsen) URL: NVIDIA#477
This PR is a follow-up for PR NVIDIA#272 to add documentation and additional tests Authors: - Ramakrishnap (https://github.com/rgsl888prabhu) - Hugo Linsenmaier (https://github.com/hlinsen) - Chris Maes (https://github.com/chris-maes) Approvers: - Ishika Roy (https://github.com/Iroy30) - Trevor McKay (https://github.com/tmckayus) - Chris Maes (https://github.com/chris-maes) URL: NVIDIA#449
Minor tweaks to the barrier log. Authors: - Chris Maes (https://github.com/chris-maes) Approvers: - Hugo Linsenmaier (https://github.com/hlinsen) URL: NVIDIA#478
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. Authors: - Nicolas L. Guidotti (https://github.com/nguidotti) Approvers: - Akif ÇÖRDÜK (https://github.com/akifcorduk) - Chris Maes (https://github.com/chris-maes) URL: NVIDIA#474
CUDSS does not throw on OOM and even though we check for the return status. There was a sticky cuda error on any successive CUDA API call. A workaround is to throw if `cudaMallocAsync` fails in the device mem handler and catch that error in the solve method. Authors: - Hugo Linsenmaier (https://github.com/hlinsen) Approvers: - Chris Maes (https://github.com/chris-maes) URL: NVIDIA#483
…VIDIA#484) @tmckayus reported that in concurrent mode, he was seeing lots of barrier numerical status. This was due to incorrectly setting the status to NUMERIC, when barrier was stopped by PDLP or concurrent. This PR fixes the issue, by checking for concurrent limit first. Logs are also changed for consistency. Authors: - Chris Maes (https://github.com/chris-maes) Approvers: - Hugo Linsenmaier (https://github.com/hlinsen) URL: NVIDIA#484
## Issue Authors: - Cindy Wilkinson (https://github.com/cwilkinson76) - Ramakrishnap (https://github.com/rgsl888prabhu) - Chris Maes (https://github.com/chris-maes) Approvers: - Trevor McKay (https://github.com/tmckayus) URL: NVIDIA#432
Print scaling information and issue a warning if the range of coefficients is greater than 10^6. This PR also prints out problem size information before scaling information. It also changes 'Running' to 'Reading' so that users understand that we are still reading the file. Authors: - Chris Maes (https://github.com/chris-maes) Approvers: - Hugo Linsenmaier (https://github.com/hlinsen) URL: NVIDIA#488
TODO: Check reason of mismtach Authors: - Hugo Linsenmaier (https://github.com/hlinsen) Approvers: - Chris Maes (https://github.com/chris-maes) URL: NVIDIA#489
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.
Greptile Overview
Greptile Summary
This PR implements basis factorization reuse optimization in the dual simplex solver for branch-and-bound mixed-integer programming. The key change allows child nodes in the search tree to reuse the LU factorization from their parent node, avoiding expensive recomputation when solving LP relaxations. This is achieved by extracting factorization logic from the tightly-coupled dual_phase2
function and creating new variants that accept external basis_update_mpf_t
objects. The refactoring adds new constructors and methods to basis_update_mpf_t
, creates solve_linear_program_with_basis_update
and dual_phase2_with_basis_update
functions, and modifies the branch-and-bound algorithm to manage basis factorization lifecycle across parent-child relationships. The implementation maintains backward compatibility while enabling significant performance improvements in scenarios where child node constraint matrices are similar to their parents, which is common in branch-and-bound trees.
Changed Files
Filename | Score | Overview |
---|---|---|
cpp/src/dual_simplex/mip_node.hpp |
5/5 | Simple method rename from update_tree to update for better API clarity |
cpp/src/dual_simplex/basis_solves.cpp |
5/5 | Memory management optimizations with vector clearing, sizing, and unused variable removal |
cpp/src/dual_simplex/phase2.hpp |
4/5 | Added function declarations for factorize_basis and dual_phase2_with_basis_update |
cpp/src/dual_simplex/branch_and_bound.hpp |
4/5 | Extended solve_node signature with basis parameters and added refactorize_basis method |
cpp/src/dual_simplex/solve.hpp |
4/5 | Added solve_linear_program_with_basis_update function declaration for persistent basis updates |
cpp/src/dual_simplex/basis_updates.cpp |
4/5 | Implemented factorize_basis method with factorization and repair logic |
cpp/src/dual_simplex/basis_updates.hpp |
4/5 | Added constructor, reset method, and factorize_basis declaration to support persistent basis objects |
cpp/src/dual_simplex/solve.cpp |
4/5 | Major refactoring to extract basis-aware solving logic and enable factorization reuse |
cpp/src/dual_simplex/branch_and_bound.cpp |
4/5 | Complex changes to manage basis lifecycle in branch-and-bound with recompute flag logic |
cpp/src/dual_simplex/phase2.cpp |
4/5 | Significant refactoring extracting LU factorization and adding basis-aware phase2 variant |
Confidence score: 3/5
- This PR introduces significant architectural changes to a critical optimization algorithm but appears well-structured with appropriate error handling
- Score reflects the complexity of changes across multiple interconnected files in the dual simplex solver, potential for subtle bugs in numerical algorithms, and incomplete understanding of thread safety implications with shared basis objects
- Pay close attention to
cpp/src/dual_simplex/branch_and_bound.cpp
andcpp/src/dual_simplex/phase2.cpp
for the complex basis lifecycle management and factorization logic
Sequence Diagram
sequenceDiagram
participant User
participant BranchAndBound as Branch & Bound
participant Node as MIP Node
participant BasisUpdate as Basis Update (MPF)
participant Factorize as Factorize Basis
participant SolveNode as Solve Node
participant DualPhase2 as Dual Phase 2
User->>BranchAndBound: solve(mip_solution)
BranchAndBound->>BranchAndBound: solve root relaxation
loop For each node in B&B tree
BranchAndBound->>Node: get node from heap/stack
alt Node requires factorization
BranchAndBound->>Factorize: refactorize_basis(node, ft, basic_list, nonbasic_list)
Factorize->>BasisUpdate: factorize_basis(A, settings, basic_list, nonbasic_list, vstatus)
BasisUpdate-->>Factorize: return status
Factorize-->>BranchAndBound: return factorization status
end
BranchAndBound->>SolveNode: solve_node(node, leaf_problem, ft, basic_list, nonbasic_list)
SolveNode->>Node: get_variable_bounds(lower, upper, bounds_changed)
Node-->>SolveNode: updated bounds
SolveNode->>DualPhase2: dual_phase2_with_basis_update(2, 0, lp, settings, vstatus, ft, ...)
loop Simplex iterations
DualPhase2->>BasisUpdate: b_transpose_solve(rhs, solution)
BasisUpdate-->>DualPhase2: dual solution
DualPhase2->>BasisUpdate: b_solve(rhs, solution)
BasisUpdate-->>DualPhase2: primal solution
alt Basis needs update
DualPhase2->>BasisUpdate: update(utilde, etilde, leaving_index)
BasisUpdate-->>DualPhase2: update status
end
alt Refactorization needed
DualPhase2->>BasisUpdate: factorize_basis(A, settings, basic_list, nonbasic_list, vstatus)
BasisUpdate-->>DualPhase2: new factorization
end
end
DualPhase2-->>SolveNode: LP solution with status
SolveNode-->>BranchAndBound: node status and solution
alt Node has integer solution
BranchAndBound->>BranchAndBound: update incumbent
else Node needs branching
BranchAndBound->>Node: branch on fractional variable
Node->>Node: create child nodes with inherited vstatus
BranchAndBound->>BranchAndBound: add children to exploration queue
end
end
BranchAndBound-->>User: final MIP solution
10 files reviewed, 7 comments
const f_t start_time, | ||
const simplex_solver_settings_t<i_t, f_t>& settings, | ||
lp_solution_t<i_t, f_t>& original_solution, | ||
basis_update_mpf_t<i_t, f_t>& ft, |
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.
style: Parameter ft
could be more descriptively named as basis_update
or factorization
to improve code readability
Prompt To Fix With AI
This is a comment left during a code review.
Path: cpp/src/dual_simplex/solve.hpp
Line: 68:68
Comment:
**style:** Parameter `ft` could be more descriptively named as `basis_update` or `factorization` to improve code readability
How can I resolve this? If you propose a fix, please make it concise.
deficient, | ||
slacks_needed) == -1) { | ||
#ifdef CHECK_L_FACTOR | ||
if (L.check_matrix() == -1) { settings.log.printf("Bad L after basis repair\n"); } |
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.
logic: Variable L
is referenced in debug check but may not be declared in this scope
Prompt To Fix With AI
This is a comment left during a code review.
Path: cpp/src/dual_simplex/basis_updates.cpp
Line: 2102:2102
Comment:
**logic:** Variable `L` is referenced in debug check but may not be declared in this scope
How can I resolve this? If you propose a fix, please make it concise.
bool recompute = true; | ||
basis_update_mpf_t<i_t, f_t> basis_update(m, settings_.refactor_frequency); | ||
std::vector<i_t> basic_list(m); | ||
std::vector<i_t> nonbasic_list; |
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.
style: Consider extracting the basis initialization into a helper method since this pattern is repeated across multiple functions
Prompt To Fix With AI
This is a comment left during a code review.
Path: cpp/src/dual_simplex/branch_and_bound.cpp
Line: 859:862
Comment:
**style:** Consider extracting the basis initialization into a helper method since this pattern is repeated across multiple functions
How can I resolve this? If you propose a fix, please make it concise.
With this PR, the child node can reuse the basis factorisation from the parent, reducing the time required for solving the LP relaxation of the child. The solver recomputes the basis factorisation when the branch reaches the end.
This is a follow-up of #383.
Average (Primal) Gap over the MIPLIB2017 dataset:
main
(91a19f8):15.917959
with224
feasible solutionsThis PR:
15.509966
with224
feasible solutionsChecklist