C api additions #721
C api additions #721
Conversation
aliceb-nv
added
non-breaking
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
| * | ||
| * @param[in] settings - The solver settings object. | ||
| * @param[in] primal_solution - A pointer to an array of type cuopt_float_t | ||
| * of size num_variables containing the initial primal values. |
There was a problem hiding this comment.
Do we want to specify if data should be on the CPU and/or GPU?
There was a problem hiding this comment.
My understanding is that the C API abstracts away the reality of a GPU implementation - therefore all pointers involved are host pointers (as a result, we do not expose the stream(s) involved)
I think that if the need arises, we could create alternative versions of these functions to upload GPU data, or use something similar to cuSparse's pointer mode flag
There was a problem hiding this comment.
I'll note that as a new cuOpt user, I had the exact same question about host/device pointer for inputs.
I think it would be helpful to explicitly say they expect host pointers (either in the individual docstrings or in some high level place in the user guide "all C API pointers are expected to be pointers to host memory").
| * | ||
| * @param[in] settings - The solver settings object. | ||
| * @param[in] dual_solution - A pointer to an array of type cuopt_float_t | ||
| * of size num_constraints containing the initial dual values. |
| * | ||
| * @param[in] settings - The solver settings object. | ||
| * @param[in] solution - A pointer to an array of type cuopt_float_t | ||
| * of size num_variables containing the solution values. |
📝 WalkthroughWalkthroughThe pull request adds C API functions for writing optimization problems to MPS files and setting initial primal/dual solutions for LP solves, along with support for MPS start solutions. It enhances the MPS writer to handle orphan variables, improves the problem equivalence checking with GPU acceleration, and expands test coverage for round-trip serialization. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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: 1
🧹 Nitpick comments (2)
cpp/tests/linear_programming/c_api_tests/c_api_test.c (1)
1204-1283: Consider adding equivalence check to this test.The test verifies that the read-back problem can be solved, but doesn't verify that the written and read-back problems are mathematically equivalent. The more comprehensive equivalence checking is done in cpp/tests/linear_programming/c_api_tests/c_api_tests.cpp (test_mps_roundtrip), so this may be intentional as a simpler smoke test.
However, per the past review comment about checking equality between crafted and read/write problems, consider adding at least a basic check that the objective value matches expectations, or document that this is a smoke test only.
cpp/include/cuopt/linear_programming/cuopt_c.h (1)
698-710: Documentation could clarify expected array size validation.The documentation states
num_variablesis the size of the array, but it would be helpful to clarify that the caller must ensure this matches the problem's variable count. The implementation validatesnum_variables > 0but the size mismatch with the actual problem would only be caught later during solve.Consider adding to the documentation:
* @param[in] num_variables - The number of variables (size of the primal_solution array). +* Must match the number of variables in the optimization problem.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
cpp/include/cuopt/linear_programming/constants.h(1 hunks)cpp/include/cuopt/linear_programming/cuopt_c.h(2 hunks)cpp/include/cuopt/linear_programming/optimization_problem.hpp(1 hunks)cpp/libmps_parser/src/mps_writer.cpp(3 hunks)cpp/src/linear_programming/cuopt_c.cpp(3 hunks)cpp/src/linear_programming/cuopt_c_internal.hpp(1 hunks)cpp/src/linear_programming/optimization_problem.cu(2 hunks)cpp/tests/linear_programming/CMakeLists.txt(1 hunks)cpp/tests/linear_programming/c_api_tests/c_api_test.c(1 hunks)cpp/tests/linear_programming/c_api_tests/c_api_tests.cpp(2 hunks)cpp/tests/linear_programming/c_api_tests/c_api_tests.h(1 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{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/include/cuopt/linear_programming/constants.hcpp/include/cuopt/linear_programming/optimization_problem.hppcpp/src/linear_programming/cuopt_c_internal.hppcpp/tests/linear_programming/c_api_tests/c_api_tests.hcpp/include/cuopt/linear_programming/cuopt_c.hcpp/src/linear_programming/cuopt_c.cppcpp/libmps_parser/src/mps_writer.cppcpp/src/linear_programming/optimization_problem.cucpp/tests/linear_programming/c_api_tests/c_api_tests.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/include/cuopt/linear_programming/constants.hcpp/include/cuopt/linear_programming/optimization_problem.hppcpp/src/linear_programming/cuopt_c_internal.hppcpp/tests/linear_programming/c_api_tests/c_api_tests.hcpp/include/cuopt/linear_programming/cuopt_c.h
**/*.{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/include/cuopt/linear_programming/constants.hcpp/include/cuopt/linear_programming/optimization_problem.hppcpp/src/linear_programming/cuopt_c_internal.hppcpp/tests/linear_programming/c_api_tests/c_api_tests.hcpp/include/cuopt/linear_programming/cuopt_c.hcpp/src/linear_programming/cuopt_c.cppcpp/libmps_parser/src/mps_writer.cppcpp/tests/linear_programming/c_api_tests/c_api_tests.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/include/cuopt/linear_programming/constants.hcpp/include/cuopt/linear_programming/optimization_problem.hppcpp/src/linear_programming/cuopt_c_internal.hppcpp/tests/linear_programming/c_api_tests/c_api_tests.hcpp/include/cuopt/linear_programming/cuopt_c.hcpp/src/linear_programming/cuopt_c.cppcpp/libmps_parser/src/mps_writer.cppcpp/src/linear_programming/optimization_problem.cucpp/tests/linear_programming/c_api_tests/c_api_tests.cpp
cpp/include/cuopt/**/*
⚙️ CodeRabbit configuration file
cpp/include/cuopt/**/*: For public header files (C++ API):
- Check if new public functions/classes have documentation comments (Doxygen format)
- Flag API changes that may need corresponding docs/ updates
- Verify parameter descriptions match actual types/behavior
- Suggest documenting thread-safety, GPU requirements, and numerical behavior
- For breaking changes, recommend updating docs and migration guides
Files:
cpp/include/cuopt/linear_programming/constants.hcpp/include/cuopt/linear_programming/optimization_problem.hppcpp/include/cuopt/linear_programming/cuopt_c.h
**/*.{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/linear_programming/optimization_problem.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/linear_programming/optimization_problem.cu
**/*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:
cpp/tests/linear_programming/c_api_tests/c_api_tests.cpp
🧠 Learnings (27)
📓 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-10-22T14:25:22.899Z
Learnt from: aliceb-nv
Repo: NVIDIA/cuopt PR: 527
File: cpp/src/mip/diversity/lns/rins.cu:167-175
Timestamp: 2025-10-22T14:25:22.899Z
Learning: In MIP (Mixed Integer Programming) problems in the cuOPT codebase, `n_integer_vars == 0` is impossible by definition—MIP problems must have at least one integer variable. If there are no integer variables, it would be a pure Linear Programming (LP) problem, not a MIP problem.
Applied to files:
cpp/include/cuopt/linear_programming/constants.hcpp/include/cuopt/linear_programming/cuopt_c.hcpp/src/linear_programming/cuopt_c.cppcpp/libmps_parser/src/mps_writer.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} : Check for unclosed file handles when reading MPS/QPS problem files; ensure RAII patterns or proper cleanup in exception paths
Applied to files:
cpp/include/cuopt/linear_programming/constants.hcpp/src/linear_programming/cuopt_c.cppcpp/libmps_parser/src/mps_writer.cppcpp/tests/linear_programming/c_api_tests/c_api_tests.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/include/cuopt/linear_programming/constants.hcpp/include/cuopt/linear_programming/optimization_problem.hppcpp/src/linear_programming/cuopt_c_internal.hppcpp/include/cuopt/linear_programming/cuopt_c.hcpp/src/linear_programming/cuopt_c.cppcpp/libmps_parser/src/mps_writer.cppcpp/src/linear_programming/optimization_problem.cucpp/tests/linear_programming/c_api_tests/c_api_tests.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/include/cuopt/linear_programming/constants.hcpp/src/linear_programming/cuopt_c_internal.hppcpp/include/cuopt/linear_programming/cuopt_c.hcpp/src/linear_programming/cuopt_c.cppcpp/src/linear_programming/optimization_problem.cucpp/tests/linear_programming/c_api_tests/c_api_tests.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/include/cuopt/linear_programming/constants.hcpp/src/linear_programming/cuopt_c_internal.hppcpp/tests/linear_programming/c_api_tests/c_api_tests.hcpp/tests/linear_programming/c_api_tests/c_api_test.ccpp/include/cuopt/linear_programming/cuopt_c.hcpp/src/linear_programming/cuopt_c.cppcpp/src/linear_programming/optimization_problem.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/include/cuopt/linear_programming/constants.hcpp/include/cuopt/linear_programming/optimization_problem.hppcpp/src/linear_programming/cuopt_c_internal.hppcpp/tests/linear_programming/c_api_tests/c_api_tests.hcpp/tests/linear_programming/c_api_tests/c_api_test.ccpp/include/cuopt/linear_programming/cuopt_c.hcpp/src/linear_programming/cuopt_c.cppcpp/src/linear_programming/optimization_problem.cucpp/tests/linear_programming/c_api_tests/c_api_tests.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 problem transformations: verify correctness of original→transformed→postsolve mappings and index consistency across problem representations
Applied to files:
cpp/include/cuopt/linear_programming/optimization_problem.hppcpp/tests/linear_programming/c_api_tests/c_api_tests.hcpp/tests/linear_programming/c_api_tests/c_api_test.ccpp/src/linear_programming/cuopt_c.cppcpp/src/linear_programming/optimization_problem.cucpp/tests/linear_programming/c_api_tests/c_api_tests.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/linear_programming/cuopt_c_internal.hppcpp/include/cuopt/linear_programming/cuopt_c.hcpp/src/linear_programming/cuopt_c.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/linear_programming/cuopt_c_internal.hppcpp/include/cuopt/linear_programming/cuopt_c.h
📚 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/linear_programming/cuopt_c_internal.hppcpp/tests/linear_programming/c_api_tests/c_api_tests.hcpp/src/linear_programming/cuopt_c.cppcpp/src/linear_programming/optimization_problem.cucpp/tests/linear_programming/c_api_tests/c_api_tests.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/linear_programming/cuopt_c_internal.hppcpp/tests/linear_programming/c_api_tests/c_api_tests.hcpp/include/cuopt/linear_programming/cuopt_c.hcpp/src/linear_programming/cuopt_c.cppcpp/libmps_parser/src/mps_writer.cppcpp/src/linear_programming/optimization_problem.cucpp/tests/linear_programming/c_api_tests/c_api_tests.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/linear_programming/cuopt_c_internal.hppcpp/src/linear_programming/optimization_problem.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} : 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/linear_programming/cuopt_c_internal.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/linear_programming/cuopt_c_internal.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} : Write tests validating numerical correctness of optimization results (not just 'runs without error'); test degenerate cases (infeasible, unbounded, empty, singleton problems)
Applied to files:
cpp/tests/linear_programming/c_api_tests/c_api_tests.hcpp/tests/linear_programming/c_api_tests/c_api_test.ccpp/tests/linear_programming/c_api_tests/c_api_tests.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/tests/linear_programming/c_api_tests/c_api_tests.hcpp/tests/linear_programming/c_api_tests/c_api_test.ccpp/src/linear_programming/cuopt_c.cppcpp/tests/linear_programming/c_api_tests/c_api_tests.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/tests/linear_programming/c_api_tests/c_api_tests.hcpp/tests/linear_programming/c_api_tests/c_api_test.ccpp/tests/linear_programming/c_api_tests/c_api_tests.cpp
📚 Learning: 2025-12-06T00:22:48.638Z
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 + ...
Applied to files:
cpp/tests/linear_programming/c_api_tests/c_api_tests.h
📚 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/linear_programming/cuopt_c.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/linear_programming/cuopt_c.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/linear_programming/optimization_problem.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} : 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/linear_programming/optimization_problem.cucpp/tests/linear_programming/c_api_tests/c_api_tests.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 **/*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/linear_programming/optimization_problem.cucpp/tests/linear_programming/c_api_tests/c_api_tests.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 : 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
Applied to files:
cpp/src/linear_programming/optimization_problem.cu
📚 Learning: 2025-12-03T23:29:26.391Z
Learnt from: chris-maes
Repo: NVIDIA/cuopt PR: 500
File: cpp/src/dual_simplex/sparse_matrix.cpp:519-524
Timestamp: 2025-12-03T23:29:26.391Z
Learning: In cpp/src/dual_simplex/sparse_matrix.cpp, the check_matrix() function is debug/diagnostic code (wrapped in #ifdef CHECK_MATRIX) that intentionally prints errors without necessarily returning early. The return codes from this debug code are not actively checked; the purpose is to print all validation errors in one pass for better diagnostics.
Applied to files:
cpp/src/linear_programming/optimization_problem.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 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/tests/linear_programming/c_api_tests/c_api_tests.cpp
🧬 Code graph analysis (4)
cpp/tests/linear_programming/c_api_tests/c_api_tests.h (1)
cpp/tests/linear_programming/c_api_tests/c_api_test.c (1)
test_write_problem(1204-1283)
cpp/include/cuopt/linear_programming/cuopt_c.h (1)
cpp/src/linear_programming/cuopt_c.cpp (8)
cuOptWriteProblem(71-89)cuOptWriteProblem(71-73)cuOptSetInitialPrimalSolution(705-721)cuOptSetInitialPrimalSolution(705-707)cuOptSetInitialDualSolution(723-739)cuOptSetInitialDualSolution(723-725)cuOptAddMIPStart(741-757)cuOptAddMIPStart(741-743)
cpp/src/linear_programming/optimization_problem.cu (2)
cpp/include/cuopt/linear_programming/optimization_problem.hpp (1)
other(120-120)cpp/src/dual_simplex/sparse_matrix.hpp (3)
j(65-65)j(115-126)i(157-168)
cpp/tests/linear_programming/c_api_tests/c_api_tests.cpp (3)
cpp/include/cuopt/linear_programming/optimization_problem.hpp (1)
mps_file_path(358-358)cpp/src/linear_programming/cuopt_c_internal.hpp (1)
problem_and_stream_view_t(22-25)cpp/src/linear_programming/cuopt_c.cpp (6)
cuOptReadProblem(45-69)cuOptReadProblem(45-45)cuOptWriteProblem(71-89)cuOptWriteProblem(71-73)cuOptDestroyProblem(338-344)cuOptDestroyProblem(338-338)
⏰ 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.0.2, 3.13, 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, amd64, rockylinux8
- 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.11, 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.10, amd64, rockylinux8
- GitHub Check: wheel-build-cuopt-mps-parser / 13.0.2, 3.12, amd64, rockylinux8
- GitHub Check: wheel-build-cuopt-sh-client / 13.0.2, 3.10, amd64, rockylinux8
- GitHub Check: checks / check-style
🔇 Additional comments (24)
cpp/src/linear_programming/optimization_problem.cu (2)
20-29: LGTM: Appropriate includes for GPU operations.The new Thrust and RMM includes support the GPU-accelerated equivalence checking functionality.
107-205: Thrust operations can throw exceptions that propagate throughis_equivalent().The Thrust algorithms in
csr_matrices_equivalent_with_permutation()(lines 107-205) usermm::exec_policy(stream), which can throw exceptions on memory allocation failures or CUDA errors. These exceptions will naturally propagate throughis_equivalent()to calling code. The semantic mismatch—abool-returning function that can throw—may surprise callers; verify that calling code is prepared to handle these exceptions or add explicit error handling with meaningful error codes if the function is part of a user-facing API.cpp/include/cuopt/linear_programming/optimization_problem.hpp (1)
108-120: LGTM: Well-documented public API addition.The is_equivalent method is properly documented with clear semantics about variable/constraint name-based matching and permutation handling.
cpp/tests/linear_programming/CMakeLists.txt (1)
55-57: Clarify the need for conditional linker flag.The conditional addition of
-Wl,--enable-new-dtagsis unusual. This flag affects how RPATH is set (RUNPATH vs RPATH). Please document why this is needed specifically for C_API_TEST and only when INSTALL_TARGET is not defined.cpp/include/cuopt/linear_programming/constants.h (1)
110-111: LGTM: File format constant for MPS I/O.The new constant properly supports the MPS file format specification for cuOptWriteProblem.
cpp/tests/linear_programming/c_api_tests/c_api_tests.h (1)
37-37: LGTM: Test function declaration.The new test_write_problem declaration follows the existing pattern in the test suite.
cpp/tests/linear_programming/c_api_tests/c_api_tests.cpp (4)
10-14: LGTM: Includes for roundtrip testing.The filesystem and internal header includes support the new MPS roundtrip validation tests.
136-143: LGTM: Basic write test with cleanup.The test exercises the write API and properly cleans up the temporary file.
145-187: Excellent: Comprehensive roundtrip equivalence validation.This helper properly tests write/read roundtrips by verifying that the original and re-read problems are mathematically equivalent using the new is_equivalent method. The use of goto cleanup ensures proper resource cleanup even on error paths.
This addresses the past review comment about checking equality between original and read/write problems.
189-223: LGTM: Comprehensive test coverage.The parameterized test suite validates roundtrip integrity across a diverse set of MPS problem instances, including LP, MIP, and various edge cases (free bounds, crossing bounds, infeasible problems).
As per coding guidelines, this provides good test coverage for problem transformations and I/O operations.
cpp/src/linear_programming/cuopt_c.cpp (5)
14-14: LGTM: Internal header include.The cuopt_c_internal.hpp include provides access to the problem_and_stream_view_t wrapper needed for the new APIs.
71-89: LGTM: Comprehensive input validation.The cuOptWriteProblem implementation properly validates all inputs including the empty filename check (line 77), addresses the past review comment, and handles exceptions appropriately.
Addresses past review comment about checking for empty filename.
705-721: LGTM: Proper validation for initial primal solution.The function correctly validates settings, primal_solution pointer, and array size before passing to the underlying solver settings.
723-739: LGTM: Proper validation for initial dual solution.The function correctly validates settings, dual_solution pointer, and array size before passing to the underlying solver settings.
741-757: LGTM: Proper validation for MIP start.The function correctly validates settings, solution pointer, and array size before adding to MIP settings.
cpp/include/cuopt/linear_programming/cuopt_c.h (2)
114-126: Good API addition with clear documentation.The
cuOptWriteProblemfunction follows existing API conventions and provides proper Doxygen documentation. The implementation (per relevant snippets) validates inputs correctly.Consider documenting memory ownership and thread-safety in a future iteration:
- Whether the function is thread-safe
- Whether file I/O blocks the calling thread
726-743: Good documentation of the presolve limitation.The
@attentiontag clearly documents that MIP starts are currently unsupported with presolve enabled. This is helpful for users.One consideration: the function name
cuOptAddMIPStartimplies multiple starts can be added (and the doc confirms this), which aligns well with the implementation usingadd_initial_solution.cpp/libmps_parser/src/mps_writer.cpp (5)
141-169: Good handling of orphan variables for MPS compliance.The tracking of variables that don't appear in any constraint is necessary for correct MPS output. The separation by type (integer vs continuous) ensures orphan variables are emitted within the correct INTORG/INTEND marker sections.
175-184: Orphan variables always emit objective coefficient, including zero.For orphan variables, the code unconditionally emits the objective coefficient (line 183), even when zero. This differs from the non-orphan handling (lines 196-200) which only emits non-zero coefficients. This is intentional to ensure the variable is declared in COLUMNS, but it creates an inconsistency in the output format.
This is acceptable since the MPS spec allows zero coefficients, and some parsers require column declarations before bounds. Just noting the asymmetry for awareness.
260-264: Good explicit handling of fixed variables.The explicit FX BOUND1 emission for fixed variables (when lower == upper) avoids ambiguity in the MPS spec regarding the interpretation of UP BOUND1 with value 0 when lower bound is also 0. This defensive approach improves compatibility with various MPS parsers.
274-279: Integer upper bound emission handles infinity correctly.The condition correctly emits upper bounds for integer variables even when infinite, which is necessary because integer variables have different default bounds than continuous variables in some MPS readers. The UI bound type is correctly used for integers.
30-35: File handle cleanup follows RAII via std::ofstream.The file is opened via
std::ofstreamand will be automatically closed when the function exits (including on exceptions). The explicitmps_file.close()at line 284 is redundant but harmless. Themps_parser_expectscheck at line 32-35 ensures early failure if the file cannot be opened. As per coding guidelines, this follows proper RAII patterns.cpp/src/linear_programming/cuopt_c_internal.hpp (2)
21-30: Member declaration order differs from initialization order.The member initialization list initializes
op_problem,stream_view,handlein that order, but members are declared as:op_problem(line 27),stream_view(line 28),handle(line 29). Since C++ initializes members in declaration order (not initializer list order), this works correctly here. However, the initializer list orderop_problem(nullptr), stream_view(...), handle(...)matches the declaration order, so this is fine.The use of
rmm::cuda_stream_per_threadis appropriate for thread-safe C API usage. As per coding guidelines, stream lifecycle is documented by the choice of per-thread streams.
32-44: Raw pointers require external ownership management.The
solution_and_stream_view_tstruct stores raw pointers tomip_solution_tandoptimization_problem_solution_t. This is acceptable for an internal struct where ownership is managed by the C API functions (cuOptSolve creates, cuOptDestroySolution destroys).Ensure that the C API implementation properly manages the lifecycle of these pointers. Based on the existing pattern (void* handles with create/destroy functions), this appears to be the intended design.
| template <typename i_t, typename f_t> | ||
| bool optimization_problem_t<i_t, f_t>::is_equivalent( | ||
| const optimization_problem_t<i_t, f_t>& other) const | ||
| { | ||
| if (maximize_ != other.maximize_) { return false; } | ||
| if (n_vars_ != other.n_vars_) { return false; } | ||
| if (n_constraints_ != other.n_constraints_) { return false; } | ||
| if (objective_scaling_factor_ != other.objective_scaling_factor_) { return false; } | ||
| if (objective_offset_ != other.objective_offset_) { return false; } | ||
| if (problem_category_ != other.problem_category_) { return false; } | ||
| if (A_.size() != other.A_.size()) { return false; } | ||
|
|
||
| if (var_names_.empty() || other.var_names_.empty()) { return false; } | ||
| if (row_names_.empty() || other.row_names_.empty()) { return false; } | ||
|
|
||
| // Build variable permutation: var_perm[i] = index j in other where var_names_[i] == | ||
| // other.var_names_[j] | ||
| std::unordered_map<std::string, i_t> other_var_idx; | ||
| for (size_t j = 0; j < other.var_names_.size(); ++j) { | ||
| other_var_idx[other.var_names_[j]] = static_cast<i_t>(j); | ||
| } | ||
| std::vector<i_t> var_perm(n_vars_); | ||
| for (i_t i = 0; i < n_vars_; ++i) { | ||
| auto it = other_var_idx.find(var_names_[i]); | ||
| if (it == other_var_idx.end()) { return false; } | ||
| var_perm[i] = it->second; | ||
| } | ||
|
|
||
| // Build row permutation: row_perm[i] = index j in other where row_names_[i] == | ||
| // other.row_names_[j] | ||
| std::unordered_map<std::string, i_t> other_row_idx; | ||
| for (size_t j = 0; j < other.row_names_.size(); ++j) { | ||
| other_row_idx[other.row_names_[j]] = static_cast<i_t>(j); | ||
| } | ||
| std::vector<i_t> row_perm(n_constraints_); | ||
| for (i_t i = 0; i < n_constraints_; ++i) { | ||
| auto it = other_row_idx.find(row_names_[i]); | ||
| if (it == other_row_idx.end()) { return false; } | ||
| row_perm[i] = it->second; | ||
| } | ||
|
|
||
| // Upload permutations to GPU | ||
| rmm::device_uvector<i_t> d_var_perm(n_vars_, stream_view_); | ||
| rmm::device_uvector<i_t> d_row_perm(n_constraints_, stream_view_); | ||
| raft::copy(d_var_perm.data(), var_perm.data(), n_vars_, stream_view_); | ||
| raft::copy(d_row_perm.data(), row_perm.data(), n_constraints_, stream_view_); | ||
|
|
||
| auto policy = rmm::exec_policy(stream_view_); | ||
|
|
||
| auto permuted_eq = [&](auto this_begin, auto this_end, auto other_begin, auto perm_begin) { | ||
| auto other_perm = thrust::make_permutation_iterator(other_begin, perm_begin); | ||
| return thrust::equal(policy, this_begin, this_end, other_perm); | ||
| }; | ||
|
|
||
| // Compare variable-indexed arrays | ||
| if (!permuted_eq(c_.begin(), c_.end(), other.c_.begin(), d_var_perm.begin())) { return false; } | ||
| if (!permuted_eq(variable_lower_bounds_.begin(), | ||
| variable_lower_bounds_.end(), | ||
| other.variable_lower_bounds_.begin(), | ||
| d_var_perm.begin())) { | ||
| return false; | ||
| } | ||
| if (!permuted_eq(variable_upper_bounds_.begin(), | ||
| variable_upper_bounds_.end(), | ||
| other.variable_upper_bounds_.begin(), | ||
| d_var_perm.begin())) { | ||
| return false; | ||
| } | ||
| if (!permuted_eq(variable_types_.begin(), | ||
| variable_types_.end(), | ||
| other.variable_types_.begin(), | ||
| d_var_perm.begin())) { | ||
| return false; | ||
| } | ||
|
|
||
| // Compare constraint-indexed arrays | ||
| if (!permuted_eq(b_.begin(), b_.end(), other.b_.begin(), d_row_perm.begin())) { return false; } | ||
| if (!permuted_eq(constraint_lower_bounds_.begin(), | ||
| constraint_lower_bounds_.end(), | ||
| other.constraint_lower_bounds_.begin(), | ||
| d_row_perm.begin())) { | ||
| return false; | ||
| } | ||
| if (!permuted_eq(constraint_upper_bounds_.begin(), | ||
| constraint_upper_bounds_.end(), | ||
| other.constraint_upper_bounds_.begin(), | ||
| d_row_perm.begin())) { | ||
| return false; | ||
| } | ||
| if (!permuted_eq( | ||
| row_types_.begin(), row_types_.end(), other.row_types_.begin(), d_row_perm.begin())) { | ||
| return false; | ||
| } | ||
|
|
||
| // Build inverse permutations on CPU (needed for CSR comparisons) | ||
| std::vector<i_t> var_perm_inv(n_vars_); | ||
| for (i_t i = 0; i < n_vars_; ++i) { | ||
| var_perm_inv[var_perm[i]] = i; | ||
| } | ||
| std::vector<i_t> row_perm_inv(n_constraints_); | ||
| for (i_t i = 0; i < n_constraints_; ++i) { | ||
| row_perm_inv[row_perm[i]] = i; | ||
| } | ||
|
|
||
| // Upload inverse permutations to GPU | ||
| rmm::device_uvector<i_t> d_var_perm_inv(n_vars_, stream_view_); | ||
| rmm::device_uvector<i_t> d_row_perm_inv(n_constraints_, stream_view_); | ||
| raft::copy(d_var_perm_inv.data(), var_perm_inv.data(), n_vars_, stream_view_); | ||
| raft::copy(d_row_perm_inv.data(), row_perm_inv.data(), n_constraints_, stream_view_); | ||
|
|
||
| // Constraint matrix (A) comparison with row and column permutations | ||
| if (!csr_matrices_equivalent_with_permutation(A_offsets_, | ||
| A_indices_, | ||
| A_, | ||
| other.A_offsets_, | ||
| other.A_indices_, | ||
| other.A_, | ||
| d_row_perm_inv, | ||
| d_var_perm_inv, | ||
| n_vars_, | ||
| stream_view_)) { | ||
| return false; | ||
| } | ||
|
|
||
| // Q matrix writing to MPS not supported yet. Don't check for equivalence here | ||
|
|
||
| return true; | ||
| } |
There was a problem hiding this comment.
Add problem size validation and stream synchronization.
The method performs multiple GPU allocations (lines 249-252, 312-315) without checking if the problem dimensions are reasonable. For very large problems (millions of variables/constraints), this could cause OOM errors.
Additionally, the method uses asynchronous GPU operations but doesn't synchronize the stream before returning the boolean result (line 333), which could lead to race conditions.
Apply this diff to add size checks and synchronization:
bool optimization_problem_t<i_t, f_t>::is_equivalent(
const optimization_problem_t<i_t, f_t>& other) const
{
if (maximize_ != other.maximize_) { return false; }
if (n_vars_ != other.n_vars_) { return false; }
if (n_constraints_ != other.n_constraints_) { return false; }
+
+ // Prevent OOM on very large problems
+ constexpr i_t max_reasonable_size = 100000000; // 100M
+ if (n_vars_ > max_reasonable_size || n_constraints_ > max_reasonable_size) {
+ CUOPT_LOG_WARN("Problem too large for equivalence check");
+ return false;
+ }
+
if (objective_scaling_factor_ != other.objective_scaling_factor_) { return false; }
// ... rest of checks ...
// Q matrix writing to MPS not supported yet. Don't check for equivalence here
+ // Ensure all async GPU operations complete before returning
+ stream_view_.synchronize();
return true;
}As per coding guidelines, verify correct problem size checks before expensive GPU operations.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In cpp/src/linear_programming/optimization_problem.cu around lines 207 to 334,
add explicit problem-size validation before any GPU allocations (before creating
d_var_perm/d_row_perm and before creating d_var_perm_inv/d_row_perm_inv): check
n_vars_ and n_constraints_ (and A_.size()/other.A_.size() where relevant)
against a sensible maximum (or check for zero/negative values) and return false
(or raise a clear error) if sizes are unreasonable to avoid attempting huge
allocations; also add a stream synchronization (e.g., synchronize stream_view_
or call stream_view_.synchronize()) after all raft/thrust copy and comparison
calls but before the final return true to ensure asynchronous GPU work is
complete and avoid race conditions.
|
🔔 Hi @anandhkb, this pull request has had no activity for 7 days. Please update or let us know if it can be closed. Thank you! If this is an "epic" issue, then please add the "epic" label to this issue. |
4 participants
This PR extends the C API to add support for saving the problem to a file, adding initial primal and dual solutions for LP, and initial solutions for MIP. This functionality exists in the underlying C++ layer, and is a feature of most LP/MIP solver APIs available.
Kept as a draft for now, as it is intended as a proposal to iterate on.
closes #719
closes #720
Summary by CodeRabbit
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.