Fix root LP solution copy in diversity_manager_t #685
Conversation
aliceb-nv
added
bug
📝 WalkthroughWalkthroughDirect-copy LP primal and dual solutions from Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 |
nguidotti
left a comment
nguidotti
left a comment
There was a problem hiding this comment.
Looks good to me! Nice work, Alice!
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
python/cuopt/cuopt/tests/linear_programming/test_python_API.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{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:
python/cuopt/cuopt/tests/linear_programming/test_python_API.py
**/*test*.{cpp,cu,py}
📄 CodeRabbit inference engine (.github/.coderabbit_review_guide.md)
**/*test*.{cpp,cu,py}: Write tests validating numerical correctness of optimization results (not just 'runs without error'); test degenerate cases (infeasible, unbounded, empty, singleton problems)
Ensure test isolation: prevent GPU state, cached memory, and global variables from leaking between test cases; verify each test independently initializes its environment
Add tests for algorithm phase transitions: verify correct initialization of bounds and state when transitioning from presolve to simplex to diving to crossover
Add tests for problem transformations: verify correctness of original→transformed→postsolve mappings and index consistency across problem representations
Test with free variables, singleton problems, and extreme problem dimensions near resource limits to validate edge case handling
Files:
python/cuopt/cuopt/tests/linear_programming/test_python_API.py
🧠 Learnings (6)
📓 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 docs/**/*.{rst,md} : Document numerical tolerances, parameter ranges, and algorithm phase descriptions in API documentation when solver behavior or tolerances change
Applied to files:
python/cuopt/cuopt/tests/linear_programming/test_python_API.py
📚 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} : Write tests validating numerical correctness of optimization results (not just 'runs without error'); test degenerate cases (infeasible, unbounded, empty, singleton problems)
Applied to files:
python/cuopt/cuopt/tests/linear_programming/test_python_API.py
📚 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:
python/cuopt/cuopt/tests/linear_programming/test_python_API.py
📚 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:
python/cuopt/cuopt/tests/linear_programming/test_python_API.py
📚 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:
python/cuopt/cuopt/tests/linear_programming/test_python_API.py
🧬 Code graph analysis (1)
python/cuopt/cuopt/tests/linear_programming/test_python_API.py (1)
python/cuopt/cuopt/linear_programming/problem.py (3)
getValue(128-133)getValue(376-386)getValue(745-753)
| assert x1.getValue() == pytest.approx(0.2295081, abs=1e-3) | ||
| assert x2.getValue() == pytest.approx(0.0000000, abs=0.000001) | ||
| assert x3.getValue() == pytest.approx(0.1092896) | ||
| assert problem.ObjValue == pytest.approx(-0.284153) | ||
| assert x3.getValue() == pytest.approx(0.1092896, abs=1e-3) | ||
| assert problem.ObjValue == pytest.approx(-0.284153, abs=1e-3) |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
# First, check if the file exists and examine the specific lines
head -815 python/cuopt/cuopt/tests/linear_programming/test_python_API.py | tail -35Repository: NVIDIA/cuopt
Length of output: 168
🏁 Script executed:
# Check the function context around those lines
sed -n '783,815p' python/cuopt/cuopt/tests/linear_programming/test_python_API.pyRepository: NVIDIA/cuopt
Length of output: 168
🏁 Script executed:
# Look for related test functions and tolerance patterns
grep -n "test_quadratic_objective" python/cuopt/cuopt/tests/linear_programming/test_python_API.pyRepository: NVIDIA/cuopt
Length of output: 168
🏁 Script executed:
# Search for diversity_manager references in the codebase
rg "diversity_manager" --type py --type cu --type cpp -A 3Repository: NVIDIA/cuopt
Length of output: 168
Document the rationale for tolerance changes when solver behavior shifts.
The tolerance adjustments (abs=1e-3 for x1, x3, ObjValue; abs=0.000001 for x2) align with expected numerical differences in optimization results, where inactive variables (x2≈0) warrant tighter tolerances than active variables. However, per coding guidelines, when solver behavior changes (as from the diversity_manager.cu fix), document the numerical tolerance rationale and any expected precision variations in the test or related documentation to explain why these tolerances are appropriate for the modified algorithm.
There was a problem hiding this comment.
Rationale is temp. fix for bug outlined here
#686
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Fetch details of issue #686 to understand the underlying bug
gh issue view 686 --repo NVIDIA/cuopt --json title,body,url,state,labels
Length of output: 612
@aliceb-nv, thank you for clarifying! I see that issue #686 tracks the underlying nondeterminism in the QP solver that's causing slight objective value variations. Using tolerance adjustments as a temporary workaround is a pragmatic approach to unblock this PR while the root cause is investigated.
For future reference, you might consider adding a brief TODO comment in the test pointing to issue #686, so it's clear these tolerances should be revisited once the nondeterminism is resolved:
# TODO(#686): Tolerances adjusted to handle QP nondeterminism - revisit when fixed
assert x1.getValue() == pytest.approx(0.2295081, abs=1e-3)This would help ensure the temporary nature is visible in the code itself.
🐰💭
🧬 Code Graph Analysis Results
[python/cuopt/cuopt/linear_programming/problem.py 309-319]
def get_pdlp_warm_start_data(self):
"""
Note: Applicable to only LP.
Allows to retrieve the warm start data from the PDLP solver
once the problem is solved.
This data can be used to warmstart the next PDLP solve by setting it
in :py:meth:cuopt.linear_programming.solver_settings.SolverSettings.set_pdlp_warm_start_data # noqa
Examples
--------
>>> problem = problem.Problem.readMPS("LP.mps")
>>> problem.solve()
>>> warmstart_data = problem.get_pdlp_warm_start_data()
>>> settings.set_pdlp_warm_start_data(warmstart_data)
>>> updated_problem = problem.Problem.readMPS("updated_LP.mps")
>>> updated_problem.solve(settings)
"""
return self.warmstart_data
[python/cuopt/cuopt/linear_programming/problem.py 1590-1612]
def readMPS(cls, mps_file):
"""
Initiliaze a problem from an MPS <https://en.wikipedia.org/wiki/MPS_(format)>__ file. # noqa
Examples
--------
>>> problem = problem.Problem.readMPS("model.mps")
"""
problem = cls()
data_model = cuopt_mps_parser.ParseMps(mps_file)
problem._from_data_model(data_model)
problem.model = data_model
return problem
def writeMPS(self, mps_file):
"""
Write the problem into an MPS <https://en.wikipedia.org/wiki/MPS_(format)>__ file. # noqa
Examples
--------
>>> problem.writeMPS("model.mps")
"""
if self.model is None:
self._to_data_model()
self.model.writeMPS(mps_file)
[python/cuopt/cuopt/linear_programming/solver_settings/solver_settings.py 309-319]
def get_pdlp_warm_start_data(self):
"""
Note: Applicable to only LP.
Allows to retrieve the warm start data from the PDLP solver
once the problem is solved.
This data can be used to warmstart the next PDLP solve by setting it
in :py:meth:cuopt.linear_programming.solver_settings.SolverSettings.set_pdlp_warm_start_data # noqa
Examples
--------
>>> problem = problem.Problem.readMPS("LP.mps")
>>> problem.solve()
>>> warmstart_data = problem.get_pdlp_warm_start_data()
>>> settings.set_pdlp_warm_start_data(warmstart_data)
>>> updated_problem = problem.Problem.readMPS("updated_LP.mps")
>>> updated_problem.solve(settings)
"""
return self.warmstart_data
[python/cuopt/cuopt/linear_programming/data_model/data_model.py 683-684]
def writeMPS(self, user_problem_file):
return super().writeMPS(user_problem_file)
[python/cuopt/cuopt/tests/linear_programming/test_incumbent_callbacks.py 35-51]
class CustomGetSolutionCallback(GetSolutionCallback):
def init(self):
super().init()
self.n_callbacks = 0
self.solutions = []
def get_solution(self, solution, solution_cost):
self.n_callbacks += 1
assert len(solution) > 0
assert len(solution_cost) == 1
self.solutions.append(
{
"solution": solution.copy_to_host(),
"cost": solution_cost.copy_to_host()[0],
}
)
[python/cuopt/cuopt/tests/linear_programming/test_incumbent_callbacks.py 53-65]
class CustomSetSolutionCallback(SetSolutionCallback):
def init(self, get_callback):
super().init()
self.n_callbacks = 0
self.get_callback = get_callback
def set_solution(self, solution, solution_cost):
self.n_callbacks += 1
if self.get_callback.solutions:
solution[:] = self.get_callback.solutions[-1]["solution"]
solution_cost[0] = float(
self.get_callback.solutions[-1]["cost"]
)
[python/cuopt/cuopt/tests/linear_programming/test_lp_solver.py 497-525]
def test_warm_start():
file_path = RAPIDS_DATASET_ROOT_DIR + "/linear_programming/a2864/a2864.mps"
data_model_obj = cuopt_mps_parser.ParseMps(file_path)
settings = solver_settings.SolverSettings()
settings.set_parameter(CUOPT_PDLP_SOLVER_MODE, PDLPSolverMode.Stable2)
settings.set_optimality_tolerance(1e-3)
settings.set_parameter(CUOPT_INFEASIBILITY_DETECTION, False)
# Solving from scratch until 1e-3
solution = solver.Solve(data_model_obj, settings)
iterations_first_solve = solution.get_lp_stats()["nb_iterations"]
# Solving until 1e-2 to use the result as a warm start
settings.set_optimality_tolerance(1e-2)
solution2 = solver.Solve(data_model_obj, settings)
iterations_second_solve = solution2.get_lp_stats()["nb_iterations"]
# Solving until 1e-3 using the previous state as a warm start
settings.set_optimality_tolerance(1e-3)
settings.set_pdlp_warm_start_data(solution2.get_pdlp_warm_start_data())
solution3 = solver.Solve(data_model_obj, settings)
iterations_third_solve = solution3.get_lp_stats()["nb_iterations"]
assert (
iterations_third_solve + iterations_second_solve
== iterations_first_solve
)
🧠 Learnings used
Learnt from: chris-maes
Repo: NVIDIA/cuopt PR: 500
File: cpp/tests/linear_programming/c_api_tests/c_api_test.c:1033-1048
Timestamp: 2025-12-06T00:22:48.638Z
Learning: In cuOPT's quadratic programming API, when a user provides a quadratic objective matrix Q via set_quadratic_objective_matrix or the C API functions cuOptCreateQuadraticProblem/cuOptCreateQuadraticRangedProblem, the API internally computes Q_symmetric = Q + Q^T and the barrier solver uses 0.5 * x^T * Q_symmetric * x. From the user's perspective, the convention is x^T Q x. For a diagonal Q with values [q1, q2, ...], the resulting quadratic terms are q1*x1^2 + q2*x2^2 + ...
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} : Write tests validating numerical correctness of optimization results (not just 'runs without error'); test degenerate cases (infeasible, unbounded, empty, singleton problems)
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 docs/**/*.{rst,md} : Document numerical tolerances, parameter ranges, and algorithm phase descriptions in API documentation when solver behavior or tolerances change
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)
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
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} : Test with free variables, singleton problems, and extreme problem dimensions near resource limits to validate edge case handling
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
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 : 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
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
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
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
cpp/src/linear_programming/translate.hpp (1)
82-89: Fallback column naming logic is sound; consider mirroring safety for row namesThe new block on Line 85–Line 89 correctly guarantees a stable, index-aligned name for every column when
model.var_names.size() < nby using"_CUOPT_x" + std::to_string(j). This is good for maintaining a consistent mapping between LP columns and names across transforms.Two small follow‑ups you might consider:
- If you expect
model.row_names.size()to always equalm, an explicitcuopt_assert(model.row_names.size() >= static_cast<size_t>(m), ...)before the loop at Line 78 would document and enforce that contract, mirroring the robustness you just added for column names.- If you ever want names even when there are no user‑provided names, you could drop the
if (model.var_names.size() > 0)guard and always fillcol_nameswith either user names or the fallback; that would make naming behavior more uniform.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
cpp/src/linear_programming/translate.hpp(1 hunks)cpp/src/mip/diversity/population.cu(1 hunks)
🧰 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/linear_programming/translate.hppcpp/src/mip/diversity/population.cu
**/*.{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/linear_programming/translate.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/linear_programming/translate.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/linear_programming/translate.hppcpp/src/mip/diversity/population.cu
**/*.{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/population.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/population.cu
🧠 Learnings (10)
📓 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} : 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/linear_programming/translate.hppcpp/src/mip/diversity/population.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/linear_programming/translate.hppcpp/src/mip/diversity/population.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/linear_programming/translate.hppcpp/src/mip/diversity/population.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 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/linear_programming/translate.hppcpp/src/mip/diversity/population.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 **/*test*.{cpp,cu,py} : Add tests for problem transformations: verify correctness of original→transformed→postsolve mappings and index consistency across problem representations
Applied to files:
cpp/src/linear_programming/translate.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/mip/diversity/population.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/mip/diversity/population.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} : 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/population.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 **/*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/mip/diversity/population.cu
⏰ 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). (5)
- GitHub Check: wheel-build-cuopt-mps-parser / 13.0.2, 3.10, arm64, rockylinux8
- GitHub Check: wheel-build-cuopt-mps-parser / 13.0.2, 3.12, arm64, rockylinux8
- GitHub Check: wheel-build-cuopt-mps-parser / 13.0.2, 3.11, amd64, 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.11, arm64, rockylinux8
🔇 Additional comments (1)
cpp/src/mip/diversity/population.cu (1)
286-290: Good separation of feasibility check from callback solution; please confirm lifetime/async assumptionsThe new
scaled_solpath on Line 286–Line 290 cleanly decouples the feasibility check on the unscaled/original problem from bothsolandtemp_sol, which prevents unintended mutation of the solution that drives callbacks. Bindingscaled_sol.problem_ptrto the stack‑allocatedn_problemand then callingresize_to_original_problem()andcompute_feasibility()is consistent with keeping feasibility checks in the correct (original) problem context, which matches the solver‑phase/context learnings.Two things worth double‑checking:
- That
solution_t::resize_to_original_problem()andsolution_t::compute_feasibility()do not retain references ton_problembeyond the lifetime ofscaled_sol(e.g., via async work that would outlive this scope).- That
resize_to_original_problem()indeed preparesscaled_solcorrectly for feasibility in the unscaled original problem, given that onlytemp_sol.assignmentis explicitly unscaled.If both hold, this change looks solid and aligns well with the intent to validate feasibility post‑unscaling without corrupting the original LP solution.
Based on learnings, this also helps keep variable/constraint indices consistent across problem contexts.
hlinsen
left a comment
hlinsen
left a comment
There was a problem hiding this comment.
Thanks Alice for fixing my mistake
|
/merge |
rapids-bot
bot
merged commit 675f902
into
NVIDIA:release/25.12
This PR fixes a missing copy in diversity_manager_t after the root node solve, which caused uninitialized data to remain in the lp_optimal_solution vector and causing errors later down the line. ## Summary by CodeRabbit * **Refactor** * Streamlined solution handling to copy LP primal/dual results directly and avoid redundant temporary allocations; preserved synchronization behavior. * Adjusted feasibility validation to operate on a separate scaled copy to prevent unintended mutation. * **Bug Fixes** * Added size and finiteness checks to validate solutions before use. * **User-facing** * Deterministic fallback names for unnamed variables so every column has a label. * **Tests** * Relaxed numerical tolerances in some objective and variable checks. <sub>✏️ Tip: You can customize this high-level summary in your review settings.</sub> Authors: - Alice Boucher (https://github.com/aliceb-nv) Approvers: - Nicolas L. Guidotti (https://github.com/nguidotti) - Ramakrishnap (https://github.com/rgsl888prabhu) - Hugo Linsenmaier (https://github.com/hlinsen) URL: NVIDIA#685
4 participants
This PR fixes a missing copy in diversity_manager_t after the root node solve, which caused uninitialized data to remain in the lp_optimal_solution vector and causing errors later down the line.
Summary by CodeRabbit
Refactor
Bug Fixes
User-facing
Tests
✏️ Tip: You can customize this high-level summary in your review settings.