refactor(tests): link internal tests against cuopt_static by ramakrishnap-nv · Pull Request #1581 · NVIDIA/cuopt

ramakrishnap-nv · 2026-07-16T15:40:41Z

Summary

Prerequisite structural work for enabling -fvisibility=hidden on libcuopt.so (issue #1213). An earlier attempt using CUOPT_INTERNAL_EXPORT annotations (PR #1574) was closed after the scope of symbols to annotate proved too large (including static data members that cannot carry visibility attributes).

This PR puts the test infrastructure in place so that hidden visibility can be enabled in a follow-up.

cuopt_objs OBJECT library — compile sources once; both cuopt (shared) and cuopt_static (static, BUILD_TESTS only) consume the same object files
STATIC_LIB option in ConfigureTest — links cuopt_static and omits cuopttestutils (which transitively pulls in the shared lib) for tests that need compiled internal symbols
ROUTING_INTERNAL_TEST — new target consolidating GES solver, cross-route insertion, local-search candidate, and top-k tests; also registers four previously unbuilt files (l0_scross_test.cu, local_search_cand_test.cu, load_balancing_test.cu, feasibility_jump_tests.cu)
gRPC tests — GRPC_CLIENT_TEST and GRPC_INTEGRATION_TEST now compile the gRPC client/mapper sources directly into the test binary, following the pattern already used by GRPC_PIPE_SERIALIZATION_TEST; no internal symbols need exporting from the shared lib
cuOptIsCusparseRuntimeMixedPrecisionSupported() — promoted from an internal function to the public C API; c_api_tests.cpp updated to call the public version

copy-pr-bot · 2026-07-16T15:40:45Z

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.

coderabbitai · 2026-07-16T19:55:14Z

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

@coderabbitai resume to resume automatic reviews.
@coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

▶️ Resume reviews
🔍 Trigger review

📝 Walkthrough

Walkthrough

Changes

The build now compiles shared CUOPT objects once for shared and static libraries. Test linking supports static-library targets, internal tests share one entry point, test registrations are consolidated, gRPC sources are expanded, and the PDLP precision test uses compile-time cuSPARSE version checks.

CUOPT build refactor

Layer / File(s)	Summary
Object-based library targets `cpp/CMakeLists.txt`, `cpp/src/utilities/omp_helpers.hpp`	`cuopt_objs` supplies shared objects to `cuopt_static` and `cuopt`, with dependencies, compile definitions, includes, and namespace guard placement updated.
Shared test linking and entry points `cpp/tests/CMakeLists.txt`, `cpp/tests/internal/`, `cpp/tests/routing/CMakeLists.txt`, `cpp/tests//*_test.cu`	`ConfigureTest` supports `STATIC_LIB`; internal tests use `NUMOPT_INTERNAL_TEST` and a shared RMM/GoogleTest main, while selected test-specific main macros are removed.
Test suite and integration targets `cpp/tests/linear_programming/CMakeLists.txt`, `cpp/tests/linear_programming/grpc/CMakeLists.txt`, `cpp/tests/mip/CMakeLists.txt`, `cpp/tests/mip/*_test.cu`	Test registrations and gRPC compilation inputs are revised, and MIP helper functions receive internal linkage.
cuSPARSE precision test branching `cpp/tests/linear_programming/c_api_tests/c_api_tests.cpp`	The mixed-precision test branches on `CUSPARSE_VERSION` and validates supported or unsupported solve outcomes accordingly.

Estimated code review effort: 4 (Complex) | ~45 minutes

Suggested reviewers: hlinsen, yuwenchen95, tmckayus

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 10.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title is concise and matches a major part of the changes: internal tests now link against cuopt_static.
Description check	✅ Passed	The description is clearly related to the refactor and matches the changeset's test infrastructure updates.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch refactor/test-static-link-for-internal-tests

_{Comment @coderabbitai help to get the list of available commands.}

github-actions · 2026-07-16T19:55:27Z

CI Test Summary

✅ All 0 test job(s) passed.

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@cpp/CMakeLists.txt`:
- Around line 639-677: Update target_link_libraries for the shared cuopt target
to link PSLP directly, matching the existing cuopt_static linkage by reusing
$<TARGET_FILE:PSLP>. Place it with the other required PUBLIC dependencies so
third_party_presolve.cpp symbols resolve when building cuopt.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: ca9f970f-6e82-46b2-bac4-a965fe3f4b2a

📥 Commits

Reviewing files that changed from the base of the PR and between d66bf29 and 349b12a.

📒 Files selected for processing (18)

cpp/CMakeLists.txt
cpp/include/cuopt/mathematical_optimization/cuopt_c.h
cpp/src/pdlp/cuopt_c.cpp
cpp/src/utilities/omp_helpers.hpp
cpp/tests/CMakeLists.txt
cpp/tests/dual_simplex/CMakeLists.txt
cpp/tests/internal/CMakeLists.txt
cpp/tests/internal/main.cu
cpp/tests/linear_programming/CMakeLists.txt
cpp/tests/linear_programming/c_api_tests/c_api_tests.cpp
cpp/tests/linear_programming/grpc/CMakeLists.txt
cpp/tests/mip/CMakeLists.txt
cpp/tests/mip/bounds_standardization_test.cu
cpp/tests/mip/elim_var_remap_test.cu
cpp/tests/mip/multi_probe_test.cu
cpp/tests/routing/CMakeLists.txt
cpp/tests/socp/CMakeLists.txt
cpp/tests/utilities/base_fixture.hpp

💤 Files with no reviewable changes (3)

cpp/tests/dual_simplex/CMakeLists.txt
cpp/tests/linear_programming/CMakeLists.txt
cpp/tests/socp/CMakeLists.txt

mlubin · 2026-07-16T21:41:05Z

 * the command line to customize test behavior, like the allocation mode used
 * for creating the default memory resource.
+ *
+ * When CUOPT_DISABLE_TEST_MAIN is defined (e.g. when compiling multiple test


Do we need this macro? What about deleting CUOPT_TEST_PROGRAM_MAIN in the test files that no longer need a main?

akifcorduk · 2026-07-17T08:11:26Z

load_balancing_test.cu, feasibility_jump_tests.cu the description says these are now part of ROUTING_INTERNAL_TEST but they are MIP tests.

akifcorduk · 2026-07-17T08:11:43Z

+ *
+ * @return 1 if supported, 0 otherwise.
+ */
+cuopt_int_t cuOptIsCusparseRuntimeMixedPrecisionSupported();


Why do we make this a public function?

The test needs to know whether the cuSPARSE runtime supports mixed-precision SpMV so it can set the right expectation (success vs. error/non-optimal). Originally I added it as a public C API so the test binary (which links only against the shared cuopt) could call it — but that unnecessarily grows our public surface with an implementation detail.

Replaced with a compile-time #if CUSPARSE_VERSION >= 12500 check in the test. For cuOpt's deployment model the compile-time and runtime toolkit versions match, so this is equivalent and avoids adding anything to the public API.

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@cpp/tests/linear_programming/c_api_tests/c_api_tests.cpp`:
- Around line 452-456: Update the test around test_pdlp_precision_mixed so it
requires status != CUOPT_SUCCESS directly. Remove the
solve_returned_error/solve_returned_non_optimal combination and assert the API
reports an error, ensuring unsupported mixed precision cannot pass with a
successful non-optimal termination.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 1961826f-9b83-4189-88a2-742447184a00

📥 Commits

Reviewing files that changed from the base of the PR and between 921078b and 7677573.

📒 Files selected for processing (9)

cpp/tests/internal/CMakeLists.txt
cpp/tests/internal/main.cu
cpp/tests/linear_programming/c_api_tests/c_api_tests.cpp
cpp/tests/linear_programming/pdlp_test.cu
cpp/tests/linear_programming/unit_tests/presolve_test.cu
cpp/tests/routing/CMakeLists.txt
cpp/tests/routing/level0/l0_ges_test.cu
cpp/tests/routing/unit_tests/top_k.cu
cpp/tests/utilities/base_fixture.hpp

💤 Files with no reviewable changes (7)

cpp/tests/routing/level0/l0_ges_test.cu
cpp/tests/linear_programming/unit_tests/presolve_test.cu
cpp/tests/internal/main.cu
cpp/tests/linear_programming/pdlp_test.cu
cpp/tests/internal/CMakeLists.txt
cpp/tests/utilities/base_fixture.hpp
cpp/tests/routing/CMakeLists.txt

ramakrishnap-nv · 2026-07-17T15:41:15Z

Good catch — those files were tried during development but removed because their constructor signatures no longer match the current API (load_balancing_test.cu and feasibility_jump_tests.cu are MIP tests, not routing, and both had stale APIs). The description was wrong; updated to accurately describe what's in each target.

mlubin

Wait for @akifcorduk's approval as well

Prerequisite structural work for enabling -fvisibility=hidden on libcuopt.so (issue #1213). - Introduce cuopt_objs OBJECT library so cuopt (shared) and cuopt_static (static, BUILD_TESTS only) share one compile step - Add STATIC_LIB option to ConfigureTest; when set, links cuopt_static and omits cuopttestutils to avoid transitive cuopt-shared pull-in - Route all tests with compiled internal symbol dependencies through STATIC_LIB: DUAL_SIMPLEX_TEST, LP_INTERNAL_TEST, PDLP_TEST, MPS_PARSER_TEST, most MIP tests, SOCP_TEST, ROUTING_INTERNAL_TEST - Create ROUTING_INTERNAL_TEST consolidating GES, scross, local-search candidate, and top-k tests; register previously unbuilt test files (l0_scross_test.cu, local_search_cand_test.cu, load_balancing_test.cu, feasibility_jump_tests.cu) - Add gRPC source files directly to GRPC_CLIENT_TEST and GRPC_INTEGRATION_TEST so they do not depend on internal symbols being exported from the shared library - Promote is_cusparse_runtime_mixed_precision_supported() to the public C API as cuOptIsCusparseRuntimeMixedPrecisionSupported(); update c_api_tests.cpp to use the public function Signed-off-by: Ramakrishna Prabhu <ramakrishnap@nvidia.com>

Collapse 19 separate STATIC_LIB test binaries (dual_simplex, LP internal, PDLP, MPS parser, 11 MIP, SOCP) into a single NUMOPT_INTERNAL_TEST binary linked against cuopt_static. Routing internal tests remain in ROUTING_INTERNAL_TEST. This reduces total static-linked binary count from 19 to 2, cutting link time and disk usage. Also fix cuopt_c.cpp to forward-declare is_cusparse_runtime_mixed_precision_supported() instead of including cusparse_view.hpp, which pulled CUDA intrinsics into a CXX translation unit. Signed-off-by: Ramakrishna Prabhu <ramakrishnap@nvidia.com>

Fix four issues discovered when building NUMOPT_INTERNAL_TEST and ROUTING_INTERNAL_TEST: - omp_helpers.hpp: closing namespace brace was outside #ifdef _OPENMP, causing "expected a declaration" when compiled without -fopenmp. - cuopt_static: propagate OpenMP::OpenMP_CXX PUBLIC and -fopenmp for CUDA TUs so test binaries that link cuopt_static compile OMP-heavy internal headers correctly. - cuopt shared: restore CUSPARSE_ENABLE_EXPERIMENTAL_API and CUOPT_LOG_ACTIVE_LEVEL as PUBLIC compile definitions; they were lost when compile definitions moved from cuopt to cuopt_objs (the $<TARGET_OBJECTS:...> pattern does not carry INTERFACE properties). - Combined binaries: add CUOPT_DISABLE_TEST_MAIN guard to CUOPT_TEST_PROGRAM_MAIN() in base_fixture.hpp; provide a single main.cu entry point per binary; make init_handler static in three MIP test files to resolve ODR violations; compile check_constraints.cu directly into ROUTING_INTERNAL_TEST for check_route; add missing src/ and src/io/ include paths to GRPC_CLIENT_TEST and GRPC_INTEGRATION_TEST. Signed-off-by: Ramakrishna Prabhu <ramakrishnap@nvidia.com>

Signed-off-by: Ramakrishna Prabhu <ramakrishnap@nvidia.com>

All tests from these directories moved to NUMOPT_INTERNAL_TEST. The subdirectories and add_subdirectory calls serve no purpose now. Signed-off-by: Ramakrishna Prabhu <ramakrishnap@nvidia.com>

$<TARGET_OBJECTS:cuopt_objs> does not propagate PRIVATE link dependencies to consuming targets; PSLP must be linked explicitly on both cuopt_static and cuopt (shared). Signed-off-by: Ramakrishna Prabhu <ramakrishnap@nvidia.com>

…test files Files compiled into NUMOPT_INTERNAL_TEST and ROUTING_INTERNAL_TEST no longer need their own main(); remove the call directly rather than suppressing it via the CUOPT_DISABLE_TEST_MAIN guard. Simplifies base_fixture.hpp and the CMakeLists files. Signed-off-by: Ramakrishna Prabhu <ramakrishnap@nvidia.com>

Remove cuOptIsCusparseRuntimeMixedPrecisionSupported from the public C API. The c_api_test already includes internal headers; use the compile-time CUSPARSE_VERSION macro instead, which is accurate for cuOpt's deployment model (compile-time and runtime toolkit match). Signed-off-by: Ramakrishna Prabhu <ramakrishnap@nvidia.com>

When cuSPARSE < 12.5, cuopt_expects throws ValidationError before the solve begins, so the C API always returns an error — CUOPT_SUCCESS with non-optimal is impossible. Assert status != CUOPT_SUCCESS directly. Signed-off-by: Ramakrishna Prabhu <ramakrishnap@nvidia.com>

After thrust::exclusive_scan, offsets.back() already equals the total number of non-zeros (standard CSR sentinel). The previous + 1 produced one entry beyond the last row, which is invalid CSR and caused cusparseXcsrsort to produce incorrect results when assertions are enabled (PR CI uses -a / DEFINE_ASSERT=ON which enables -UNDEBUG). The resulting out-of-bounds column index triggered the check_csr_representation assertion in compute_transpose_of_problem, crashing NUMOPT_INTERNAL_TEST with SIGABRT in PR CI. Signed-off-by: Ramakrishna Prabhu <ramakrishnap@nvidia.com>

The compile-time #if CUSPARSE_VERSION >= 12500 guard only checks the header version used during build. On systems where the runtime cuSPARSE library is older than 12.5 (e.g., oldest-deps conda builds), the solver correctly throws ValidationError via is_cusparse_runtime_mixed_precision_supported(), but the test unconditionally expected CUOPT_SUCCESS. Fix by calling cusparseGetProperty() at runtime in the test to mirror the same check the solver performs, routing the test to the success or failure assertion accordingly. Signed-off-by: Ramakrishna Prabhu <ramakrishnap@nvidia.com>

top_k.cu was moved to ROUTING_INTERNAL_TEST because it uses internal symbols (routing/detail). It was also the only source in ROUTING_UNIT_TEST that provided CUOPT_TEST_PROGRAM_MAIN(), which sets up the RMM device memory resource before running tests. Without RMM setup, the routing solver allocations use the default cuda_memory_resource, causing SIGSEGV in vehicle_breaks.non_uniform_breaks (and potentially other tests that stress the allocator). Fix by adding internal/main.cu to ROUTING_UNIT_TEST sources. The custom main() it provides takes precedence over GTest::gmock_main's default main and properly initializes the RMM memory resource. Signed-off-by: Ramakrishna Prabhu <ramakrishnap@nvidia.com>

The SPDX header used bare `/* text` lines without closing `*/` on each line, leaving an unclosed C comment. Subsequent `/*` markers triggered -Werror=comment when internal/main.cu was compiled into new targets (e.g. ROUTING_UNIT_TEST). Use the standard block-comment style matching the rest of the test suite. Signed-off-by: Ramakrishna Prabhu <ramakrishnap@nvidia.com>

cuopt_objs is the OBJECT library that compiles all CUDA TUs. Before the cuopt_objs refactor, cuopt SHARED had OpenMP::OpenMP_CUDA as a PRIVATE dep, which passed -fopenmp to nvcc and defined _OPENMP for all .cu files. After the refactor, cuopt_objs only inherited OpenMP::OpenMP_CXX (via CUOPT_PRIVATE_CUDA_LIBS), which only applies to CXX compilation. Without -fopenmp, omp_atomic_t<T> is undefined in CUDA TUs that include omp_helpers.hpp (e.g. via dual_simplex/solution.hpp), causing build failures. Signed-off-by: Ramakrishna Prabhu <ramakrishnap@nvidia.com>

ramakrishnap-nv added this to the 26.08 milestone Jul 16, 2026

ramakrishnap-nv added non-breaking Introduces a non-breaking change improvement Improves an existing functionality labels Jul 16, 2026

ramakrishnap-nv marked this pull request as ready for review July 16, 2026 19:53

ramakrishnap-nv requested review from a team as code owners July 16, 2026 19:53

ramakrishnap-nv requested review from hlinsen, tmckayus and yuwenchen95 July 16, 2026 19:53

coderabbitai Bot reviewed Jul 16, 2026

View reviewed changes

Comment thread cpp/CMakeLists.txt

ramakrishnap-nv mentioned this pull request Jul 16, 2026

[DRAFT] Minimize cuopt_static test linkage to avoid 5 GB package size explosion #1566

Closed

3 tasks

mlubin reviewed Jul 16, 2026

View reviewed changes

akifcorduk reviewed Jul 17, 2026

View reviewed changes

coderabbitai Bot reviewed Jul 17, 2026

View reviewed changes

Comment thread cpp/tests/linear_programming/c_api_tests/c_api_tests.cpp Outdated

ramakrishnap-nv requested review from akifcorduk and mlubin July 20, 2026 17:14

mlubin approved these changes Jul 20, 2026

View reviewed changes

ramakrishnap-nv added 7 commits July 20, 2026 12:38

style(tests): remove stale comments from empty CMakeLists stubs

a59bd8c

Signed-off-by: Ramakrishna Prabhu <ramakrishnap@nvidia.com>

refactor(tests): remove empty dual_simplex and socp CMakeLists stubs

bd27745

All tests from these directories moved to NUMOPT_INTERNAL_TEST. The subdirectories and add_subdirectory calls serve no purpose now. Signed-off-by: Ramakrishna Prabhu <ramakrishnap@nvidia.com>

fix(build): link PSLP directly on cuopt shared target

0e5689a

$<TARGET_OBJECTS:cuopt_objs> does not propagate PRIVATE link dependencies to consuming targets; PSLP must be linked explicitly on both cuopt_static and cuopt (shared). Signed-off-by: Ramakrishna Prabhu <ramakrishnap@nvidia.com>

ramakrishnap-nv added 6 commits July 20, 2026 12:39

ramakrishnap-nv force-pushed the refactor/test-static-link-for-internal-tests branch from 39fe088 to 755ec5d Compare July 20, 2026 17:40

Uh oh!

Conversation

ramakrishnap-nv commented Jul 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Uh oh!

copy-pr-bot Bot commented Jul 16, 2026

Uh oh!

coderabbitai Bot commented Jul 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviews paused

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

github-actions Bot commented Jul 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

CI Test Summary

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mlubin Jul 16, 2026

Choose a reason for hiding this comment

Uh oh!

akifcorduk commented Jul 17, 2026

Uh oh!

akifcorduk Jul 17, 2026

Choose a reason for hiding this comment

Uh oh!

ramakrishnap-nv Jul 17, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ramakrishnap-nv commented Jul 17, 2026

Uh oh!

mlubin left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ramakrishnap-nv commented Jul 16, 2026 •

edited

Loading

coderabbitai Bot commented Jul 16, 2026 •

edited

Loading

github-actions Bot commented Jul 16, 2026 •

edited

Loading