-
Notifications
You must be signed in to change notification settings - Fork 158
Main merge release/26.02 #801
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -84,14 +84,16 @@ class branch_and_bound_t { | |
| f_t user_objective, | ||
| i_t iterations) | ||
| { | ||
| root_crossover_soln_.x = primal; | ||
| root_crossover_soln_.y = dual; | ||
| root_crossover_soln_.z = reduced_costs; | ||
| root_objective_ = objective; | ||
| root_crossover_soln_.objective = objective; | ||
| root_crossover_soln_.user_objective = user_objective; | ||
| root_crossover_soln_.iterations = iterations; | ||
| root_crossover_solution_set_.store(true, std::memory_order_release); | ||
| if (!is_root_solution_set) { | ||
| root_crossover_soln_.x = primal; | ||
| root_crossover_soln_.y = dual; | ||
| root_crossover_soln_.z = reduced_costs; | ||
| root_objective_ = objective; | ||
| root_crossover_soln_.objective = objective; | ||
| root_crossover_soln_.user_objective = user_objective; | ||
| root_crossover_soln_.iterations = iterations; | ||
| root_crossover_solution_set_.store(true, std::memory_order_release); | ||
| } | ||
|
Comment on lines
+87
to
+96
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: find . -path "*/dual_simplex/branch_and_bound.hpp" -type fRepository: NVIDIA/cuopt Length of output: 99 🏁 Script executed: cat -n cpp/src/dual_simplex/branch_and_bound.hpp | sed -n '80,105p'Repository: NVIDIA/cuopt Length of output: 1469 🏁 Script executed: cat -n cpp/src/dual_simplex/branch_and_bound.hpp | sed -n '160,175p'Repository: NVIDIA/cuopt Length of output: 659 🏁 Script executed: rg -n "is_root_solution_set" cpp/src/dual_simplex/Repository: NVIDIA/cuopt Length of output: 293 🏁 Script executed: rg -n "set_root_relaxation_solution" cpp/src/dual_simplex/ -A 20Repository: NVIDIA/cuopt Length of output: 2015 🏁 Script executed: cat -n cpp/src/dual_simplex/branch_and_bound.cpp | sed -n '1375,1395p'Repository: NVIDIA/cuopt Length of output: 892 🏁 Script executed: rg -n "is_root_solution_set\s*=" cpp/src/dual_simplex/ -B 5 -A 2Repository: NVIDIA/cuopt Length of output: 654 🏁 Script executed: rg -n "set_root_relaxation_solution" cpp/src/dual_simplex/branch_and_bound.cpp -B 5 -A 15Repository: NVIDIA/cuopt Length of output: 38 🏁 Script executed: rg -n "set_root_relaxation_solution\(" cpp/src/dual_simplex/ -B 3 -A 3Repository: NVIDIA/cuopt Length of output: 758 🏁 Script executed: cat -n cpp/src/dual_simplex/branch_and_bound.cpp | sed -n '1320,1390p'Repository: NVIDIA/cuopt Length of output: 3543 🏁 Script executed: rg -n "enable_concurrent_lp_root_solve" cpp/src/dual_simplex/ -B 2 -A 2Repository: NVIDIA/cuopt Length of output: 1974 🏁 Script executed: rg -n "PDLP\|pdlp" cpp/src/dual_simplex/branch_and_bound.cpp -B 2 -A 2 | head -40Repository: NVIDIA/cuopt Length of output: 38 🏁 Script executed: cat -n cpp/src/dual_simplex/branch_and_bound.cpp | sed -n '1250,1290p'Repository: NVIDIA/cuopt Length of output: 1804 🏁 Script executed: rg -n "set_root_relaxation_solution" cpp/src/ -B 2 -A 2Repository: NVIDIA/cuopt Length of output: 3247 🏁 Script executed: cat -n cpp/src/dual_simplex/branch_and_bound.cpp | sed -n '1430,1470p'Repository: NVIDIA/cuopt Length of output: 2151 Fix data race on
🐛 Proposed fix- bool is_root_solution_set{false};
+ std::atomic<bool> is_root_solution_set{false};
void set_root_relaxation_solution(...){
- if (!is_root_solution_set) {
+ if (!is_root_solution_set.exchange(true, std::memory_order_acq_rel)) {
root_crossover_soln_.x = primal;
root_crossover_soln_.y = dual;
root_crossover_soln_.z = reduced_costs;
root_objective_ = objective;
root_crossover_soln_.objective = objective;
root_crossover_soln_.user_objective = user_objective;
root_crossover_soln_.iterations = iterations;
root_crossover_solution_set_.store(true, std::memory_order_release);
}
}As per coding guidelines, prevent thread-unsafe use of shared solver state. Also applies to: 167 🤖 Prompt for AI Agents |
||
| } | ||
|
|
||
| // Set a solution based on the user problem during the course of the solve | ||
|
|
@@ -162,6 +164,7 @@ class branch_and_bound_t { | |
| std::atomic<bool> root_crossover_solution_set_{false}; | ||
| bool enable_concurrent_lp_root_solve_{false}; | ||
| std::atomic<int> root_concurrent_halt_{0}; | ||
| bool is_root_solution_set{false}; | ||
|
|
||
| // Pseudocosts | ||
| pseudo_costs_t<i_t, f_t> pc_; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,6 @@ | ||
| /* clang-format off */ | ||
| /* | ||
| * SPDX-FileCopyrightText: Copyright (c) 2025, NVIDIA CORPORATION & AFFILIATES. All rights reserved. | ||
| * SPDX-FileCopyrightText: Copyright (c) 2025-2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved. | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
| /* clang-format on */ | ||
|
|
@@ -30,6 +30,22 @@ enum class lp_status_t { | |
| UNSET = 8 | ||
| }; | ||
|
|
||
| static std::string lp_status_to_string(lp_status_t status) | ||
| { | ||
| switch (status) { | ||
| case lp_status_t::OPTIMAL: return "OPTIMAL"; | ||
| case lp_status_t::INFEASIBLE: return "INFEASIBLE"; | ||
| case lp_status_t::UNBOUNDED: return "UNBOUNDED"; | ||
| case lp_status_t::ITERATION_LIMIT: return "ITERATION_LIMIT"; | ||
| case lp_status_t::TIME_LIMIT: return "TIME_LIMIT"; | ||
| case lp_status_t::NUMERICAL_ISSUES: return "NUMERICAL_ISSUES"; | ||
| case lp_status_t::CUTOFF: return "CUTOFF"; | ||
| case lp_status_t::CONCURRENT_LIMIT: return "CONCURRENT_LIMIT"; | ||
| case lp_status_t::UNSET: return "UNSET"; | ||
| } | ||
| return "UNKNOWN"; | ||
| } | ||
|
Comment on lines
+33
to
+47
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: find . -name "solve.hpp" -type f | head -20Repository: NVIDIA/cuopt Length of output: 175 🏁 Script executed: cat -n cpp/src/dual_simplex/solve.hpp | head -60Repository: NVIDIA/cuopt Length of output: 2449 🏁 Script executed: # Check if <string> is already included directly
grep -n "^#include <string>" cpp/src/dual_simplex/solve.hppRepository: NVIDIA/cuopt Length of output: 38 🏁 Script executed: # Check what headers are transitively including string
grep -r "include <string>" cpp/src/dual_simplex/*.hpp | head -20Repository: NVIDIA/cuopt Length of output: 218 🏁 Script executed: # Check what types.hpp includes to see if it brings in <string>
cat cpp/src/dual_simplex/types.hpp | head -50Repository: NVIDIA/cuopt Length of output: 573 🏁 Script executed: # Also check if any included header brings in <string>
for header in basis_updates initial_basis presolve simplex_solver_settings types; do
echo "=== $header.hpp ==="
grep -n "#include <string>" cpp/src/dual_simplex/${header}.hpp
doneRepository: NVIDIA/cuopt Length of output: 182 🏁 Script executed: # Check if std::vector is used and where it comes from
grep -n "std::vector" cpp/src/dual_simplex/solve.hppRepository: NVIDIA/cuopt Length of output: 665 🏁 Script executed: # Check if any of the included headers transitively includes <string> or <vector>
for header in basis_updates initial_basis presolve simplex_solver_settings; do
echo "=== Checking $header.hpp ==="
head -30 cpp/src/dual_simplex/${header}.hpp | grep -E "^#include"
doneRepository: NVIDIA/cuopt Length of output: 1002 🏁 Script executed: # Check if there's a common precompiled header or config header
grep -r "^#pragma once" cpp/src/dual_simplex/*.hpp | wc -lRepository: NVIDIA/cuopt Length of output: 58 🏁 Script executed: # Let's check one of the included headers more thoroughly to see all its includes
head -40 cpp/src/dual_simplex/basis_updates.hppRepository: NVIDIA/cuopt Length of output: 1262 Consider adding
🤖 Prompt for AI Agents |
||
|
|
||
| template <typename i_t, typename f_t> | ||
| f_t compute_objective(const lp_problem_t<i_t, f_t>& problem, const std::vector<f_t>& x); | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
Repository: NVIDIA/cuopt
Length of output: 312
🏁 Script executed:
Repository: NVIDIA/cuopt
Length of output: 1255
🏁 Script executed:
Repository: NVIDIA/cuopt
Length of output: 3037
Replace plain
bool is_root_solution_setwithstd::atomic<bool>to prevent race condition.The flag guards against concurrent PDLP/Barrier solvers overwriting the root solution when B&B is already running, as confirmed by the PR context. Currently,
is_root_solution_setis declared as a plainboolat line 167 of the header without synchronization, while being read byset_root_relaxation_solution()(external solvers) and written by the B&B solve function at line 1385. This creates a data race between concurrent threads.Change the declaration to
std::atomic<bool> is_root_solution_set{false};and use appropriate memory ordering (e.g.,std::memory_order_acquirefor read,std::memory_order_releasefor write), consistent with the nearbyroot_crossover_solution_set_pattern already used in the class.🤖 Prompt for AI Agents