Diving Improvements #697
Diving Improvements #697
Conversation
… on the objective pseudcost estimate.
… on the objective pseudcost estimate.
nguidotti
added
non-breaking
📝 WalkthroughWalkthroughAdds diving heuristics and a node_queue scheduler, replaces the legacy diving_queue, refactors B&B worker types to bnb_worker_type_t with multiple diving strategies, extends basis repair/refactor APIs to accept variable lower/upper bounds, and propagates related signature, settings, logging, and pseudo-costs changes across dual_simplex. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cpp/src/dual_simplex/phase2.cpp (1)
2837-2858: Passprimal_infeasibility_squaredinstead ofprimal_infeasibilitytoupdate_single_primal_infeasibilityat line 2858.The previous two calls to
update_primal_infeasibilities(lines 2837 and 2849) correctly accumulate squared infeasibility changes intoprimal_infeasibility_squared. However,update_single_primal_infeasibilityalso operates on squared infeasibilities internally (computinginfeas * infeasand accumulating viaprimal_inf + (new_val - old_val)), so it should update the same accumulator. Passingprimal_infeasibilityinstead breaks infeasibility tracking—the entering variable's squared infeasibility delta is accumulated into the wrong variable, leavingprimal_infeasibility_squaredincomplete.
🧹 Nitpick comments (8)
cpp/src/mip/diversity/recombiners/sub_mip.cuh (1)
108-114: Diving heuristics temporarily restricted with clear intent.The explicit configuration disables most diving heuristics (guided, coefficient, pseudocost) for SubMIP, restricting to line search diving only. The comment clearly indicates this is a temporary restriction pending future enablement of all heuristics.
Consider extracting this diving configuration pattern into a helper function since the same configuration appears in multiple files (sub_mip.cuh and rins.cu). This would reduce duplication and make future updates easier.
// Example helper static auto create_line_search_only_diving_settings() { diving_heuristics_settings_t settings; settings.num_diving_tasks = 1; settings.disable_guided_diving = true; settings.disable_coefficient_diving = true; settings.disable_pseudocost_diving = true; return settings; }Based on coding guidelines: refactor code duplication in solver components.
cpp/src/mip/diversity/lns/rins.cu (1)
258-270: Branch-and-bound settings correctly updated for new diving API.The configuration properly integrates the new
diving_settingsstructure while preserving existing behavior for threading, tolerances, logging, and callbacks. The temporary restriction to line search diving only is clearly documented.This file contains the same diving configuration pattern as
sub_mip.cuh. Consider consolidating this repeated configuration into a shared helper function to reduce duplication.Based on coding guidelines: refactor code duplication in solver components (3+ occurrences).
cpp/src/dual_simplex/diving_heuristics.hpp (1)
10-13: Consider adding explicit logger include.
logger_t&is used in all function signatures, but the header relies on transitive inclusion throughpseudo_costs.hpp. For better header self-containment and faster compile times, consider adding an explicit include or forward declaration:#include <dual_simplex/basis_updates.hpp> #include <dual_simplex/bounds_strengthening.hpp> #include <dual_simplex/pseudo_costs.hpp> +#include <dual_simplex/logger.hpp> #include <vector>cpp/src/dual_simplex/basis_solves.cpp (1)
663-671: Logic flow issue: condition at line 667 may be unreachable in practice.The current logic:
- Line 663: If
lower == -inf && upper == inf→ FREE- Line 665: Else if
lower > -inf→ LOWER- Line 667: Else if
upper < inf→ UPPER- Line 669: Else → assert
At line 667, we know
lower[bad_j] <= -inf(from failing line 665). Combined with failing line 663, we must haveupper[bad_j] < inf, so the condition is always true when reached. Theelseat line 669 is dead code.Consider simplifying:
if (lower[bad_j] == -inf && upper[bad_j] == inf) { vstatus[bad_j] = variable_status_t::NONBASIC_FREE; } else if (lower[bad_j] > -inf) { vstatus[bad_j] = variable_status_t::NONBASIC_LOWER; - } else if (upper[bad_j] < inf) { - vstatus[bad_j] = variable_status_t::NONBASIC_UPPER; } else { - assert(1 == 0); + vstatus[bad_j] = variable_status_t::NONBASIC_UPPER; }The assert is unnecessary since the logic is exhaustive.
cpp/src/dual_simplex/diving_heuristics.cpp (1)
49-55: Edge case: emptyfractionalvector causes undefined behavior.If
fractionalis empty, the fallback at line 50 accessesfractional[0]which is undefined, and the assertions at lines 54-55 will fail. While callers should ensurefractionalis non-empty, defensive handling would improve robustness:if (round_dir == rounding_direction_t::NONE) { + assert(!fractional.empty() && "line_search_diving called with empty fractional list"); branch_var = fractional[0]; round_dir = rounding_direction_t::DOWN; } - - assert(round_dir != rounding_direction_t::NONE); - assert(branch_var >= 0);cpp/src/dual_simplex/node_queue.hpp (3)
33-38: Incorrect use ofstd::forwardwith doubly-referenced type.The
Args&&in the template parameter already declares forwarding references. Usingstd::forward<Args&&>adds an extra&&, which is redundant and could cause issues with some compilers.template <typename... Args> void emplace(Args&&... args) { - buffer.emplace_back(std::forward<Args&&>(args)...); + buffer.emplace_back(std::forward<Args>(args)...); std::push_heap(buffer.begin(), buffer.end(), comp); }
136-146: Potential integer type narrowing in size accessors.
heap_t::size()returnssize_t, butdiving_queue_size()andbest_first_queue_size()returni_t(typicallyint). For very large heaps exceedingINT_MAX, this could cause truncation.Consider returning
size_tor usingstatic_castwith appropriate bounds checking:- i_t diving_queue_size() + size_t diving_queue_size() { std::lock_guard<omp_mutex_t> lock(mutex); return diving_heap.size(); } - i_t best_first_queue_size() + size_t best_first_queue_size() { std::lock_guard<omp_mutex_t> lock(mutex); return best_first_heap.size(); }
116-134: Stale entries indiving_heapcause O(n) iterations inpop_diving().Both
best_first_heapanddiving_heapreference the sameheap_entry_tobjects viashared_ptr. Whenpop_best_first()executesstd::exchange(entry.value()->node, nullptr)at line 110, it nullifies the entry's node pointer. However, the entry itself remains indiving_heap. Subsequent calls topop_diving()must iterate through these stale entries (lines 122–130) until finding a valid one, resulting in O(n) complexity where n is the number of stale entries accumulated.Consider maintaining a count of valid entries or implementing atomic removal from both heaps to avoid worst-case iteration when heavy best-first activity precedes diving operations.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (23)
cpp/src/dual_simplex/CMakeLists.txt(1 hunks)cpp/src/dual_simplex/basis_solves.cpp(3 hunks)cpp/src/dual_simplex/basis_solves.hpp(1 hunks)cpp/src/dual_simplex/basis_updates.cpp(2 hunks)cpp/src/dual_simplex/basis_updates.hpp(1 hunks)cpp/src/dual_simplex/bounds_strengthening.cpp(2 hunks)cpp/src/dual_simplex/branch_and_bound.cpp(30 hunks)cpp/src/dual_simplex/branch_and_bound.hpp(8 hunks)cpp/src/dual_simplex/crossover.cpp(3 hunks)cpp/src/dual_simplex/diving_heuristics.cpp(1 hunks)cpp/src/dual_simplex/diving_heuristics.hpp(1 hunks)cpp/src/dual_simplex/diving_queue.hpp(0 hunks)cpp/src/dual_simplex/logger.hpp(1 hunks)cpp/src/dual_simplex/mip_node.hpp(4 hunks)cpp/src/dual_simplex/node_queue.hpp(1 hunks)cpp/src/dual_simplex/phase2.cpp(12 hunks)cpp/src/dual_simplex/primal.cpp(1 hunks)cpp/src/dual_simplex/pseudo_costs.cpp(4 hunks)cpp/src/dual_simplex/pseudo_costs.hpp(1 hunks)cpp/src/dual_simplex/simplex_solver_settings.hpp(3 hunks)cpp/src/mip/diversity/lns/rins.cu(1 hunks)cpp/src/mip/diversity/recombiners/sub_mip.cuh(1 hunks)cpp/src/mip/solver.cu(1 hunks)
💤 Files with no reviewable changes (1)
- cpp/src/dual_simplex/diving_queue.hpp
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{cu,cuh,cpp,hpp,h}
📄 CodeRabbit inference engine (.github/.coderabbit_review_guide.md)
**/*.{cu,cuh,cpp,hpp,h}: Track GPU device memory allocations and deallocations to prevent memory leaks; ensure cudaMalloc/cudaFree balance and cleanup of streams/events
Validate algorithm correctness in optimization logic: simplex pivots, branch-and-bound decisions, routing heuristics, and constraint/objective handling must produce correct results
Check numerical stability: prevent overflow/underflow, precision loss, division by zero/near-zero, and use epsilon comparisons for floating-point equality checks
Validate correct initialization of variable bounds, constraint coefficients, and algorithm state before solving; ensure reset when transitioning between algorithm phases (presolve, simplex, diving, crossover)
Ensure variables and constraints are accessed from the correct problem context (original vs presolve vs folded vs postsolve); verify index mapping consistency across problem transformations
For concurrent CUDA operations (barriers, async operations), explicitly create and manage dedicated streams instead of reusing the default stream; document stream lifecycle
Eliminate unnecessary host-device synchronization (cudaDeviceSynchronize) in hot paths that blocks GPU pipeline; use streams and events for async execution
Assess algorithmic complexity for large-scale problems (millions of variables/constraints); ensure O(n log n) or better complexity, not O(n²) or worse
Verify correct problem size checks before expensive GPU/CPU operations; prevent resource exhaustion on oversized problems
Identify assertions with overly strict numerical tolerances that fail on legitimate degenerate/edge cases (near-zero pivots, singular matrices, empty problems)
Ensure race conditions are absent in multi-GPU code and multi-threaded server implementations; verify proper synchronization of shared state
Refactor code duplication in solver components (3+ occurrences) into shared utilities; for GPU kernels, use templated device functions to avoid duplication
Check that hard-coded GPU de...
Files:
cpp/src/dual_simplex/basis_updates.hppcpp/src/dual_simplex/bounds_strengthening.cppcpp/src/dual_simplex/basis_solves.hppcpp/src/dual_simplex/logger.hppcpp/src/mip/diversity/recombiners/sub_mip.cuhcpp/src/mip/diversity/lns/rins.cucpp/src/mip/solver.cucpp/src/dual_simplex/primal.cppcpp/src/dual_simplex/mip_node.hppcpp/src/dual_simplex/crossover.cppcpp/src/dual_simplex/basis_updates.cppcpp/src/dual_simplex/simplex_solver_settings.hppcpp/src/dual_simplex/pseudo_costs.hppcpp/src/dual_simplex/basis_solves.cppcpp/src/dual_simplex/diving_heuristics.hppcpp/src/dual_simplex/pseudo_costs.cppcpp/src/dual_simplex/phase2.cppcpp/src/dual_simplex/diving_heuristics.cppcpp/src/dual_simplex/node_queue.hppcpp/src/dual_simplex/branch_and_bound.hppcpp/src/dual_simplex/branch_and_bound.cpp
**/*.{h,hpp,py}
📄 CodeRabbit inference engine (.github/.coderabbit_review_guide.md)
Verify C API does not break ABI stability (no struct layout changes, field reordering); maintain backward compatibility in Python and server APIs with deprecation warnings
Files:
cpp/src/dual_simplex/basis_updates.hppcpp/src/dual_simplex/basis_solves.hppcpp/src/dual_simplex/logger.hppcpp/src/dual_simplex/mip_node.hppcpp/src/dual_simplex/simplex_solver_settings.hppcpp/src/dual_simplex/pseudo_costs.hppcpp/src/dual_simplex/diving_heuristics.hppcpp/src/dual_simplex/node_queue.hppcpp/src/dual_simplex/branch_and_bound.hpp
**/*.{cpp,hpp,h}
📄 CodeRabbit inference engine (.github/.coderabbit_review_guide.md)
**/*.{cpp,hpp,h}: Check for unclosed file handles when reading MPS/QPS problem files; ensure RAII patterns or proper cleanup in exception paths
Validate input sanitization to prevent buffer overflows and resource exhaustion attacks; avoid unsafe deserialization of problem files
Prevent thread-unsafe use of global and static variables; use proper mutex/synchronization in server code accessing shared solver state
Files:
cpp/src/dual_simplex/basis_updates.hppcpp/src/dual_simplex/bounds_strengthening.cppcpp/src/dual_simplex/basis_solves.hppcpp/src/dual_simplex/logger.hppcpp/src/dual_simplex/primal.cppcpp/src/dual_simplex/mip_node.hppcpp/src/dual_simplex/crossover.cppcpp/src/dual_simplex/basis_updates.cppcpp/src/dual_simplex/simplex_solver_settings.hppcpp/src/dual_simplex/pseudo_costs.hppcpp/src/dual_simplex/basis_solves.cppcpp/src/dual_simplex/diving_heuristics.hppcpp/src/dual_simplex/pseudo_costs.cppcpp/src/dual_simplex/phase2.cppcpp/src/dual_simplex/diving_heuristics.cppcpp/src/dual_simplex/node_queue.hppcpp/src/dual_simplex/branch_and_bound.hppcpp/src/dual_simplex/branch_and_bound.cpp
**/*.{cu,cpp,hpp,h}
📄 CodeRabbit inference engine (.github/.coderabbit_review_guide.md)
Avoid inappropriate use of exceptions in performance-critical GPU operation paths; prefer error codes or CUDA error checking for latency-sensitive code
Files:
cpp/src/dual_simplex/basis_updates.hppcpp/src/dual_simplex/bounds_strengthening.cppcpp/src/dual_simplex/basis_solves.hppcpp/src/dual_simplex/logger.hppcpp/src/mip/diversity/lns/rins.cucpp/src/mip/solver.cucpp/src/dual_simplex/primal.cppcpp/src/dual_simplex/mip_node.hppcpp/src/dual_simplex/crossover.cppcpp/src/dual_simplex/basis_updates.cppcpp/src/dual_simplex/simplex_solver_settings.hppcpp/src/dual_simplex/pseudo_costs.hppcpp/src/dual_simplex/basis_solves.cppcpp/src/dual_simplex/diving_heuristics.hppcpp/src/dual_simplex/pseudo_costs.cppcpp/src/dual_simplex/phase2.cppcpp/src/dual_simplex/diving_heuristics.cppcpp/src/dual_simplex/node_queue.hppcpp/src/dual_simplex/branch_and_bound.hppcpp/src/dual_simplex/branch_and_bound.cpp
**/*.{cu,cuh}
📄 CodeRabbit inference engine (.github/.coderabbit_review_guide.md)
**/*.{cu,cuh}: Every CUDA kernel launch and memory operation must have error checking with CUDA_CHECK or equivalent verification
Avoid reinventing functionality already available in Thrust, CCCL, or RMM libraries; prefer standard library utilities over custom implementations
Files:
cpp/src/mip/diversity/recombiners/sub_mip.cuhcpp/src/mip/diversity/lns/rins.cucpp/src/mip/solver.cu
**/*.cu
📄 CodeRabbit inference engine (.github/.coderabbit_review_guide.md)
**/*.cu: Verify race conditions and correctness of GPU kernel shared memory, atomics, and warp-level operations
Detect inefficient GPU kernel launches with low occupancy or poor memory access patterns; optimize for coalesced memory access and minimize warp divergence in hot paths
Files:
cpp/src/mip/diversity/lns/rins.cucpp/src/mip/solver.cu
🧠 Learnings (18)
📓 Common learnings
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Reduce tight coupling between solver components (presolve, simplex, basis, barrier); increase modularity and reusability of optimization algorithms
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Validate correct initialization of variable bounds, constraint coefficients, and algorithm state before solving; ensure reset when transitioning between algorithm phases (presolve, simplex, diving, crossover)
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*test*.{cpp,cu,py} : Add tests for algorithm phase transitions: verify correct initialization of bounds and state when transitioning from presolve to simplex to diving to crossover
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Validate algorithm correctness in optimization logic: simplex pivots, branch-and-bound decisions, routing heuristics, and constraint/objective handling must produce correct results
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Reduce tight coupling between solver components (presolve, simplex, basis, barrier); increase modularity and reusability of optimization algorithms
Applied to files:
cpp/src/dual_simplex/basis_updates.hppcpp/src/dual_simplex/bounds_strengthening.cppcpp/src/dual_simplex/basis_solves.hppcpp/src/mip/solver.cucpp/src/dual_simplex/primal.cppcpp/src/dual_simplex/crossover.cppcpp/src/dual_simplex/basis_updates.cppcpp/src/dual_simplex/simplex_solver_settings.hppcpp/src/dual_simplex/basis_solves.cppcpp/src/dual_simplex/phase2.cppcpp/src/dual_simplex/branch_and_bound.hppcpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Validate correct initialization of variable bounds, constraint coefficients, and algorithm state before solving; ensure reset when transitioning between algorithm phases (presolve, simplex, diving, crossover)
Applied to files:
cpp/src/dual_simplex/basis_updates.hppcpp/src/dual_simplex/bounds_strengthening.cppcpp/src/dual_simplex/basis_solves.hppcpp/src/mip/diversity/recombiners/sub_mip.cuhcpp/src/dual_simplex/CMakeLists.txtcpp/src/mip/diversity/lns/rins.cucpp/src/mip/solver.cucpp/src/dual_simplex/primal.cppcpp/src/dual_simplex/mip_node.hppcpp/src/dual_simplex/crossover.cppcpp/src/dual_simplex/basis_updates.cppcpp/src/dual_simplex/simplex_solver_settings.hppcpp/src/dual_simplex/basis_solves.cppcpp/src/dual_simplex/diving_heuristics.hppcpp/src/dual_simplex/pseudo_costs.cppcpp/src/dual_simplex/phase2.cppcpp/src/dual_simplex/diving_heuristics.cppcpp/src/dual_simplex/branch_and_bound.hppcpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Validate algorithm correctness in optimization logic: simplex pivots, branch-and-bound decisions, routing heuristics, and constraint/objective handling must produce correct results
Applied to files:
cpp/src/dual_simplex/basis_updates.hppcpp/src/dual_simplex/bounds_strengthening.cppcpp/src/dual_simplex/basis_solves.hppcpp/src/dual_simplex/CMakeLists.txtcpp/src/mip/diversity/lns/rins.cucpp/src/mip/solver.cucpp/src/dual_simplex/primal.cppcpp/src/dual_simplex/crossover.cppcpp/src/dual_simplex/basis_updates.cppcpp/src/dual_simplex/simplex_solver_settings.hppcpp/src/dual_simplex/pseudo_costs.hppcpp/src/dual_simplex/basis_solves.cppcpp/src/dual_simplex/diving_heuristics.hppcpp/src/dual_simplex/pseudo_costs.cppcpp/src/dual_simplex/phase2.cppcpp/src/dual_simplex/diving_heuristics.cppcpp/src/dual_simplex/node_queue.hppcpp/src/dual_simplex/branch_and_bound.hppcpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Ensure variables and constraints are accessed from the correct problem context (original vs presolve vs folded vs postsolve); verify index mapping consistency across problem transformations
Applied to files:
cpp/src/dual_simplex/bounds_strengthening.cppcpp/src/mip/solver.cucpp/src/dual_simplex/crossover.cppcpp/src/dual_simplex/basis_solves.cppcpp/src/dual_simplex/diving_heuristics.hppcpp/src/dual_simplex/pseudo_costs.cppcpp/src/dual_simplex/phase2.cppcpp/src/dual_simplex/diving_heuristics.cppcpp/src/dual_simplex/branch_and_bound.hppcpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*test*.{cpp,cu,py} : Add tests for algorithm phase transitions: verify correct initialization of bounds and state when transitioning from presolve to simplex to diving to crossover
Applied to files:
cpp/src/dual_simplex/bounds_strengthening.cppcpp/src/dual_simplex/CMakeLists.txtcpp/src/mip/diversity/lns/rins.cucpp/src/mip/solver.cucpp/src/dual_simplex/primal.cppcpp/src/dual_simplex/crossover.cppcpp/src/dual_simplex/basis_updates.cppcpp/src/dual_simplex/simplex_solver_settings.hppcpp/src/dual_simplex/basis_solves.cppcpp/src/dual_simplex/diving_heuristics.hppcpp/src/dual_simplex/phase2.cppcpp/src/dual_simplex/diving_heuristics.cppcpp/src/dual_simplex/branch_and_bound.hppcpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Identify assertions with overly strict numerical tolerances that fail on legitimate degenerate/edge cases (near-zero pivots, singular matrices, empty problems)
Applied to files:
cpp/src/dual_simplex/bounds_strengthening.cppcpp/src/dual_simplex/basis_solves.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*test*.{cpp,cu,py} : Test with free variables, singleton problems, and extreme problem dimensions near resource limits to validate edge case handling
Applied to files:
cpp/src/dual_simplex/bounds_strengthening.cppcpp/src/dual_simplex/CMakeLists.txt
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Assess algorithmic complexity for large-scale problems (millions of variables/constraints); ensure O(n log n) or better complexity, not O(n²) or worse
Applied to files:
cpp/src/dual_simplex/bounds_strengthening.cppcpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Refactor code duplication in solver components (3+ occurrences) into shared utilities; for GPU kernels, use templated device functions to avoid duplication
Applied to files:
cpp/src/dual_simplex/CMakeLists.txtcpp/src/mip/diversity/lns/rins.cucpp/src/mip/solver.cucpp/src/dual_simplex/simplex_solver_settings.hppcpp/src/dual_simplex/diving_heuristics.hppcpp/src/dual_simplex/diving_heuristics.cppcpp/src/dual_simplex/node_queue.hppcpp/src/dual_simplex/branch_and_bound.hppcpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh} : Avoid reinventing functionality already available in Thrust, CCCL, or RMM libraries; prefer standard library utilities over custom implementations
Applied to files:
cpp/src/dual_simplex/CMakeLists.txt
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Check that hard-coded GPU device IDs and resource limits are made configurable; abstract multi-backend support for different CUDA versions
Applied to files:
cpp/src/dual_simplex/CMakeLists.txtcpp/src/mip/diversity/lns/rins.cucpp/src/mip/solver.cu
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Verify correct problem size checks before expensive GPU/CPU operations; prevent resource exhaustion on oversized problems
Applied to files:
cpp/src/dual_simplex/CMakeLists.txt
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Track GPU device memory allocations and deallocations to prevent memory leaks; ensure cudaMalloc/cudaFree balance and cleanup of streams/events
Applied to files:
cpp/src/dual_simplex/CMakeLists.txt
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*benchmark*.{cpp,cu,py} : Include performance benchmarks and regression detection for GPU operations; verify near real-time performance on million-variable problems
Applied to files:
cpp/src/dual_simplex/CMakeLists.txt
📚 Learning: 2025-12-04T20:09:09.264Z
Learnt from: chris-maes
Repo: NVIDIA/cuopt PR: 602
File: cpp/src/linear_programming/solve.cu:732-742
Timestamp: 2025-12-04T20:09:09.264Z
Learning: In cpp/src/linear_programming/solve.cu, the barrier solver does not currently return INFEASIBLE or UNBOUNDED status. It only returns OPTIMAL, TIME_LIMIT, NUMERICAL_ISSUES, or CONCURRENT_LIMIT.
Applied to files:
cpp/src/mip/solver.cucpp/src/dual_simplex/diving_heuristics.hppcpp/src/dual_simplex/phase2.cppcpp/src/dual_simplex/diving_heuristics.cppcpp/src/dual_simplex/branch_and_bound.hppcpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cpp,hpp,h} : Prevent thread-unsafe use of global and static variables; use proper mutex/synchronization in server code accessing shared solver state
Applied to files:
cpp/src/mip/solver.cucpp/src/dual_simplex/pseudo_costs.cpp
📚 Learning: 2025-12-04T04:11:12.640Z
Learnt from: chris-maes
Repo: NVIDIA/cuopt PR: 500
File: cpp/src/dual_simplex/scaling.cpp:68-76
Timestamp: 2025-12-04T04:11:12.640Z
Learning: In the cuOPT dual simplex solver, CSR/CSC matrices (including the quadratic objective matrix Q) are required to have valid dimensions and indices by construction. Runtime bounds checking in performance-critical paths like matrix scaling is avoided to prevent slowdowns. Validation is performed via debug-only check_matrix() calls wrapped in #ifdef CHECK_MATRIX.
Applied to files:
cpp/src/dual_simplex/basis_solves.cppcpp/src/dual_simplex/branch_and_bound.hpp
🧬 Code graph analysis (11)
cpp/src/dual_simplex/basis_updates.hpp (1)
cpp/src/dual_simplex/presolve.cpp (3)
lower(240-240)upper(85-85)upper(241-241)
cpp/src/dual_simplex/logger.hpp (1)
cpp/src/dual_simplex/simplex_solver_settings.hpp (2)
log_filename(99-99)log_filename(99-99)
cpp/src/dual_simplex/primal.cpp (2)
cpp/src/dual_simplex/basis_solves.cpp (3)
basis_repair(614-675)basis_repair(614-622)basis_repair(860-868)cpp/src/dual_simplex/basis_solves.hpp (1)
basis_repair(43-51)
cpp/src/dual_simplex/crossover.cpp (2)
cpp/src/dual_simplex/basis_solves.cpp (3)
basis_repair(614-675)basis_repair(614-622)basis_repair(860-868)cpp/src/dual_simplex/basis_solves.hpp (1)
basis_repair(43-51)
cpp/src/dual_simplex/basis_updates.cpp (3)
cpp/src/dual_simplex/basis_solves.cpp (3)
basis_repair(614-675)basis_repair(614-622)basis_repair(860-868)cpp/src/dual_simplex/basis_solves.hpp (1)
basis_repair(43-51)cpp/src/dual_simplex/basis_updates.hpp (1)
A(374-380)
cpp/src/dual_simplex/basis_solves.cpp (1)
cpp/src/dual_simplex/presolve.cpp (3)
lower(240-240)upper(85-85)upper(241-241)
cpp/src/dual_simplex/diving_heuristics.hpp (2)
cpp/src/dual_simplex/pseudo_costs.hpp (2)
fractional(46-49)fractional(51-52)cpp/src/dual_simplex/simplex_solver_settings.hpp (2)
log(98-98)log(100-100)
cpp/src/dual_simplex/pseudo_costs.cpp (1)
cpp/src/dual_simplex/pseudo_costs.hpp (2)
fractional(46-49)fractional(51-52)
cpp/src/dual_simplex/phase2.cpp (1)
cpp/src/dual_simplex/primal.cpp (2)
primal_infeasibility(202-240)primal_infeasibility(202-205)
cpp/src/dual_simplex/node_queue.hpp (1)
cpp/src/dual_simplex/mip_node.hpp (4)
lower(97-108)lower(97-99)lower(112-130)lower(112-114)
cpp/src/dual_simplex/branch_and_bound.hpp (1)
cpp/src/dual_simplex/node_queue.hpp (4)
node(21-25)node(21-21)node(27-31)node(27-27)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: wheel-build-cuopt-mps-parser / 13.0.2, 3.11, arm64, rockylinux8
- GitHub Check: wheel-build-cuopt-mps-parser / 13.0.2, 3.13, arm64, rockylinux8
- GitHub Check: wheel-build-cuopt-mps-parser / 13.0.2, 3.10, amd64, rockylinux8
- GitHub Check: wheel-build-cuopt-mps-parser / 13.0.2, 3.13, amd64, rockylinux8
- GitHub Check: wheel-build-cuopt-mps-parser / 13.0.2, 3.11, amd64, rockylinux8
- GitHub Check: wheel-build-cuopt-sh-client / 13.0.2, 3.10, amd64, rockylinux8
- GitHub Check: checks / check-style
🔇 Additional comments (39)
cpp/src/dual_simplex/CMakeLists.txt (1)
34-34: LGTM!The addition of
diving_heuristics.cppto the build aligns with the PR objectives to introduce new diving heuristic strategies.cpp/src/dual_simplex/bounds_strengthening.cpp (2)
157-164: Logging level correctly adjusted to debug.The change from
printftosettings.log.debug()appropriately reduces stdout noise while preserving diagnostic information for debugging. This is proper logging hygiene for infeasibility detection during bounds strengthening iterations.
214-216: Logging level correctly adjusted to debug.Consistent with the previous logging change, this infeasibility message is appropriately moved to debug level logging.
cpp/src/dual_simplex/primal.cpp (1)
301-309: All basis_repair call sites have been consistently updated with bound parameters.The call in
primal.cppat lines 301-309 correctly passeslp.lowerandlp.upperas the 3rd and 4th parameters. Verification confirms this pattern is consistent across all five basis_repair call sites in the codebase:
primal.cpp:301- passeslp.lower, lp.uppercrossover.cpp:788, 1142, 1341- all passlp.lower, lp.upperbasis_updates.cpp:2071- passeslower, upper(local variables in templated context)This ensures bounds-aware basis repair during algorithm phase transitions (simplex, crossover phases).
cpp/src/mip/solver.cu (1)
176-180: Threading configuration correctly refactored for new diving_settings structure.The code properly updates the thread distribution logic to use the new
diving_settings.num_diving_tasksfield instead of the previousnum_diving_threads. The thread allocation strategy remains unchanged (BFS threads = num_threads/4, diving tasks = remaining threads).cpp/src/dual_simplex/pseudo_costs.hpp (1)
46-49: API change correctly expands pseudo-cost functionality.The function signature update from
variable_selectiontovariable_selection_and_obj_estimateproperly reflects the expanded responsibility: returning both the selected variable index and an objective estimate. The addition of thelower_boundparameter enables more sophisticated objective estimation for diving heuristics.All call sites have been correctly updated. The old
variable_selectionfunction is preserved as a backward-compatibility wrapper that properly unpacks the returned pair viastd::tie, stores the objective estimate in node state for search tree ordering, and returns the branch decision. New call sites directly usevariable_selection_and_obj_estimatewith proper structured binding to capture both returned elements.cpp/src/dual_simplex/logger.hpp (1)
33-44: LGTM! Backward-compatible API refinement.The change from
std::stringtoconst char*for the mode parameter avoids unnecessary string construction when passing literals. The default value"w"ensures backward compatibility with existing callers likeset_log_filenamethat don't specify a mode.cpp/src/dual_simplex/basis_solves.hpp (1)
42-51: LGTM! Bound-aware basis repair signature.The addition of
lowerandupperbound vectors enablesbasis_repairto correctly assign variable status (NONBASIC_FREE, NONBASIC_LOWER, or NONBASIC_UPPER) for variables being removed from the basis. This is essential for maintaining dual feasibility during repair operations. Based on coding guidelines, this correctly validates variable bounds during algorithm state transitions.cpp/src/dual_simplex/crossover.cpp (3)
788-796: LGTM! Correct bound propagation to basis repair.The
lp.lowerandlp.upperbounds are correctly passed tobasis_repair, enabling proper variable status assignment during the primal push refactoring phase.
1142-1150: LGTM! Consistent bound handling in crossover initialization.Bounds are correctly propagated during the initial basis repair in the crossover function, consistent with the other call sites.
1341-1349: LGTM! Bounds passed after phase 1 completion.The third call site correctly passes bounds after phase 1 completes, maintaining consistency across all
basis_repairinvocations in the crossover workflow.cpp/src/dual_simplex/basis_updates.cpp (2)
2046-2054: LGTM! Signature update matches declaration.The
refactor_basisimplementation correctly accepts the newlowerandupperbound parameters, consistent with the header declaration inbasis_updates.hpp.
2071-2072: LGTM! Bounds correctly forwarded to basis_repair.When initial factorization fails, the bounds are properly passed to
basis_repairto enable correct variable status assignment during the repair process. Based on coding guidelines, this ensures correct initialization of variable bounds when transitioning between algorithm phases.cpp/src/dual_simplex/basis_updates.hpp (1)
374-380: Verify all callers ofrefactor_basishave been updated to pass the new bound parameters.The
refactor_basisdeclaration correctly addslowerandupperbound vectors as const references. All callers in phase2.cpp (lines 2248, 2910, 2917) have been properly updated to passlp.lowerandlp.upper. This is a breaking API change requiring all callers to be updated.cpp/src/dual_simplex/simplex_solver_settings.hpp (1)
22-34: Well-structured diving configuration.The new
diving_heuristics_settings_tstruct cleanly encapsulates diving-related parameters with sensible defaults. The separation of heuristic enable/disable flags and resource limits (node_limit, iteration_limit_factor, backtrack) follows good configuration design practices.cpp/src/dual_simplex/pseudo_costs.cpp (2)
202-202: Good improvement: RAII-based locking.Using
std::lock_guard<omp_mutex_t>ensures the mutex is properly released even if an exception occurs or the function returns early. This is a safer pattern than manual lock/unlock.
256-321: Correct implementation of pseudocost-based objective estimate.The function correctly:
- Uses RAII locking for thread safety
- Computes per-variable pseudocosts with fallback to averages for uninitialized values
- Applies the reliability branching score formula (product of down/up estimates)
- Accumulates a conservative objective estimate using the minimum of up/down contributions
The algorithm aligns with standard MIP solver literature (Achterberg Sect. 6.4).
cpp/src/dual_simplex/diving_heuristics.hpp (1)
17-47: Clean diving heuristics API design.The header provides a well-organized interface for the four diving strategies from Achterberg's thesis:
- Line search diving (Sect. 9.2.4)
- Pseudocost diving (Sect. 9.2.5)
- Guided diving (Sect. 9.2.3)
- Coefficient diving (Sect. 9.2.1)
The
branch_variable_treturn type cleanly bundles the selected variable with its rounding direction.cpp/src/dual_simplex/diving_heuristics.cpp (3)
66-139: Correct pseudocost diving implementation with thread safety.The implementation correctly:
- Acquires mutex lock before accessing shared pseudocost data
- Uses fallback averages for uninitialized pseudocosts
- Applies the scoring formula from Achterberg Sect. 9.2.5
- Uses threshold-based direction selection (0.4 for root distance, 0.3/0.7 for fractionality)
The magic constants are standard heuristic values from MIP literature.
141-197: Correct guided diving implementation.The function properly implements Achterberg's guided diving (Sect. 9.2.3):
- Direction determined by proximity to incumbent solution
- Score weighted 5:1 in favor of the preferred direction
- Thread-safe pseudocost access
Note the same empty
fractionaledge case applies here.
229-273: Correct coefficient diving implementation.The function implements Achterberg's coefficient diving (Sect. 9.2.1):
- Selects variable with minimum constraint locks
- Prefers rounding direction that creates fewer constraint violations
- Uses fractionality as tie-breaker when locks are equal
The same empty
fractionaledge case consideration applies.cpp/src/dual_simplex/basis_solves.cpp (1)
613-622: Signature extension for bound-aware basis repair is complete and correctly integrated.The addition of
lowerandupperbound vectors enables correct variable status assignment during basis repair. All call sites have been updated to pass the bound vectors:
primal.cpp(1 call)crossover.cpp(3 calls)basis_updates.cpp(1 call)All invocations properly pass
lp.lowerandlp.upper(or locallower/upperfrom the calling context). The signature change aligns with the PR's goal of propagating bound information through solver phases.cpp/src/dual_simplex/node_queue.hpp (1)
60-101: Well-structured dual-heap design for unified node scheduling.The
node_queue_tclass effectively unifies best-first and diving heaps with shared entries, aligning with the PR objective to reduce memory consumption. The use ofshared_ptr<heap_entry_t>for cross-heap entry sharing is a clean approach.cpp/src/dual_simplex/phase2.cpp (3)
620-651: Signature change correctly separates squared and linear infeasibility.The updated
compute_initial_primal_infeasibilitiesnow properly computes both forms:
- Returns
primal_inf_squared(sum of squared infeasibilities)- Outputs
primal_inf(sum of linear infeasibilities) via referenceThis distinction is important for different algorithmic needs (squared for steepest-edge pricing weights, linear for feasibility checks). The initialization of
squared_infeasibilitiesvector at lines 631-632 ensures clean state.
2365-2373: Proper initialization of infeasibility tracking variables.The new code correctly initializes
primal_infeasibilityandprimal_infeasibility_squaredseparately, withcompute_initial_primal_infeasibilitiespopulating both values. This ensures clean state before the main iteration loop.
2248-2251: Allrefactor_basiscall sites are consistent with the function signature.Verification confirms that all three call sites in phase2.cpp (lines 2248, 2910–2911, and 2917–2918) correctly include the
lp.lowerandlp.upperbound parameters matching the function signature in basis_updates.hpp and basis_updates.cpp.cpp/src/dual_simplex/branch_and_bound.hpp (4)
56-67: Clear documentation and well-structured enum for diving strategies.The
bnb_thread_type_tenum provides clear categorization with references to the Achterberg dissertation sections. This aligns well with the PR objective of implementing multiple diving heuristics.
75-86: Thread-safe statistics structure with atomic counters.The
bnb_stats_tstruct appropriately usesomp_atomic_tfor counters that may be updated from multiple threads. Thelast_logandnodes_since_last_logmembers with the comment about main-thread-only usage help clarify the intended access pattern.
182-184: Unified node queue replaces separate heap structures.The replacement of the previous heap-based structures with
node_queue_taligns with the PR objective to "unify diving and best-first heaps into a single node_queue object." This should help reduce memory consumption as mentioned in the PR description.
261-266:variable_selectionreturn type and parameters are well-designed.Returning
branch_variable_t<i_t>which includes both the variable index and rounding direction is cleaner than having multiple output parameters. The additionallogger_t& logparameter enables per-call logging control.cpp/src/dual_simplex/branch_and_bound.cpp (9)
558-577: Martin's criteria implementation for rounding direction.The
martin_criteriafunction correctly implements the rounding heuristic based on distance from the root relaxation value. The epsilon tolerance (1e-6) for tie-breaking is reasonable.
579-619: Clean dispatch pattern for variable selection strategies.The
variable_selectionmethod uses a clear switch statement to dispatch to different diving heuristics. The objective estimate calculation is correctly scoped to only the EXPLORATION case where it's needed for heap ordering.
640-643: Appropriate fallback from GUIDED_DIVING to PSEUDOCOST_DIVING.When there's no incumbent solution (
upper_bound == inf), guided diving cannot function since it relies on the incumbent for guidance. Falling back to pseudocost diving is a sensible default.
656-661: Iteration budget enforcement for diving threads.The iteration limit for diving threads is computed as a factor of total B&B LP iterations minus iterations already consumed by this dive. This prevents diving from consuming too many iterations.
However, if
stats.total_lp_itersis close tomax_iter, this could returnITERATION_LIMITvery early. Verify that theiteration_limit_factorsetting provides sufficient budget for meaningful dives.
1143-1219: Deep diving implementation with backtrack limit.The
dive_frommethod implements the diving heuristics with proper backtracking control. Key observations:
- Lines 1163-1166: Local
dive_statsproperly tracks per-dive statistics- Lines 1179-1180: Time and node limits checked within the dive loop
- Lines 1215-1218: Backtrack limit enforces shallow backtracking by removing nodes from the back of the stack when depth difference exceeds the limit
The algorithm correctly prioritizes depth-first exploration while limiting the breadth of backtracking.
1248-1255: Bounds propagation for diving thread efficiency.The code correctly retrieves variable bounds from the node and applies bounds strengthening before diving. The
reset_starting_boundsflag ensures bounds are properly reset when starting from a new node.
1518-1535: Diving strategy configuration is flexible and well-structured.The diving strategies are populated based on settings, allowing users to disable specific strategies. The
reserve(4)call is appropriate given the four possible strategies.
251-260: Improved lower bound computation uses node_queue.The
get_lower_boundmethod correctly combines:
lower_bound_ceiling_(for numerical issue tracking)node_queue.get_lower_bound()(heap minimum)local_lower_bounds_(per-thread tracking)This provides an accurate global lower bound across all active exploration paths.
1354-1354: Row-major matrix initialization for bounds strengthening.Converting the constraint matrix to row-major format (
Arow_) enables efficient row-based access during bounds strengthening. This is initialized once at the start ofsolve(), avoiding repeated conversions.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
cpp/src/dual_simplex/pseudo_costs.cpp (2)
199-214: Guard against divide-by-zero / near-zerofracinupdate_pseudo_costs().
change_in_obj / fraccan explode ifnode_ptr->fractional_valis numerically (near-)integral (or if caller misclassifies), producing inf/NaN pseudo-costs that then poison selection/estimates. As per coding guidelines, this is a numerical stability footgun.Proposed fix
void pseudo_costs_t<i_t, f_t>::update_pseudo_costs(mip_node_t<i_t, f_t>* node_ptr, f_t leaf_objective) { std::lock_guard<omp_mutex_t> lock(mutex); const f_t change_in_obj = leaf_objective - node_ptr->lower_bound; const f_t frac = node_ptr->branch_dir == rounding_direction_t::DOWN ? node_ptr->fractional_val - std::floor(node_ptr->fractional_val) : std::ceil(node_ptr->fractional_val) - node_ptr->fractional_val; + constexpr f_t eps = 1e-9; + if (!std::isfinite(change_in_obj) || frac <= eps) { return; } if (node_ptr->branch_dir == rounding_direction_t::DOWN) { pseudo_cost_sum_down[node_ptr->branch_var] += change_in_obj / frac; pseudo_cost_num_down[node_ptr->branch_var]++; } else { pseudo_cost_sum_up[node_ptr->branch_var] += change_in_obj / frac; pseudo_cost_num_up[node_ptr->branch_var]++; } }Based on learnings, keeping pseudo-cost state finite is important before transitioning/using it in diving/branching.
255-317: Fix potential crash whenfractionalis empty +score[-1]access (cppcheck Line 314).
branch_var = fractional[0]is UB iffractional.empty().selectstarts at-1, andlog.debug(... score[select])can dereferencescore[-1]if the loop doesn’t run (empty), which matches the static analysis warning.Proposed fix
i_t pseudo_costs_t<i_t, f_t>::variable_selection(const std::vector<i_t>& fractional, const std::vector<f_t>& solution, logger_t& log) { std::lock_guard<omp_mutex_t> lock(mutex); const i_t num_fractional = fractional.size(); + if (num_fractional == 0) { + log.debug("PC: no fractional variables; skipping pseudocost branching.\n"); + return -1; + } std::vector<f_t> pseudo_cost_up(num_fractional); std::vector<f_t> pseudo_cost_down(num_fractional); std::vector<f_t> score(num_fractional); @@ - i_t branch_var = fractional[0]; - f_t max_score = -1; - i_t select = -1; + i_t branch_var = -1; + f_t max_score = -std::numeric_limits<f_t>::infinity(); + i_t select = -1; for (i_t k = 0; k < num_fractional; k++) { if (score[k] > max_score) { max_score = score[k]; branch_var = fractional[k]; select = k; } } - log.debug("Pseudocost branching on %d. Value %e. Score %e.\n", - branch_var, - solution[branch_var], - score[select]); + if (select >= 0) { + log.debug("Pseudocost branching on %d. Value %e. Score %e.\n", + branch_var, + solution[branch_var], + score[select]); + } return branch_var; }As per coding guidelines, this is a correctness/stability issue in solver decision logic.
🤖 Fix all issues with AI agents
In @cpp/src/dual_simplex/pseudo_costs.cpp:
- Around line 319-362: The obj_estimate() logic misinterprets negative
pseudo-costs (e.g., pseudo_cost_down < 0) by forcing their contribution to eps,
which hides beneficial branches; fix by ensuring pseudo-costs are non-negative:
clamp pseudo_cost values to >= 0 when they are recorded in update() (or at the
point you compute them), and in obj_estimate() compute
pseudo_cost_down/pseudo_cost_up as std::max(original, (f_t)0) before multiplying
with f_down/f_up. Also reduce lock contention by copying the necessary
arrays/averages (pseudo_cost_sum_down, pseudo_cost_sum_up, pseudo_cost_num_down,
pseudo_cost_num_up, pseudo_cost_down_avg, pseudo_cost_up_avg) while holding
mutex and then releasing it before the loop that iterates fractional; keep
log.debug after the loop as now but using the local estimate value.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
cpp/src/dual_simplex/pseudo_costs.cpp
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{cu,cuh,cpp,hpp,h}
📄 CodeRabbit inference engine (.github/.coderabbit_review_guide.md)
**/*.{cu,cuh,cpp,hpp,h}: Track GPU device memory allocations and deallocations to prevent memory leaks; ensure cudaMalloc/cudaFree balance and cleanup of streams/events
Validate algorithm correctness in optimization logic: simplex pivots, branch-and-bound decisions, routing heuristics, and constraint/objective handling must produce correct results
Check numerical stability: prevent overflow/underflow, precision loss, division by zero/near-zero, and use epsilon comparisons for floating-point equality checks
Validate correct initialization of variable bounds, constraint coefficients, and algorithm state before solving; ensure reset when transitioning between algorithm phases (presolve, simplex, diving, crossover)
Ensure variables and constraints are accessed from the correct problem context (original vs presolve vs folded vs postsolve); verify index mapping consistency across problem transformations
For concurrent CUDA operations (barriers, async operations), explicitly create and manage dedicated streams instead of reusing the default stream; document stream lifecycle
Eliminate unnecessary host-device synchronization (cudaDeviceSynchronize) in hot paths that blocks GPU pipeline; use streams and events for async execution
Assess algorithmic complexity for large-scale problems (millions of variables/constraints); ensure O(n log n) or better complexity, not O(n²) or worse
Verify correct problem size checks before expensive GPU/CPU operations; prevent resource exhaustion on oversized problems
Identify assertions with overly strict numerical tolerances that fail on legitimate degenerate/edge cases (near-zero pivots, singular matrices, empty problems)
Ensure race conditions are absent in multi-GPU code and multi-threaded server implementations; verify proper synchronization of shared state
Refactor code duplication in solver components (3+ occurrences) into shared utilities; for GPU kernels, use templated device functions to avoid duplication
Check that hard-coded GPU de...
Files:
cpp/src/dual_simplex/pseudo_costs.cpp
**/*.{cpp,hpp,h}
📄 CodeRabbit inference engine (.github/.coderabbit_review_guide.md)
**/*.{cpp,hpp,h}: Check for unclosed file handles when reading MPS/QPS problem files; ensure RAII patterns or proper cleanup in exception paths
Validate input sanitization to prevent buffer overflows and resource exhaustion attacks; avoid unsafe deserialization of problem files
Prevent thread-unsafe use of global and static variables; use proper mutex/synchronization in server code accessing shared solver state
Files:
cpp/src/dual_simplex/pseudo_costs.cpp
**/*.{cu,cpp,hpp,h}
📄 CodeRabbit inference engine (.github/.coderabbit_review_guide.md)
Avoid inappropriate use of exceptions in performance-critical GPU operation paths; prefer error codes or CUDA error checking for latency-sensitive code
Files:
cpp/src/dual_simplex/pseudo_costs.cpp
🧠 Learnings (16)
📓 Common learnings
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Reduce tight coupling between solver components (presolve, simplex, basis, barrier); increase modularity and reusability of optimization algorithms
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Validate correct initialization of variable bounds, constraint coefficients, and algorithm state before solving; ensure reset when transitioning between algorithm phases (presolve, simplex, diving, crossover)
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Validate algorithm correctness in optimization logic: simplex pivots, branch-and-bound decisions, routing heuristics, and constraint/objective handling must produce correct results
Applied to files:
cpp/src/dual_simplex/pseudo_costs.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Validate correct initialization of variable bounds, constraint coefficients, and algorithm state before solving; ensure reset when transitioning between algorithm phases (presolve, simplex, diving, crossover)
Applied to files:
cpp/src/dual_simplex/pseudo_costs.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Ensure variables and constraints are accessed from the correct problem context (original vs presolve vs folded vs postsolve); verify index mapping consistency across problem transformations
Applied to files:
cpp/src/dual_simplex/pseudo_costs.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cpp,hpp,h} : Prevent thread-unsafe use of global and static variables; use proper mutex/synchronization in server code accessing shared solver state
Applied to files:
cpp/src/dual_simplex/pseudo_costs.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*test*.{cpp,cu,py} : Add tests for algorithm phase transitions: verify correct initialization of bounds and state when transitioning from presolve to simplex to diving to crossover
Applied to files:
cpp/src/dual_simplex/pseudo_costs.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Check that hard-coded GPU device IDs and resource limits are made configurable; abstract multi-backend support for different CUDA versions
Applied to files:
cpp/src/dual_simplex/pseudo_costs.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Track GPU device memory allocations and deallocations to prevent memory leaks; ensure cudaMalloc/cudaFree balance and cleanup of streams/events
Applied to files:
cpp/src/dual_simplex/pseudo_costs.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Ensure race conditions are absent in multi-GPU code and multi-threaded server implementations; verify proper synchronization of shared state
Applied to files:
cpp/src/dual_simplex/pseudo_costs.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cpp,hpp,h} : Avoid inappropriate use of exceptions in performance-critical GPU operation paths; prefer error codes or CUDA error checking for latency-sensitive code
Applied to files:
cpp/src/dual_simplex/pseudo_costs.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Verify error propagation from CUDA to user-facing APIs is complete; ensure CUDA errors are caught and mapped to meaningful user error codes
Applied to files:
cpp/src/dual_simplex/pseudo_costs.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Refactor code duplication in solver components (3+ occurrences) into shared utilities; for GPU kernels, use templated device functions to avoid duplication
Applied to files:
cpp/src/dual_simplex/pseudo_costs.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.cu : Verify race conditions and correctness of GPU kernel shared memory, atomics, and warp-level operations
Applied to files:
cpp/src/dual_simplex/pseudo_costs.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Verify correct problem size checks before expensive GPU/CPU operations; prevent resource exhaustion on oversized problems
Applied to files:
cpp/src/dual_simplex/pseudo_costs.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Eliminate unnecessary host-device synchronization (cudaDeviceSynchronize) in hot paths that blocks GPU pipeline; use streams and events for async execution
Applied to files:
cpp/src/dual_simplex/pseudo_costs.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : For concurrent CUDA operations (barriers, async operations), explicitly create and manage dedicated streams instead of reusing the default stream; document stream lifecycle
Applied to files:
cpp/src/dual_simplex/pseudo_costs.cpp
🧬 Code graph analysis (1)
cpp/src/dual_simplex/pseudo_costs.cpp (1)
cpp/src/dual_simplex/pseudo_costs.hpp (4)
num_initialized_down(41-44)fractional(46-48)fractional(50-53)fractional(55-56)
🪛 Cppcheck (2.19.0)
cpp/src/dual_simplex/pseudo_costs.cpp
[warning] 314-314: Array index -1 is out of bounds.
(negativeContainerIndex)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: wheel-build-cuopt-mps-parser / 13.1.0, 3.11, arm64, rockylinux8
- GitHub Check: wheel-build-cuopt-mps-parser / 13.1.0, 3.12, arm64, rockylinux8
- GitHub Check: wheel-build-cuopt-mps-parser / 13.1.0, 3.10, arm64, rockylinux8
- GitHub Check: wheel-build-cuopt-mps-parser / 13.1.0, 3.11, amd64, rockylinux8
- GitHub Check: wheel-build-cuopt-mps-parser / 13.1.0, 3.13, arm64, rockylinux8
- GitHub Check: wheel-build-cuopt-mps-parser / 13.1.0, 3.10, amd64, rockylinux8
- GitHub Check: wheel-build-cuopt-mps-parser / 13.1.0, 3.12, amd64, rockylinux8
- GitHub Check: wheel-build-cuopt-mps-parser / 13.1.0, 3.13, amd64, rockylinux8
- GitHub Check: wheel-build-cuopt-sh-client / 13.1.0, 3.10, amd64, rockylinux8
- GitHub Check: checks / check-style
🔇 Additional comments (1)
cpp/src/dual_simplex/pseudo_costs.cpp (1)
3-3: Copyright year bump looks fine.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @cpp/src/dual_simplex/branch_and_bound.cpp:
- Around line 22-23: exploration_stats_ is mutated concurrently by OpenMP tasks
causing data races; add a mutex (e.g., mutex_stats_) as a member and use it to
guard all reads/writes to exploration_stats_ in parallel regions (wrap accesses
in exploration_ramp_up, solve_node and any task bodies with
std::lock_guard<std::mutex> or std::scoped_lock), mirroring the pattern used by
mutex_upper_ for upper_bound_, and ensure increments/decrements of
nodes_unexplored, nodes_explored, nodes_since_last_log and updates to last_log
are performed while holding the lock.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
cpp/src/dual_simplex/branch_and_bound.cppcpp/src/mip/diversity/lns/rins.cu
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{cu,cuh}
📄 CodeRabbit inference engine (.github/.coderabbit_review_guide.md)
**/*.{cu,cuh}: Every CUDA kernel launch and memory operation must have error checking with CUDA_CHECK or equivalent verification
Avoid reinventing functionality already available in Thrust, CCCL, or RMM libraries; prefer standard library utilities over custom implementations
Files:
cpp/src/mip/diversity/lns/rins.cu
**/*.cu
📄 CodeRabbit inference engine (.github/.coderabbit_review_guide.md)
**/*.cu: Verify race conditions and correctness of GPU kernel shared memory, atomics, and warp-level operations
Detect inefficient GPU kernel launches with low occupancy or poor memory access patterns; optimize for coalesced memory access and minimize warp divergence in hot paths
Files:
cpp/src/mip/diversity/lns/rins.cu
**/*.{cu,cuh,cpp,hpp,h}
📄 CodeRabbit inference engine (.github/.coderabbit_review_guide.md)
**/*.{cu,cuh,cpp,hpp,h}: Track GPU device memory allocations and deallocations to prevent memory leaks; ensure cudaMalloc/cudaFree balance and cleanup of streams/events
Validate algorithm correctness in optimization logic: simplex pivots, branch-and-bound decisions, routing heuristics, and constraint/objective handling must produce correct results
Check numerical stability: prevent overflow/underflow, precision loss, division by zero/near-zero, and use epsilon comparisons for floating-point equality checks
Validate correct initialization of variable bounds, constraint coefficients, and algorithm state before solving; ensure reset when transitioning between algorithm phases (presolve, simplex, diving, crossover)
Ensure variables and constraints are accessed from the correct problem context (original vs presolve vs folded vs postsolve); verify index mapping consistency across problem transformations
For concurrent CUDA operations (barriers, async operations), explicitly create and manage dedicated streams instead of reusing the default stream; document stream lifecycle
Eliminate unnecessary host-device synchronization (cudaDeviceSynchronize) in hot paths that blocks GPU pipeline; use streams and events for async execution
Assess algorithmic complexity for large-scale problems (millions of variables/constraints); ensure O(n log n) or better complexity, not O(n²) or worse
Verify correct problem size checks before expensive GPU/CPU operations; prevent resource exhaustion on oversized problems
Identify assertions with overly strict numerical tolerances that fail on legitimate degenerate/edge cases (near-zero pivots, singular matrices, empty problems)
Ensure race conditions are absent in multi-GPU code and multi-threaded server implementations; verify proper synchronization of shared state
Refactor code duplication in solver components (3+ occurrences) into shared utilities; for GPU kernels, use templated device functions to avoid duplication
Check that hard-coded GPU de...
Files:
cpp/src/mip/diversity/lns/rins.cucpp/src/dual_simplex/branch_and_bound.cpp
**/*.{cu,cpp,hpp,h}
📄 CodeRabbit inference engine (.github/.coderabbit_review_guide.md)
Avoid inappropriate use of exceptions in performance-critical GPU operation paths; prefer error codes or CUDA error checking for latency-sensitive code
Files:
cpp/src/mip/diversity/lns/rins.cucpp/src/dual_simplex/branch_and_bound.cpp
**/*.{cpp,hpp,h}
📄 CodeRabbit inference engine (.github/.coderabbit_review_guide.md)
**/*.{cpp,hpp,h}: Check for unclosed file handles when reading MPS/QPS problem files; ensure RAII patterns or proper cleanup in exception paths
Validate input sanitization to prevent buffer overflows and resource exhaustion attacks; avoid unsafe deserialization of problem files
Prevent thread-unsafe use of global and static variables; use proper mutex/synchronization in server code accessing shared solver state
Files:
cpp/src/dual_simplex/branch_and_bound.cpp
🧠 Learnings (22)
📓 Common learnings
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Reduce tight coupling between solver components (presolve, simplex, basis, barrier); increase modularity and reusability of optimization algorithms
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Validate correct initialization of variable bounds, constraint coefficients, and algorithm state before solving; ensure reset when transitioning between algorithm phases (presolve, simplex, diving, crossover)
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Validate correct initialization of variable bounds, constraint coefficients, and algorithm state before solving; ensure reset when transitioning between algorithm phases (presolve, simplex, diving, crossover)
Applied to files:
cpp/src/mip/diversity/lns/rins.cucpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Check that hard-coded GPU device IDs and resource limits are made configurable; abstract multi-backend support for different CUDA versions
Applied to files:
cpp/src/mip/diversity/lns/rins.cu
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Refactor code duplication in solver components (3+ occurrences) into shared utilities; for GPU kernels, use templated device functions to avoid duplication
Applied to files:
cpp/src/mip/diversity/lns/rins.cucpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Ensure race conditions are absent in multi-GPU code and multi-threaded server implementations; verify proper synchronization of shared state
Applied to files:
cpp/src/mip/diversity/lns/rins.cu
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cpp,hpp,h} : Prevent thread-unsafe use of global and static variables; use proper mutex/synchronization in server code accessing shared solver state
Applied to files:
cpp/src/mip/diversity/lns/rins.cu
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Validate algorithm correctness in optimization logic: simplex pivots, branch-and-bound decisions, routing heuristics, and constraint/objective handling must produce correct results
Applied to files:
cpp/src/mip/diversity/lns/rins.cucpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.cu : Verify race conditions and correctness of GPU kernel shared memory, atomics, and warp-level operations
Applied to files:
cpp/src/mip/diversity/lns/rins.cu
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Ensure variables and constraints are accessed from the correct problem context (original vs presolve vs folded vs postsolve); verify index mapping consistency across problem transformations
Applied to files:
cpp/src/mip/diversity/lns/rins.cucpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Verify correct problem size checks before expensive GPU/CPU operations; prevent resource exhaustion on oversized problems
Applied to files:
cpp/src/mip/diversity/lns/rins.cucpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : For concurrent CUDA operations (barriers, async operations), explicitly create and manage dedicated streams instead of reusing the default stream; document stream lifecycle
Applied to files:
cpp/src/mip/diversity/lns/rins.cu
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Eliminate unnecessary host-device synchronization (cudaDeviceSynchronize) in hot paths that blocks GPU pipeline; use streams and events for async execution
Applied to files:
cpp/src/mip/diversity/lns/rins.cu
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cpp,hpp,h} : Avoid inappropriate use of exceptions in performance-critical GPU operation paths; prefer error codes or CUDA error checking for latency-sensitive code
Applied to files:
cpp/src/mip/diversity/lns/rins.cucpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Reduce tight coupling between solver components (presolve, simplex, basis, barrier); increase modularity and reusability of optimization algorithms
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Identify assertions with overly strict numerical tolerances that fail on legitimate degenerate/edge cases (near-zero pivots, singular matrices, empty problems)
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*test*.{cpp,cu,py} : Test with free variables, singleton problems, and extreme problem dimensions near resource limits to validate edge case handling
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Verify error propagation from CUDA to user-facing APIs is complete; ensure CUDA errors are caught and mapped to meaningful user error codes
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Assess algorithmic complexity for large-scale problems (millions of variables/constraints); ensure O(n log n) or better complexity, not O(n²) or worse
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Check numerical stability: prevent overflow/underflow, precision loss, division by zero/near-zero, and use epsilon comparisons for floating-point equality checks
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*test*.{cpp,cu,py} : Add tests for algorithm phase transitions: verify correct initialization of bounds and state when transitioning from presolve to simplex to diving to crossover
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-12-04T20:09:09.264Z
Learnt from: chris-maes
Repo: NVIDIA/cuopt PR: 602
File: cpp/src/linear_programming/solve.cu:732-742
Timestamp: 2025-12-04T20:09:09.264Z
Learning: In cpp/src/linear_programming/solve.cu, the barrier solver does not currently return INFEASIBLE or UNBOUNDED status. It only returns OPTIMAL, TIME_LIMIT, NUMERICAL_ISSUES, or CONCURRENT_LIMIT.
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-12-04T04:11:12.640Z
Learnt from: chris-maes
Repo: NVIDIA/cuopt PR: 500
File: cpp/src/dual_simplex/scaling.cpp:68-76
Timestamp: 2025-12-04T04:11:12.640Z
Learning: In the cuOPT dual simplex solver, CSR/CSC matrices (including the quadratic objective matrix Q) are required to have valid dimensions and indices by construction. Runtime bounds checking in performance-critical paths like matrix scaling is avoided to prevent slowdowns. Validation is performed via debug-only check_matrix() calls wrapped in #ifdef CHECK_MATRIX.
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cpp
🧬 Code graph analysis (2)
cpp/src/mip/diversity/lns/rins.cu (1)
cpp/src/mip/solver.cu (6)
solution(64-68)solution(64-64)solution(70-75)solution(70-72)solution(77-80)solution(77-77)
cpp/src/dual_simplex/branch_and_bound.cpp (1)
cpp/src/dual_simplex/branch_and_bound.hpp (6)
node_ptr(250-262)node_ptr(265-269)solution(116-116)solution(134-134)solution(204-204)node(218-218)
🪛 Clang (14.0.6)
cpp/src/dual_simplex/branch_and_bound.cpp
[error] 8-8: 'dual_simplex/branch_and_bound.hpp' file not found
(clang-diagnostic-error)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: wheel-build-cuopt-sh-client / 13.1.0, 3.10, amd64, rockylinux8
- GitHub Check: wheel-build-cuopt-mps-parser / 13.1.0, 3.10, amd64, rockylinux8
- GitHub Check: wheel-build-cuopt-mps-parser / 13.1.0, 3.11, amd64, rockylinux8
- GitHub Check: wheel-build-cuopt-mps-parser / 13.1.0, 3.12, arm64, rockylinux8
- GitHub Check: wheel-build-cuopt-mps-parser / 13.1.0, 3.13, amd64, rockylinux8
- GitHub Check: wheel-build-cuopt-mps-parser / 13.1.0, 3.11, arm64, rockylinux8
- GitHub Check: wheel-build-cuopt-mps-parser / 13.1.0, 3.10, arm64, rockylinux8
- GitHub Check: wheel-build-cuopt-mps-parser / 13.1.0, 3.12, amd64, rockylinux8
- GitHub Check: wheel-build-cuopt-mps-parser / 13.1.0, 3.13, arm64, rockylinux8
🔇 Additional comments (24)
cpp/src/dual_simplex/branch_and_bound.cpp (20)
197-207: LGTM – Worker type-aware logging symbols.The function correctly maps worker types to logging symbols. All diving strategies share "D " while exploration uses "B ", which aligns with the unified diving framework.
253-262: LGTM – Node queue integration for lower bound tracking.The method correctly computes the lower bound using
node_queue.get_lower_bound()and combines it with local thread bounds and the ceiling. The fallback to-inffor non-finite values is appropriate.
264-283: LGTM – Heuristic solution reporting.The method appropriately formats and logs heuristic solutions with different output based on solver state. The gap calculation and user objective conversion are correct.
286-307: LGTM – Consolidated progress reporting.The method centralizes reporting logic with clear formatting. The statistics computation (iterations per node, gap calculation) is correct and aligns with the updated logging format mentioned in PR objectives.
561-580: LGTM – Martin's criteria implementation.The function correctly implements Martin's preferred rounding direction criterion from the cited reference. The epsilon-based comparison (1e-6) provides appropriate tie-breaking for near-equidistant cases.
583-621: LGTM – Worker type-aware variable selection.The method correctly dispatches to different branching strategies based on worker type. The exploration path appropriately combines pseudocost selection with Martin's criteria, while diving workers use their respective heuristics. The objective estimate computation (lines 601-602) is correctly scoped to exploration workers only.
643-649: LGTM – Guided diving fallback when no incumbent.The fallback logic correctly switches guided diving to pseudocost diving (or coefficient diving if pseudocost is disabled) when no incumbent solution exists. This is necessary since guided diving requires an incumbent to guide the search.
779-785: LGTM – Appropriate callback scoping for exploration workers.Restricting the
node_processed_callbackto exploration workers is correct, as diving workers are speculative and their node explorations shouldn't trigger external callbacks.
800-802: LGTM – Defensive assertions after variable selection.The assertions correctly validate that
variable_selectionreturned a valid branching variable and rounding direction, catching potential logic errors in the selection strategies.
941-1053: LGTM – Best-first plunging implementation.The
plunge_frommethod correctly implements best-first search with depth-first plunging. Key strengths:
- Proper basis reuse tracking via
recompute_bounds_and_basis- Local lower bound updates for thread-specific tracking
- Stack-based sibling management with global queue overflow
- Appropriate time and node limit checks
1056-1118: LGTM – Best-first search worker thread.The method implements a proper worker thread for best-first search:
- Correct work availability check combining
active_subtrees_and queue size- Appropriate cutoff check before plunging (lines 1080-1088)
- Proper completion detection when all threads finish (lines 1111-1117)
1121-1197: LGTM – Diving heuristic implementation with proper limits.The
dive_frommethod correctly implements diving heuristics with:
- Isolated subtree for speculative exploration (line 1135)
- Separate statistics tracking to avoid contaminating exploration metrics
- Node, time, and iteration limit enforcement (lines 1157, 1156, 1179-1180)
- Backtrack limit based on depth (lines 1192-1195) to force branch exploration
The implementation aligns well with the diving framework described in PR objectives.
1200-1245: LGTM – Diving worker thread with proper bound management.The diving thread correctly manages variable bounds across multiple dives:
- Starting bounds are reset between dives (lines 1218-1223)
pop_divingpopulates bounds along the node's path (line 1226)- Bounds strengthening is applied before diving (line 1231)
- Feasibility check prevents diving into infeasible space (line 1232)
1331-1331: LGTM – Arow_ properly initialized before parallel execution.The constraint matrix is correctly converted to compressed row format and stored in
Arow_before spawning worker threads, ensuring all threads have access to properly initialized data.
1333-1354: LGTM – Flexible diving strategy configuration.The conditional assembly of
diving_strategiesbased on settings flags (lines 1336-1350) allows fine-grained control over which diving heuristics are active. The warning when all strategies are disabled (lines 1352-1354) helps catch configuration errors.
1499-1503: LGTM – Clear threading configuration logging.The log message clearly communicates the thread allocation between best-first and diving workers, aiding in performance tuning and debugging.
1513-1515: LGTM – Table header updated for two-letter symbols.The table header format correctly accommodates two-letter symbols (line 1513 shows " |" at start), aligning with PR objective #714.
1517-1548: LGTM – OpenMP task-based parallel branch-and-bound.The parallel structure correctly uses:
- Master section for centralized task spawning (line 1519)
- Taskgroup to synchronize ramp-up before launching worker tasks (lines 1526-1533)
- Separate task loops for BFS workers (lines 1535-1538) and diving workers (lines 1540-1546)
- Round-robin assignment of diving strategies (line 1542)
The structure ensures proper initialization order and work distribution.
662-667: Iteration limit for diving workers is intentionally proportional to exploration effort.The 5%
iteration_limit_factor(lines 664–665) is a designed trade-off where diving heuristics receive a budget proportional to the main exploration phase's iteration count. This is a reasonable resource allocation strategy, not a bug. Diving is secondary to branch-and-bound exploration and starts only aftermin_node_depth = 5, mitigating the early-search constraint you noted.If you believe accounting for dive depth or objective improvement would yield better heuristic quality, that would be a valid optimization to explore, but the current behavior is correct and intentional.
224-224: No action required.Arow_is correctly initialized at line 1331 viaoriginal_lp_.A.to_compressed_row(Arow_)before OpenMP tasks are spawned. The worker methods (exploration_ramp_up,best_first_thread,diving_thread) that useArow_are invoked only after this initialization.Likely an incorrect or invalid review comment.
cpp/src/mip/diversity/lns/rins.cu (4)
263-268: LGTM – Intentional restriction to guided diving for RINS.The configuration appropriately restricts RINS sub-MIPs to guided diving only, with a clear comment (lines 263-264) indicating this is a deliberate temporary choice with plans for future expansion. This is reasonable for RINS since:
- RINS problems are already restricted by variable fixing
- Guided diving can leverage the incumbent from the main MIP
- Reducing diving diversity decreases overhead in frequent RINS calls
260-261: LGTM – Threading configuration updated to new API.The changes correctly update from the deprecated
num_bfs_threadstonum_bfs_workers(line 261) and introduce the structureddiving_settings.num_diving_workers(line 265), maintaining the same 2-thread configuration (1 BFS + 1 diving).
271-274: LGTM – Solution callback correctly captures queue.The lambda callback correctly captures
rins_solution_queueby reference and accepts the standard two-parameter signature (solution,objective), consistent with the callback pattern shown incpp/src/mip/solver.culines 63-67.
259-268: No action needed—the fallback concern does not apply here.RINS always calls
set_initial_guess()beforesolve()(line 277), providing the refined incumbent as the initial guess. This ensuresupper_bound != infwhen the fallback logic at branch_and_bound.cpp lines 643–649 is evaluated, so the fallback case (no incumbent) never occurs. The configuration is correct because RINS is a refinement heuristic that inherently operates with an incumbent solution.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
cpp/src/dual_simplex/branch_and_bound.cpp (1)
801-806: Add defensive check for emptyfractionallist.If
variable_selectionreturnsbranch_var = -1(e.g., due to an emptyfractionallist in an edge case), the assertions on lines 805-806 will trigger. Consider adding a defensive check before the assertions or ensuringfractionalis never empty at this point.♻️ Suggested defensive check
// Choose fractional variable to branch on auto [branch_var, round_dir] = variable_selection(node_ptr, leaf_fractional, leaf_solution.x, thread_type); assert(leaf_vstatus.size() == leaf_problem.num_cols); - assert(branch_var >= 0); - assert(round_dir != rounding_direction_t::NONE); + if (branch_var < 0 || round_dir == rounding_direction_t::NONE) { + log.printf("Warning: variable_selection failed for node %d\n", node_ptr->node_id); + search_tree.update(node_ptr, node_status_t::NUMERICAL); + return node_solve_info_t::NUMERICAL; + }cpp/src/dual_simplex/node_queue.hpp (1)
144-153: Potential performance concern: diving heap traversal under lock.The
while (!diving_heap.empty())loop pops entries until finding one with a non-null node. With many consumed entries, this could hold the lock for extended periods, blocking other threads.Consider periodically cleaning up consumed entries or using a separate cleanup mechanism to reduce contention. However, this is a minor concern if the ratio of consumed entries is typically low.
cpp/src/dual_simplex/diving_heuristics.cpp (2)
28-38: Potential division by near-zero in line search diving.When
solution[j]is very close toroot_solution[j](withineps), neither branch is taken andscoreremainsinf. However, if|solution[j] - root_solution[j]|is small but >eps,dcould be very small, causing numerical instability inscore = f / d.♻️ Add minimum denominator check
if (solution[j] < root_solution[j] - eps) { f_t f = solution[j] - std::floor(solution[j]); f_t d = root_solution[j] - solution[j]; - score = f / d; + score = d > eps ? f / d : inf; dir = rounding_direction_t::DOWN; } else if (solution[j] > root_solution[j] + eps) { f_t f = std::ceil(solution[j]) - solution[j]; f_t d = solution[j] - root_solution[j]; - score = f / d; + score = d > eps ? f / d : inf; dir = rounding_direction_t::UP; }
241-260: Unused variableepsin coefficient diving.The variable
epsis declared on line 239 but never used in the function.♻️ Either use eps in tie-breaking or remove it
i_t branch_var = -1; i_t min_locks = std::numeric_limits<i_t>::max(); rounding_direction_t round_dir = rounding_direction_t::NONE; - constexpr f_t eps = 1e-6; for (auto j : fractional) { f_t f_down = solution[j] - std::floor(solution[j]); f_t f_up = std::ceil(solution[j]) - solution[j]; auto [up_lock, down_lock] = calculate_variable_locks(lp_problem, j); i_t locks = std::min(up_lock, down_lock); if (min_locks > locks) { min_locks = locks; branch_var = j; if (up_lock < down_lock) { round_dir = rounding_direction_t::UP; } else if (up_lock > down_lock) { round_dir = rounding_direction_t::DOWN; - } else if (f_down < f_up + eps) { + } else if (f_down < f_up) { round_dir = rounding_direction_t::DOWN; } else {
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
cpp/src/dual_simplex/branch_and_bound.cppcpp/src/dual_simplex/branch_and_bound.hppcpp/src/dual_simplex/diving_heuristics.cppcpp/src/dual_simplex/node_queue.hppcpp/src/dual_simplex/simplex_solver_settings.hpp
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{cu,cuh,cpp,hpp,h}
📄 CodeRabbit inference engine (.github/.coderabbit_review_guide.md)
**/*.{cu,cuh,cpp,hpp,h}: Track GPU device memory allocations and deallocations to prevent memory leaks; ensure cudaMalloc/cudaFree balance and cleanup of streams/events
Validate algorithm correctness in optimization logic: simplex pivots, branch-and-bound decisions, routing heuristics, and constraint/objective handling must produce correct results
Check numerical stability: prevent overflow/underflow, precision loss, division by zero/near-zero, and use epsilon comparisons for floating-point equality checks
Validate correct initialization of variable bounds, constraint coefficients, and algorithm state before solving; ensure reset when transitioning between algorithm phases (presolve, simplex, diving, crossover)
Ensure variables and constraints are accessed from the correct problem context (original vs presolve vs folded vs postsolve); verify index mapping consistency across problem transformations
For concurrent CUDA operations (barriers, async operations), explicitly create and manage dedicated streams instead of reusing the default stream; document stream lifecycle
Eliminate unnecessary host-device synchronization (cudaDeviceSynchronize) in hot paths that blocks GPU pipeline; use streams and events for async execution
Assess algorithmic complexity for large-scale problems (millions of variables/constraints); ensure O(n log n) or better complexity, not O(n²) or worse
Verify correct problem size checks before expensive GPU/CPU operations; prevent resource exhaustion on oversized problems
Identify assertions with overly strict numerical tolerances that fail on legitimate degenerate/edge cases (near-zero pivots, singular matrices, empty problems)
Ensure race conditions are absent in multi-GPU code and multi-threaded server implementations; verify proper synchronization of shared state
Refactor code duplication in solver components (3+ occurrences) into shared utilities; for GPU kernels, use templated device functions to avoid duplication
Check that hard-coded GPU de...
Files:
cpp/src/dual_simplex/branch_and_bound.cppcpp/src/dual_simplex/simplex_solver_settings.hppcpp/src/dual_simplex/branch_and_bound.hppcpp/src/dual_simplex/diving_heuristics.cppcpp/src/dual_simplex/node_queue.hpp
**/*.{cpp,hpp,h}
📄 CodeRabbit inference engine (.github/.coderabbit_review_guide.md)
**/*.{cpp,hpp,h}: Check for unclosed file handles when reading MPS/QPS problem files; ensure RAII patterns or proper cleanup in exception paths
Validate input sanitization to prevent buffer overflows and resource exhaustion attacks; avoid unsafe deserialization of problem files
Prevent thread-unsafe use of global and static variables; use proper mutex/synchronization in server code accessing shared solver state
Files:
cpp/src/dual_simplex/branch_and_bound.cppcpp/src/dual_simplex/simplex_solver_settings.hppcpp/src/dual_simplex/branch_and_bound.hppcpp/src/dual_simplex/diving_heuristics.cppcpp/src/dual_simplex/node_queue.hpp
**/*.{cu,cpp,hpp,h}
📄 CodeRabbit inference engine (.github/.coderabbit_review_guide.md)
Avoid inappropriate use of exceptions in performance-critical GPU operation paths; prefer error codes or CUDA error checking for latency-sensitive code
Files:
cpp/src/dual_simplex/branch_and_bound.cppcpp/src/dual_simplex/simplex_solver_settings.hppcpp/src/dual_simplex/branch_and_bound.hppcpp/src/dual_simplex/diving_heuristics.cppcpp/src/dual_simplex/node_queue.hpp
**/*.{h,hpp,py}
📄 CodeRabbit inference engine (.github/.coderabbit_review_guide.md)
Verify C API does not break ABI stability (no struct layout changes, field reordering); maintain backward compatibility in Python and server APIs with deprecation warnings
Files:
cpp/src/dual_simplex/simplex_solver_settings.hppcpp/src/dual_simplex/branch_and_bound.hppcpp/src/dual_simplex/node_queue.hpp
🧠 Learnings (23)
📓 Common learnings
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Reduce tight coupling between solver components (presolve, simplex, basis, barrier); increase modularity and reusability of optimization algorithms
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Validate algorithm correctness in optimization logic: simplex pivots, branch-and-bound decisions, routing heuristics, and constraint/objective handling must produce correct results
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cppcpp/src/dual_simplex/simplex_solver_settings.hppcpp/src/dual_simplex/branch_and_bound.hppcpp/src/dual_simplex/diving_heuristics.cppcpp/src/dual_simplex/node_queue.hpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Validate correct initialization of variable bounds, constraint coefficients, and algorithm state before solving; ensure reset when transitioning between algorithm phases (presolve, simplex, diving, crossover)
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cppcpp/src/dual_simplex/simplex_solver_settings.hppcpp/src/dual_simplex/branch_and_bound.hppcpp/src/dual_simplex/diving_heuristics.cppcpp/src/dual_simplex/node_queue.hpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*test*.{cpp,cu,py} : Add tests for algorithm phase transitions: verify correct initialization of bounds and state when transitioning from presolve to simplex to diving to crossover
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cppcpp/src/dual_simplex/simplex_solver_settings.hppcpp/src/dual_simplex/branch_and_bound.hppcpp/src/dual_simplex/diving_heuristics.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Reduce tight coupling between solver components (presolve, simplex, basis, barrier); increase modularity and reusability of optimization algorithms
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cppcpp/src/dual_simplex/simplex_solver_settings.hppcpp/src/dual_simplex/branch_and_bound.hpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Ensure variables and constraints are accessed from the correct problem context (original vs presolve vs folded vs postsolve); verify index mapping consistency across problem transformations
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cppcpp/src/dual_simplex/branch_and_bound.hppcpp/src/dual_simplex/diving_heuristics.cppcpp/src/dual_simplex/node_queue.hpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Verify correct problem size checks before expensive GPU/CPU operations; prevent resource exhaustion on oversized problems
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cppcpp/src/dual_simplex/simplex_solver_settings.hpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Identify assertions with overly strict numerical tolerances that fail on legitimate degenerate/edge cases (near-zero pivots, singular matrices, empty problems)
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cppcpp/src/dual_simplex/diving_heuristics.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*test*.{cpp,cu,py} : Test with free variables, singleton problems, and extreme problem dimensions near resource limits to validate edge case handling
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cppcpp/src/dual_simplex/node_queue.hpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cpp,hpp,h} : Avoid inappropriate use of exceptions in performance-critical GPU operation paths; prefer error codes or CUDA error checking for latency-sensitive code
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cppcpp/src/dual_simplex/simplex_solver_settings.hpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Verify error propagation from CUDA to user-facing APIs is complete; ensure CUDA errors are caught and mapped to meaningful user error codes
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cppcpp/src/dual_simplex/simplex_solver_settings.hpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Check numerical stability: prevent overflow/underflow, precision loss, division by zero/near-zero, and use epsilon comparisons for floating-point equality checks
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cppcpp/src/dual_simplex/diving_heuristics.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cpp,hpp,h} : Prevent thread-unsafe use of global and static variables; use proper mutex/synchronization in server code accessing shared solver state
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cppcpp/src/dual_simplex/node_queue.hpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Ensure race conditions are absent in multi-GPU code and multi-threaded server implementations; verify proper synchronization of shared state
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cppcpp/src/dual_simplex/simplex_solver_settings.hppcpp/src/dual_simplex/node_queue.hpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.cu : Verify race conditions and correctness of GPU kernel shared memory, atomics, and warp-level operations
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cppcpp/src/dual_simplex/simplex_solver_settings.hpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Refactor code duplication in solver components (3+ occurrences) into shared utilities; for GPU kernels, use templated device functions to avoid duplication
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cppcpp/src/dual_simplex/simplex_solver_settings.hppcpp/src/dual_simplex/branch_and_bound.hppcpp/src/dual_simplex/diving_heuristics.cppcpp/src/dual_simplex/node_queue.hpp
📚 Learning: 2025-12-04T20:09:09.264Z
Learnt from: chris-maes
Repo: NVIDIA/cuopt PR: 602
File: cpp/src/linear_programming/solve.cu:732-742
Timestamp: 2025-12-04T20:09:09.264Z
Learning: In cpp/src/linear_programming/solve.cu, the barrier solver does not currently return INFEASIBLE or UNBOUNDED status. It only returns OPTIMAL, TIME_LIMIT, NUMERICAL_ISSUES, or CONCURRENT_LIMIT.
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cppcpp/src/dual_simplex/simplex_solver_settings.hppcpp/src/dual_simplex/branch_and_bound.hppcpp/src/dual_simplex/diving_heuristics.cppcpp/src/dual_simplex/node_queue.hpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Assess algorithmic complexity for large-scale problems (millions of variables/constraints); ensure O(n log n) or better complexity, not O(n²) or worse
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cppcpp/src/dual_simplex/branch_and_bound.hppcpp/src/dual_simplex/diving_heuristics.cpp
📚 Learning: 2025-12-04T04:11:12.640Z
Learnt from: chris-maes
Repo: NVIDIA/cuopt PR: 500
File: cpp/src/dual_simplex/scaling.cpp:68-76
Timestamp: 2025-12-04T04:11:12.640Z
Learning: In the cuOPT dual simplex solver, CSR/CSC matrices (including the quadratic objective matrix Q) are required to have valid dimensions and indices by construction. Runtime bounds checking in performance-critical paths like matrix scaling is avoided to prevent slowdowns. Validation is performed via debug-only check_matrix() calls wrapped in #ifdef CHECK_MATRIX.
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cppcpp/src/dual_simplex/simplex_solver_settings.hppcpp/src/dual_simplex/branch_and_bound.hpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Check that hard-coded GPU device IDs and resource limits are made configurable; abstract multi-backend support for different CUDA versions
Applied to files:
cpp/src/dual_simplex/simplex_solver_settings.hpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Track GPU device memory allocations and deallocations to prevent memory leaks; ensure cudaMalloc/cudaFree balance and cleanup of streams/events
Applied to files:
cpp/src/dual_simplex/simplex_solver_settings.hpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Eliminate unnecessary host-device synchronization (cudaDeviceSynchronize) in hot paths that blocks GPU pipeline; use streams and events for async execution
Applied to files:
cpp/src/dual_simplex/simplex_solver_settings.hpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : For concurrent CUDA operations (barriers, async operations), explicitly create and manage dedicated streams instead of reusing the default stream; document stream lifecycle
Applied to files:
cpp/src/dual_simplex/simplex_solver_settings.hpp
🧬 Code graph analysis (4)
cpp/src/dual_simplex/branch_and_bound.cpp (7)
cpp/src/dual_simplex/node_queue.hpp (9)
buffer(47-55)buffer(57-57)buffer(58-58)buffer(59-59)buffer(60-60)node(28-32)node(28-28)node(34-38)node(34-34)cpp/src/dual_simplex/branch_and_bound.hpp (8)
obj(200-200)symbol(201-201)node_ptr(249-261)node_ptr(264-267)solution(116-116)solution(134-134)solution(204-204)node(218-218)cpp/src/dual_simplex/solve.cpp (7)
compute_user_objective(88-93)compute_user_objective(88-88)compute_user_objective(96-100)compute_user_objective(96-96)compute_user_objective(661-662)compute_user_objective(664-664)basic_list(113-113)cpp/src/dual_simplex/solve.hpp (2)
compute_user_objective(37-37)compute_user_objective(40-40)cpp/src/dual_simplex/pseudo_costs.hpp (4)
node_ptr(31-31)fractional(46-48)fractional(50-53)fractional(55-56)cpp/src/dual_simplex/diving_heuristics.hpp (3)
line_search_diving(24-27)pseudocost_diving(30-34)guided_diving(37-41)cpp/src/dual_simplex/presolve.hpp (1)
uncrush_primal_solution(162-165)
cpp/src/dual_simplex/branch_and_bound.hpp (2)
cpp/src/dual_simplex/node_queue.hpp (4)
node(28-32)node(28-28)node(34-38)node(34-34)cpp/src/dual_simplex/pseudo_costs.hpp (4)
node_ptr(31-31)fractional(46-48)fractional(50-53)fractional(55-56)
cpp/src/dual_simplex/diving_heuristics.cpp (1)
cpp/src/dual_simplex/pseudo_costs.hpp (4)
fractional(46-48)fractional(50-53)fractional(55-56)num_initialized_down(41-44)
cpp/src/dual_simplex/node_queue.hpp (1)
cpp/src/dual_simplex/mip_node.hpp (4)
lower(97-108)lower(97-99)lower(112-130)lower(112-114)
🪛 Clang (14.0.6)
cpp/src/dual_simplex/branch_and_bound.cpp
[error] 8-8: 'dual_simplex/branch_and_bound.hpp' file not found
(clang-diagnostic-error)
cpp/src/dual_simplex/branch_and_bound.hpp
[error] 10-10: 'dual_simplex/diving_heuristics.hpp' file not found
(clang-diagnostic-error)
cpp/src/dual_simplex/diving_heuristics.cpp
[error] 8-8: 'dual_simplex/diving_heuristics.hpp' file not found
(clang-diagnostic-error)
cpp/src/dual_simplex/node_queue.hpp
[error] 8-8: 'algorithm' file not found
(clang-diagnostic-error)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: wheel-build-cuopt-mps-parser / 13.1.0, 3.12, amd64, rockylinux8
- GitHub Check: wheel-build-cuopt-mps-parser / 13.1.0, 3.11, amd64, rockylinux8
- GitHub Check: wheel-build-cuopt-mps-parser / 13.1.0, 3.12, arm64, rockylinux8
- GitHub Check: wheel-build-cuopt-mps-parser / 13.1.0, 3.10, arm64, rockylinux8
- GitHub Check: wheel-build-cuopt-mps-parser / 13.1.0, 3.13, arm64, rockylinux8
- GitHub Check: wheel-build-cuopt-mps-parser / 13.1.0, 3.11, arm64, rockylinux8
- GitHub Check: wheel-build-cuopt-sh-client / 13.1.0, 3.10, amd64, rockylinux8
- GitHub Check: wheel-build-cuopt-mps-parser / 13.1.0, 3.10, amd64, rockylinux8
- GitHub Check: wheel-build-cuopt-mps-parser / 13.1.0, 3.13, amd64, rockylinux8
🔇 Additional comments (27)
cpp/src/dual_simplex/branch_and_bound.cpp (10)
197-207: LGTM - Worker type symbol mapping is clear and consistent.The switch statement correctly maps all
bnb_worker_type_tvalues to two-letter symbols for logging. All diving types map to "D " while exploration maps to "B ".
561-580: Algorithm correctness: Martin's criteria implementation looks correct.The rounding direction is determined by comparing
down_distandup_distbased on the root solution value. The epsilon tolerance prevents numerical instability when distances are nearly equal.
582-626: Race condition addressed:incumbent_.xis correctly protected by mutex inGUIDED_DIVING.The mutex lock around reading
incumbent_.x(lines 617-619) prevents the race condition mentioned in the PR commit "fix race condition on guided diving". This ensures the incumbent solution isn't modified while being read for guided diving.
647-654: Verify fallback behavior when guided diving lacks incumbent.The fallback from
GUIDED_DIVINGtoPSEUDOCOST_DIVINGorCOEFFICIENT_DIVINGwhenupper_bound == infis correct. This ensures guided diving doesn't fail when no incumbent exists yet.
667-672: Iteration limit enforcement for diving threads.The iteration limit calculation
max_iter = settings_.diving_settings.iteration_limit_factor * bnb_lp_itersand early return whenlp_settings.iteration_limit <= 0prevents diving threads from consuming excessive iterations. This addresses the PR objective of encouraging exploration of different branches.
1133-1200: Dive limit enforcement with backtracking is well-implemented.The
dive_fromfunction correctly:
- Enforces
node_limitviadive_stats.nodes_explored > node_limit- Handles iteration limits with early break on
ITERATION_LIMIT- Implements backtracking by pruning the stack when depth exceeds
backtrackthreshold (lines 1197-1199)This aligns with the PR objective of adding "node, iteration, and backtrack limits to encourage exploration."
1337-1358: Diving strategies initialization is clear and configurable.The strategy vector is populated based on settings flags, allowing users to enable/disable specific diving heuristics. The warning on line 1357 when all strategies are disabled is helpful for debugging.
1517-1519: Updated log header accommodates two-letter symbols.The header format change addresses PR objective "#714" to allow two-letter symbols in the B&B log table. The spacing is consistent with the report format.
1544-1549: The round-robin distribution of diving workers across strategies is appropriate.The assignment
diving_strategies[k % num_strategies]distributesnum_diving_workers(calculated asmax(num_threads - num_bfs_workers, 1)) across available strategies. Eachdiving_threadcall operates on an independent copy of the LP problem (leaf_problem = original_lp_), so multiple workers executing the same strategy do not contend for shared mutable state. The work is properly synchronized within the#pragma omp taskgroupcontext, making this pattern sound for the solver's concurrent exploration.
250-261: No issue found: callers correctly handle-infreturn value fromget_lower_bound()The
-infsentinel is properly handled by all callers. When passed to gap calculation functions, infinite bounds result in a gap ofinfinity, which is correctly formatted as " - " for display. The code behaves as intended when no finite lower bound exists.cpp/src/dual_simplex/simplex_solver_settings.hpp (3)
23-36: Well-structured diving settings with sensible defaults.The new
diving_heuristics_settings_tstruct cleanly encapsulates all diving-related configuration. Default values appear reasonable:
iteration_limit_factor = 0.05(5% of B&B iterations)node_limit = 500per divebacktrack = 5levelsmin_node_depth = 10before diving starts
156-158: LGTM - Settings member placement and naming are clear.The
num_bfs_workersrename fromnum_bfs_threadsbetter reflects the worker-model semantics, anddiving_settingsprovides a clean namespace for diving configuration.
88-96: The code functions correctly. Withnum_threads=1, bothnum_bfs_workersandnum_diving_workersare set to 1, which is intentional design usingstd::max(..., 1)to ensure at least one worker of each type. OpenMP's task model gracefully handles task oversubscription—with fewer threads than tasks, the work-stealing scheduler executes tasks dynamically. Each task (best_first_threadanddiving_thread) is independent and maintains its own problem state, so having both worker types present with a single thread causes no functional issues. This is standard OpenMP usage.cpp/src/dual_simplex/node_queue.hpp (4)
22-65: Well-designed generic heap implementation.The
heap_ttemplate provides a clean abstraction over STL heap functions with proper move semantics support via the rvaluepushoverload andemplace. Thepop()returningstd::optional<T>is a safe pattern.
120-131: Thread-safe pop with atomic counter increment - correctly prevents premature exit.The comment explains the rationale well: incrementing
active_subtreeatomically with the pop prevents worker threads from incorrectly concluding the solver has finished. Thestd::exchange(entry.value()->node, nullptr)prevents the same node from being processed twice (once by best-first, once by diving).
138-156:pop_divingcorrectly handles node lifecycle to prevent use-after-free.The method:
- Holds the lock while traversing the heap
- Skips entries where
node_ptr == nullptr(already consumed by best-first)- Extracts bounds via
get_variable_boundswhile node is still valid- Returns a
detach_copy()instead of the original node pointerThis design prevents the race condition where a best-first thread could fathom/delete the original node while a diving thread still holds a reference.
70-80: This concern is unfounded—objective_estimateis always safely initialized.The
mip_node_tconstructor initializesobjective_estimatetostd::numeric_limits<f_t>::infinity()by default. Additionally, all code paths that push nodes to the heap (at lines 939–940 inexploration_ramp_upand line 1043 inplunge_from) occur only aftersolve_nodeis called withbnb_worker_type_t::EXPLORATION, which computesobjective_estimateat line 603–604. The code comment confirms this invariant: "exploration thread is the only one that can insert new nodes into the heap, and thus, we only need to calculate the objective estimate here." Heap ordering remains correct becauseobjective_estimateis never uninitialized at the point of use.Likely an incorrect or invalid review comment.
cpp/src/dual_simplex/branch_and_bound.hpp (4)
56-67: Well-documented worker type enum.The
bnb_worker_type_tenum clearly documents each worker type with references to Achterberg (2007) sections. This is helpful for understanding the algorithm implementation.
75-86: Thread-safe statistics structure using OpenMP atomics.The
bnb_stats_tstruct usesomp_atomic_tfor counters that may be updated by multiple threads. The comment on line 83 clarifies thatlast_logandnodes_since_last_logare main-thread only, though they're still atomic (which is safe but potentially unnecessary overhead).
182-183: Unified node queue replaces separate heap structures.The change from separate
mip_node_heap_tand diving queue to a singlenode_queue_taligns with the PR objective of reducing memory usage by sharing information between diving and best-first heaps.
263-267: Variable selection signature updated for worker-type awareness.The new signature accepts
bnb_worker_type_t typeand returnsbranch_variable_t<i_t>, enabling different variable selection strategies per worker type.cpp/src/dual_simplex/diving_heuristics.cpp (6)
13-65: Line search diving implementation is correct.The algorithm finds the variable with minimum
f/dratio where:
f= fractional part in the preferred directiond= distance from root solutionThe fallback on lines 50-53 handles the edge case where all variables equal their root values. The epsilon tolerance on line 19 prevents numerical instability.
67-140: Pseudocost diving correctly uses mutex for thread-safe access.The
std::lock_guard<omp_mutex_t> lock(pc.mutex)on line 74 ensures exclusive access to pseudocost data. The fallback to averages when a variable's pseudocost is uninitialized (lines 92-97) is appropriate.
142-198: Guided diving implementation is correct per Achterberg §9.2.3.The algorithm:
- Determines direction based on distance to incumbent
- Uses weighted pseudocost scores with 5:1 weighting for preferred direction
- Falls back gracefully when incumbent is unavailable (handled in caller)
Thread safety is ensured via mutex lock on line 149.
200-228: Variable lock calculation correctly analyzes constraint structure.The
calculate_variable_locksfunction counts how many constraints would be "locked" by rounding a variable up or down. The logic correctly handles:
- Equality constraints (both directions locked)
- Inequality constraints (direction depends on coefficient sign and bound type)
230-274: Coefficient diving implementation is correct.The algorithm selects the variable with minimum locks and breaks ties using:
- Fewer locks in one direction → round that way
- Equal locks → round toward nearer integer
This implements Achterberg §9.2.1 (coefficient diving).
99-100: Score formulas are numerically stable.The score formulas
std::sqrt(f_up) * (1 + pc_up) / (1 + pc_down)are safe from overflow/underflow. Fractional parts are bounded [0,1], pseudocost values are checked for finiteness and default to 1.0, and the (1 + pc) guard ensures the denominator is always ≥ 2, preventing division by zero or near-zero values. No numerical instability risk detected.
| } | ||
|
|
||
| inline const char* feasible_solution_symbol(thread_type_t type) | ||
| inline const char* feasible_solution_symbol(bnb_worker_type_t type) |
There was a problem hiding this comment.
Nit: return a single char and add the space outside this routine
| } | ||
|
|
||
| template <typename i_t, typename f_t> | ||
| void branch_and_bound_t<i_t, f_t>::report(std::string symbol, |
There was a problem hiding this comment.
Nit: make symbol a char
| std::vector<f_t> current_incumbent; | ||
|
|
||
| switch (type) { | ||
| case bnb_worker_type_t::EXPLORATION: |
There was a problem hiding this comment.
Nit: I guess EXPLORATION is supposed to be a thread improving the lower bound. I would suggest renaming this to BEST_FIRST or BEST_FIRST_SEARCH
| } | ||
|
|
||
| template <typename i_t, typename f_t> | ||
| branch_variable_t<i_t> branch_and_bound_t<i_t, f_t>::variable_selection( |
There was a problem hiding this comment.
It might be better for this to just do variable selection for the diving threads and handle the best-first threads separately.
| const f_t abs_fathom_tol = settings_.absolute_mip_gap_tol / 10; | ||
| const f_t upper_bound = get_upper_bound(); | ||
|
|
||
| // If there is no incumbent, use pseudocost diving instead of guided diving |
There was a problem hiding this comment.
Could this be moved into the variable_selection routine?
| // Choose fractional variable to branch on | ||
| const i_t branch_var = | ||
| pc_.variable_selection(leaf_fractional, leaf_solution.x, lp_settings.log); | ||
| auto [branch_var, round_dir] = |
There was a problem hiding this comment.
I would split variable selection up based on the thread type here.
For best-first threads retain the
i_t branch_var = pc_.variable_selection()
For diving threads call your variable_selection routine. Which I would rename to diving_variable_selection.
| return node_solve_info_t::TIME_LIMIT; | ||
|
|
||
| } else if (lp_status == dual::status_t::ITERATION_LIMIT) { | ||
| return node_solve_info_t::ITERATION_LIMIT; |
There was a problem hiding this comment.
As discussed does ITERATION_LIMIT need to be a status codes in node_solve_info_t. This is also the name of a solve status. At the very least it would be good to rename to DIVE_ITERATION_LIMIT. Better still if you could just signal that you want to stop here.
| std::vector<bnb_worker_type_t> diving_strategies; | ||
| diving_strategies.reserve(4); | ||
|
|
||
| if (!settings_.diving_settings.disable_pseudocost_diving) { |
There was a problem hiding this comment.
It's strange to have a setting disable_pseudo_cost_diving. This inverts the logic. I think it would be better if the setting was just pseudo_cost_diving. You can use -1 for automatic, 0 to disable, and 1 to enable.
| diving_strategies.push_back(bnb_worker_type_t::PSEUDOCOST_DIVING); | ||
| } | ||
|
|
||
| if (!settings_.diving_settings.disable_line_search_diving) { |
There was a problem hiding this comment.
Same for all these settings.
| settings_.num_threads - settings_.num_bfs_workers); | ||
|
|
||
| exploration_stats_.nodes_explored = 0; | ||
| exploration_stats_.nodes_explored = 1; |
There was a problem hiding this comment.
This is counting the root node as being explored?
| EXPLORATION = 0, // Best-First + Plunging. Pseudocost branching + Martin's criteria. | ||
| DIVING = 1, | ||
| enum class bnb_worker_type_t { | ||
| EXPLORATION = 0, // Best-First + Plunging. |
There was a problem hiding this comment.
Change EXPLORATION to BEST_FIRST or BEST_FIRST_PLUNGING
| f_t min_score = std::numeric_limits<f_t>::max(); | ||
| rounding_direction_t round_dir = rounding_direction_t::NONE; | ||
|
|
||
| for (auto j : fractional) { |
There was a problem hiding this comment.
Nit: use i_t instead of auto to make it clear the type for the reader.
| i_t end = lp_problem.A.col_start[var_idx + 1]; | ||
|
|
||
| for (i_t k = start; k < end; ++k) { | ||
| f_t nz_val = lp_problem.A.x[k]; |
There was a problem hiding this comment.
Nit: most other code would call this x or val and avoid the nz prefix
There was a problem hiding this comment.
f_t val = lp_problem.A.x[p];
| i_t start = lp_problem.A.col_start[var_idx]; | ||
| i_t end = lp_problem.A.col_start[var_idx + 1]; | ||
|
|
||
| for (i_t k = start; k < end; ++k) { |
There was a problem hiding this comment.
Nit: the rest of the code use p or q for the variable that loops over nonzeros.
|
|
||
| for (i_t k = start; k < end; ++k) { | ||
| f_t nz_val = lp_problem.A.x[k]; | ||
| i_t nz_row = lp_problem.A.i[k]; |
There was a problem hiding this comment.
Nit: the rest uses the convention
i_t i = lp_problem.A.i[p];
We use the convention of i for rows and j for columns
| f_t nz_val = lp_problem.A.x[k]; | ||
| i_t nz_row = lp_problem.A.i[k]; | ||
|
|
||
| if (std::isfinite(lp_problem.upper[nz_row]) && std::isfinite(lp_problem.lower[nz_row])) { |
There was a problem hiding this comment.
Is this correct? nz_row corresponds to a row (constraint) index. upper and lower are indexed by columns (variables).
| continue; | ||
| } | ||
|
|
||
| f_t sign = std::isfinite(lp_problem.upper[nz_row]) ? 1 : -1; |
There was a problem hiding this comment.
Same here. This indexing is incorrect.
| rounding_direction_t round_dir = rounding_direction_t::NONE; | ||
| constexpr f_t eps = 1e-6; | ||
|
|
||
| for (auto j : fractional) { |
There was a problem hiding this comment.
Nit: use i_t here to make the type clear to the reader
| struct diving_heuristics_settings_t { | ||
| i_t num_diving_workers = -1; | ||
|
|
||
| bool disable_line_search_diving = false; |
There was a problem hiding this comment.
See suggestions above about the naming of these settings.
i_t line_search_diving = 1;
| bool disable_coefficient_diving = false; | ||
|
|
||
| i_t min_node_depth = 10; | ||
| i_t node_limit = 500; |
There was a problem hiding this comment.
Nit: node_limit -> diving_node_limit to avoid confusion with a node limit coming from the user.
chris-maes
left a comment
chris-maes
left a comment
There was a problem hiding this comment.
Most of the comments are minor.
The most important comment to address is the likely bug with row/column indicies in calculate_variable_locks. The only reasons this isn't causing a segfault right now is because we extend with artifical variables so there are always more columns than rows. But you are likely incorrectly computing the variable locks.
The other comment is around the variable selection for diving and best first threads/workers. I think its best to split this up rather than trying to unify into a single routine.
2 participants
The goal of this PR is to improve the diving framework in the MIP solver based on [1, Section 9.2]. It introduces the following changes:
node_queue) to allow the sharing of information between them. This also greatly reduces memory consumption (33GBvs48GBforneos-848589after250s) since the lower and upper variable no longer needs to be stored for diving (Unified Node Queue + Diving Node and Iteration Limits #718).MIPLIB results (GH200):
main (#718): 226 feasible solutions with 12.8% average primal gap
Reference:
[1] T. Achterberg, “Constraint Integer Programming,” PhD, Technischen Universität Berlin,
Berlin, 2007. doi: 10.14279/depositonce-1634.
Checklist
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.