From 0f5ae25068c19f8bdb032c92330f5fee51af5e27 Mon Sep 17 00:00:00 2001 From: Ramakrishna Prabhu Date: Tue, 25 Aug 2026 15:25:52 -0500 Subject: [PATCH 1/9] refactor: split host-only members out of CUDA translation units Several classes are mostly host code but live entirely in .cu files, which means anything needing their host-side members has to link the CUDA library. This separates them so the host halves compile as plain C++. math_optimization/solver_settings.cu -> .cpp + _gpu.cu (713 lines, 5 CUDA) mip_heuristics/solver_settings.cu -> .cu + .cpp (58 lines, 3 CUDA) pdlp/solution_conversion.cu -> + solution_conversion_cpu.cpp Each split follows one rule: host code moves to the .cpp, members taking an rmm::cuda_stream_view or returning a device_uvector stay in the .cu, and the moved members are instantiated explicitly per-member rather than via `template class`. The distinction matters -- `template class` in the .cpp would emit device ctors/dtors for members the host file cannot construct. The explicit instantiations are guarded on MIP_INSTANTIATE_* / PDLP_INSTANTIATE_*, so each new file includes mip_heuristics/mip_constants.hpp. Without it the guards evaluate false and the translation unit silently compiles to zero symbols. Also replaces thrust::count with std::count in solve_remote.cpp; it operates on a host vector, so thrust was gratuitous. No behaviour change: every moved definition is byte-identical, and all files still build into libcuopt exactly as before. Co-Authored-By: Claude Opus 5 Signed-off-by: Ramakrishna Prabhu --- cpp/src/grpc/client/solve_remote.cpp | 5 +- cpp/src/math_optimization/CMakeLists.txt | 3 +- ...solver_settings.cu => solver_settings.cpp} | 80 -------- .../math_optimization/solver_settings_gpu.cu | 181 ++++++++++++++++++ cpp/src/mip_heuristics/CMakeLists.txt | 1 + cpp/src/mip_heuristics/solver_settings.cpp | 67 +++++++ cpp/src/mip_heuristics/solver_settings.cu | 23 --- cpp/src/pdlp/CMakeLists.txt | 1 + cpp/src/pdlp/solution_conversion.cu | 86 --------- cpp/src/pdlp/solution_conversion_cpu.cpp | 112 +++++++++++ 10 files changed, 366 insertions(+), 193 deletions(-) rename cpp/src/math_optimization/{solver_settings.cu => solver_settings.cpp} (90%) create mode 100644 cpp/src/math_optimization/solver_settings_gpu.cu create mode 100644 cpp/src/mip_heuristics/solver_settings.cpp create mode 100644 cpp/src/pdlp/solution_conversion_cpu.cpp diff --git a/cpp/src/grpc/client/solve_remote.cpp b/cpp/src/grpc/client/solve_remote.cpp index eabea39e05..ff808a9fdd 100644 --- a/cpp/src/grpc/client/solve_remote.cpp +++ b/cpp/src/grpc/client/solve_remote.cpp @@ -14,6 +14,7 @@ #include #include "grpc_client.hpp" +#include #include #include #include @@ -22,8 +23,6 @@ #include #include -#include - namespace cuopt::mathematical_optimization { // Buffer added to the solver's time_limit to account for worker startup, @@ -152,7 +151,7 @@ std::unique_ptr> solve_mip_remote( auto mip_callbacks = settings.get_mip_callbacks(); const auto var_types = cpu_problem.get_variable_types_host(); const bool has_sc_variables = - thrust::count(var_types.begin(), var_types.end(), var_t::SEMI_CONTINUOUS) > 0; + std::count(var_types.begin(), var_types.end(), var_t::SEMI_CONTINUOUS) > 0; if (has_sc_variables && !mip_callbacks.empty()) { CUOPT_LOG_WARN( "Disabling remote MIP get/set callbacks: semi-continuous models are not " diff --git a/cpp/src/math_optimization/CMakeLists.txt b/cpp/src/math_optimization/CMakeLists.txt index efa1600c54..e6be4079d7 100644 --- a/cpp/src/math_optimization/CMakeLists.txt +++ b/cpp/src/math_optimization/CMakeLists.txt @@ -5,7 +5,8 @@ list(PREPEND MATH_OPT_SRC_FILES - ${CMAKE_CURRENT_SOURCE_DIR}/solver_settings.cu + ${CMAKE_CURRENT_SOURCE_DIR}/solver_settings.cpp + ${CMAKE_CURRENT_SOURCE_DIR}/solver_settings_gpu.cu ${CMAKE_CURRENT_SOURCE_DIR}/solution_reader.cu ${CMAKE_CURRENT_SOURCE_DIR}/solution_writer.cu ${CMAKE_CURRENT_SOURCE_DIR}/tic_toc.cpp diff --git a/cpp/src/math_optimization/solver_settings.cu b/cpp/src/math_optimization/solver_settings.cpp similarity index 90% rename from cpp/src/math_optimization/solver_settings.cu rename to cpp/src/math_optimization/solver_settings.cpp index b3940df890..34f1124313 100644 --- a/cpp/src/math_optimization/solver_settings.cu +++ b/cpp/src/math_optimization/solver_settings.cpp @@ -413,86 +413,6 @@ std::string solver_settings_t::get_parameter_as_string(const std::stri throw std::invalid_argument("Parameter " + name + " not found"); } -template -void solver_settings_t::set_initial_pdlp_primal_solution(const f_t* solution, - i_t size, - rmm::cuda_stream_view stream) -{ - pdlp_settings.set_initial_primal_solution(solution, size, stream); -} - -template -void solver_settings_t::set_initial_pdlp_dual_solution(const f_t* solution, - i_t size, - rmm::cuda_stream_view stream) -{ - pdlp_settings.set_initial_dual_solution(solution, size, stream); -} - -template -void solver_settings_t::set_pdlp_warm_start_data( - const f_t* current_primal_solution, - const f_t* current_dual_solution, - const f_t* initial_primal_average, - const f_t* initial_dual_average, - const f_t* current_ATY, - const f_t* sum_primal_solutions, - const f_t* sum_dual_solutions, - const f_t* last_restart_duality_gap_primal_solution, - const f_t* last_restart_duality_gap_dual_solution, - i_t primal_size, - i_t dual_size, - f_t initial_primal_weight, - f_t initial_step_size, - i_t total_pdlp_iterations, - i_t total_pdhg_iterations, - f_t last_candidate_kkt_score, - f_t last_restart_kkt_score, - f_t sum_solution_weight, - i_t iterations_since_last_restart) -{ - pdlp_settings.set_pdlp_warm_start_data(current_primal_solution, - current_dual_solution, - initial_primal_average, - initial_dual_average, - current_ATY, - sum_primal_solutions, - sum_dual_solutions, - last_restart_duality_gap_primal_solution, - last_restart_duality_gap_dual_solution, - primal_size, - dual_size, - initial_primal_weight, - initial_step_size, - total_pdlp_iterations, - total_pdhg_iterations, - last_candidate_kkt_score, - last_restart_kkt_score, - sum_solution_weight, - iterations_since_last_restart); -} - -template -const rmm::device_uvector& solver_settings_t::get_initial_pdlp_primal_solution() - const -{ - return pdlp_settings.get_initial_primal_solution(); -} - -template -const rmm::device_uvector& solver_settings_t::get_initial_pdlp_dual_solution() const -{ - return pdlp_settings.get_initial_dual_solution(); -} - -template -void solver_settings_t::add_initial_mip_solution(const f_t* solution, - i_t size, - rmm::cuda_stream_view stream) -{ - mip_settings.add_initial_solution(solution, size, stream); -} - template void solver_settings_t::set_mip_callback(internals::base_solution_callback_t* callback, void* user_data) diff --git a/cpp/src/math_optimization/solver_settings_gpu.cu b/cpp/src/math_optimization/solver_settings_gpu.cu new file mode 100644 index 0000000000..a23fbf104a --- /dev/null +++ b/cpp/src/math_optimization/solver_settings_gpu.cu @@ -0,0 +1,181 @@ +/* clang-format off */ +/* + * SPDX-FileCopyrightText: Copyright (c) 2024-2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved. + * SPDX-License-Identifier: Apache-2.0 + */ +/* clang-format on */ + +// Device-facing members of solver_settings_t, split out of solver_settings.cu. +// +// Everything else in that class is host-only parameter handling, so the remainder now +// builds as solver_settings.cpp into the CUDA-free cuopt_client library. Only these +// members take an rmm::cuda_stream_view or hand back a device_uvector, so they are the +// only ones that must stay in a CUDA TU inside libcuopt. +// +// The `template class` instantiation in solver_settings.cpp cannot emit these members +// (their definitions are not visible there), so they are instantiated explicitly below. + +#include + +#include +#include + +#include + +namespace cuopt { +namespace CUOPT_EXPORT mathematical_optimization { + +template +void solver_settings_t::set_initial_pdlp_primal_solution(const f_t* solution, + i_t size, + rmm::cuda_stream_view stream) +{ + pdlp_settings.set_initial_primal_solution(solution, size, stream); +} + +template +void solver_settings_t::set_initial_pdlp_dual_solution(const f_t* solution, + i_t size, + rmm::cuda_stream_view stream) +{ + pdlp_settings.set_initial_dual_solution(solution, size, stream); +} + +template +void solver_settings_t::set_pdlp_warm_start_data( + const f_t* current_primal_solution, + const f_t* current_dual_solution, + const f_t* initial_primal_average, + const f_t* initial_dual_average, + const f_t* current_ATY, + const f_t* sum_primal_solutions, + const f_t* sum_dual_solutions, + const f_t* last_restart_duality_gap_primal_solution, + const f_t* last_restart_duality_gap_dual_solution, + i_t primal_size, + i_t dual_size, + f_t initial_primal_weight, + f_t initial_step_size, + i_t total_pdlp_iterations, + i_t total_pdhg_iterations, + f_t last_candidate_kkt_score, + f_t last_restart_kkt_score, + f_t sum_solution_weight, + i_t iterations_since_last_restart) +{ + pdlp_settings.set_pdlp_warm_start_data(current_primal_solution, + current_dual_solution, + initial_primal_average, + initial_dual_average, + current_ATY, + sum_primal_solutions, + sum_dual_solutions, + last_restart_duality_gap_primal_solution, + last_restart_duality_gap_dual_solution, + primal_size, + dual_size, + initial_primal_weight, + initial_step_size, + total_pdlp_iterations, + total_pdhg_iterations, + last_candidate_kkt_score, + last_restart_kkt_score, + sum_solution_weight, + iterations_since_last_restart); +} + +template +const rmm::device_uvector& solver_settings_t::get_initial_pdlp_primal_solution() + const +{ + return pdlp_settings.get_initial_primal_solution(); +} + +template +const rmm::device_uvector& solver_settings_t::get_initial_pdlp_dual_solution() const +{ + return pdlp_settings.get_initial_dual_solution(); +} + +template +void solver_settings_t::add_initial_mip_solution(const f_t* solution, + i_t size, + rmm::cuda_stream_view stream) +{ + mip_settings.add_initial_solution(solution, size, stream); +} + +#if MIP_INSTANTIATE_FLOAT +template CUOPT_EXPORT void solver_settings_t::set_initial_pdlp_primal_solution( + const float*, int, rmm::cuda_stream_view); +template CUOPT_EXPORT void solver_settings_t::set_initial_pdlp_dual_solution( + const float*, int, rmm::cuda_stream_view); +template CUOPT_EXPORT const rmm::device_uvector& +solver_settings_t::get_initial_pdlp_primal_solution() const; +template CUOPT_EXPORT const rmm::device_uvector& +solver_settings_t::get_initial_pdlp_dual_solution() const; +template CUOPT_EXPORT void solver_settings_t::add_initial_mip_solution( + const float*, int, rmm::cuda_stream_view); +// The 19-argument host overload. It was moved into this TU with the rest of the block, but +// `template class` in solver_settings.cpp cannot emit it (definition not visible there), so +// without this line the symbol disappears -- and it is the one the Cython layer binds to, +// which takes down every Python test, docs-build and wheel-test job. +template CUOPT_EXPORT void solver_settings_t::set_pdlp_warm_start_data(const float*, + const float*, + const float*, + const float*, + const float*, + const float*, + const float*, + const float*, + const float*, + int, + int, + float, + float, + int, + int, + float, + float, + float, + int); +#endif + +#if MIP_INSTANTIATE_DOUBLE +template CUOPT_EXPORT void solver_settings_t::set_initial_pdlp_primal_solution( + const double*, int, rmm::cuda_stream_view); +template CUOPT_EXPORT void solver_settings_t::set_initial_pdlp_dual_solution( + const double*, int, rmm::cuda_stream_view); +template CUOPT_EXPORT const rmm::device_uvector& +solver_settings_t::get_initial_pdlp_primal_solution() const; +template CUOPT_EXPORT const rmm::device_uvector& +solver_settings_t::get_initial_pdlp_dual_solution() const; +template CUOPT_EXPORT void solver_settings_t::add_initial_mip_solution( + const double*, int, rmm::cuda_stream_view); +// The 19-argument host overload. It was moved into this TU with the rest of the block, but +// `template class` in solver_settings.cpp cannot emit it (definition not visible there), so +// without this line the symbol disappears -- and it is the one the Cython layer binds to, +// which takes down every Python test, docs-build and wheel-test job. +template CUOPT_EXPORT void solver_settings_t::set_pdlp_warm_start_data(const double*, + const double*, + const double*, + const double*, + const double*, + const double*, + const double*, + const double*, + const double*, + int, + int, + double, + double, + int, + int, + double, + double, + double, + int); +#endif + +} // namespace CUOPT_EXPORT mathematical_optimization +} // namespace cuopt diff --git a/cpp/src/mip_heuristics/CMakeLists.txt b/cpp/src/mip_heuristics/CMakeLists.txt index 5fa939058c..f56c558a90 100644 --- a/cpp/src/mip_heuristics/CMakeLists.txt +++ b/cpp/src/mip_heuristics/CMakeLists.txt @@ -9,6 +9,7 @@ set(MIP_LP_NECESSARY_FILES ${CMAKE_CURRENT_SOURCE_DIR}/problem/problem.cu ${CMAKE_CURRENT_SOURCE_DIR}/problem/presolve_data.cu ${CMAKE_CURRENT_SOURCE_DIR}/solver_settings.cu + ${CMAKE_CURRENT_SOURCE_DIR}/solver_settings.cpp ${CMAKE_CURRENT_SOURCE_DIR}/solver_solution.cu ${CMAKE_CURRENT_SOURCE_DIR}/local_search/rounding/simple_rounding.cu ${CMAKE_CURRENT_SOURCE_DIR}/presolve/third_party_presolve.cpp diff --git a/cpp/src/mip_heuristics/solver_settings.cpp b/cpp/src/mip_heuristics/solver_settings.cpp new file mode 100644 index 0000000000..564248c472 --- /dev/null +++ b/cpp/src/mip_heuristics/solver_settings.cpp @@ -0,0 +1,67 @@ +/* clang-format off */ +/* + * SPDX-FileCopyrightText: Copyright (c) 2023-2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved. + * SPDX-License-Identifier: Apache-2.0 + */ +/* clang-format on */ + +// Host-only members of mip_solver_settings_t, split out of solver_settings.cu. +// +// Only add_initial_solution() touches the device (it copies into an rmm::device_uvector), +// so it stays in the CUDA TU while these build into the CUDA-free cuopt_client library. +// The gRPC client reaches get_mip_callbacks() via solve_remote's callback handling. +// +// Instantiated per-member rather than with `template class`: the class holds +// device_uvector-backed initial_solutions, so instantiating all of it here would pull +// device code into the client library. + +#include +#include +#include + +#include + +namespace cuopt::mathematical_optimization { + +template +void mip_solver_settings_t::set_mip_callback( + internals::base_solution_callback_t* callback, void* user_data) +{ + if (callback == nullptr) { return; } + callback->set_user_data(user_data); + mip_callbacks_.push_back(callback); +} + +template +const std::vector +mip_solver_settings_t::get_mip_callbacks() const +{ + return mip_callbacks_; +} + +template +typename mip_solver_settings_t::tolerances_t +mip_solver_settings_t::get_tolerances() const noexcept +{ + return tolerances; +} + +#if MIP_INSTANTIATE_FLOAT +template CUOPT_EXPORT void mip_solver_settings_t::set_mip_callback( + internals::base_solution_callback_t*, void*); +template CUOPT_EXPORT const std::vector +mip_solver_settings_t::get_mip_callbacks() const; +template CUOPT_EXPORT mip_solver_settings_t::tolerances_t +mip_solver_settings_t::get_tolerances() const noexcept; +#endif + +#if MIP_INSTANTIATE_DOUBLE +template CUOPT_EXPORT void mip_solver_settings_t::set_mip_callback( + internals::base_solution_callback_t*, void*); +template CUOPT_EXPORT const std::vector +mip_solver_settings_t::get_mip_callbacks() const; +template CUOPT_EXPORT mip_solver_settings_t::tolerances_t +mip_solver_settings_t::get_tolerances() const noexcept; +#endif + +} // namespace cuopt::mathematical_optimization diff --git a/cpp/src/mip_heuristics/solver_settings.cu b/cpp/src/mip_heuristics/solver_settings.cu index 8b454c949b..a5325137bf 100644 --- a/cpp/src/mip_heuristics/solver_settings.cu +++ b/cpp/src/mip_heuristics/solver_settings.cu @@ -24,29 +24,6 @@ void mip_solver_settings_t::add_initial_solution(const f_t* initial_so raft::copy(initial_solutions.back()->data(), initial_solution, size, stream); } -template -void mip_solver_settings_t::set_mip_callback( - internals::base_solution_callback_t* callback, void* user_data) -{ - if (callback == nullptr) { return; } - callback->set_user_data(user_data); - mip_callbacks_.push_back(callback); -} - -template -const std::vector -mip_solver_settings_t::get_mip_callbacks() const -{ - return mip_callbacks_; -} - -template -typename mip_solver_settings_t::tolerances_t -mip_solver_settings_t::get_tolerances() const noexcept -{ - return tolerances; -} - // Explicit template instantiations for common types #if MIP_INSTANTIATE_FLOAT template class CUOPT_EXPORT mip_solver_settings_t; diff --git a/cpp/src/pdlp/CMakeLists.txt b/cpp/src/pdlp/CMakeLists.txt index 2f90f94872..44dced14bc 100644 --- a/cpp/src/pdlp/CMakeLists.txt +++ b/cpp/src/pdlp/CMakeLists.txt @@ -15,6 +15,7 @@ set(LP_CORE_FILES ${CMAKE_CURRENT_SOURCE_DIR}/pdhg.cu ${CMAKE_CURRENT_SOURCE_DIR}/solver_solution.cu ${CMAKE_CURRENT_SOURCE_DIR}/solution_conversion.cu + ${CMAKE_CURRENT_SOURCE_DIR}/solution_conversion_cpu.cpp ${CMAKE_CURRENT_SOURCE_DIR}/saddle_point.cu ${CMAKE_CURRENT_SOURCE_DIR}/cusparse_view.cu ${CMAKE_CURRENT_SOURCE_DIR}/pdlp_warm_start_data.cu diff --git a/cpp/src/pdlp/solution_conversion.cu b/cpp/src/pdlp/solution_conversion.cu index 8293629e6f..533f5ccc3a 100644 --- a/cpp/src/pdlp/solution_conversion.cu +++ b/cpp/src/pdlp/solution_conversion.cu @@ -132,95 +132,9 @@ cuopt::cython::mip_ret_t gpu_mip_solution_t::to_mip_ret_t() } // =========================== -// CPU LP Solution Conversion -// =========================== - -template -cuopt::cython::linear_programming_ret_t -cpu_lp_solution_t::to_cpu_linear_programming_ret_t() -{ - using cpu_solutions_t = cuopt::cython::linear_programming_ret_t::cpu_solutions_t; - cuopt::cython::linear_programming_ret_t ret; - - cpu_solutions_t cpu; - cpu.primal_solution_ = std::move(primal_solution_); - cpu.dual_solution_ = std::move(dual_solution_); - cpu.reduced_cost_ = std::move(reduced_cost_); - - if (!pdlp_warm_start_data_.current_primal_solution_.empty()) { - cpu.current_primal_solution_ = std::move(pdlp_warm_start_data_.current_primal_solution_); - cpu.current_dual_solution_ = std::move(pdlp_warm_start_data_.current_dual_solution_); - cpu.initial_primal_average_ = std::move(pdlp_warm_start_data_.initial_primal_average_); - cpu.initial_dual_average_ = std::move(pdlp_warm_start_data_.initial_dual_average_); - cpu.current_ATY_ = std::move(pdlp_warm_start_data_.current_ATY_); - cpu.sum_primal_solutions_ = std::move(pdlp_warm_start_data_.sum_primal_solutions_); - cpu.sum_dual_solutions_ = std::move(pdlp_warm_start_data_.sum_dual_solutions_); - cpu.last_restart_duality_gap_primal_solution_ = - std::move(pdlp_warm_start_data_.last_restart_duality_gap_primal_solution_); - cpu.last_restart_duality_gap_dual_solution_ = - std::move(pdlp_warm_start_data_.last_restart_duality_gap_dual_solution_); - - ret.initial_primal_weight_ = pdlp_warm_start_data_.initial_primal_weight_; - ret.initial_step_size_ = pdlp_warm_start_data_.initial_step_size_; - ret.total_pdlp_iterations_ = pdlp_warm_start_data_.total_pdlp_iterations_; - ret.total_pdhg_iterations_ = pdlp_warm_start_data_.total_pdhg_iterations_; - ret.last_candidate_kkt_score_ = pdlp_warm_start_data_.last_candidate_kkt_score_; - ret.last_restart_kkt_score_ = pdlp_warm_start_data_.last_restart_kkt_score_; - ret.sum_solution_weight_ = pdlp_warm_start_data_.sum_solution_weight_; - ret.iterations_since_last_restart_ = pdlp_warm_start_data_.iterations_since_last_restart_; - } - - ret.solutions_ = std::move(cpu); - - ret.termination_status_ = termination_status_; - ret.error_status_ = error_status_.get_error_type(); - ret.error_message_ = std::string(error_status_.what()); - ret.l2_primal_residual_ = l2_primal_residual_; - ret.l2_dual_residual_ = l2_dual_residual_; - ret.primal_objective_ = primal_objective_; - ret.dual_objective_ = dual_objective_; - ret.gap_ = gap_; - ret.nb_iterations_ = num_iterations_; - ret.solve_time_ = solve_time_; - ret.solved_by_ = solved_by_; - - return ret; -} - -// =========================== -// CPU MIP Solution Conversion -// =========================== - -template -cuopt::cython::mip_ret_t cpu_mip_solution_t::to_cpu_mip_ret_t() -{ - cuopt::cython::mip_ret_t ret; - - ret.solution_ = std::move(solution_); - - ret.termination_status_ = termination_status_; - ret.error_status_ = error_status_.get_error_type(); - ret.error_message_ = std::string(error_status_.what()); - ret.objective_ = objective_; - ret.mip_gap_ = mip_gap_; - ret.solution_bound_ = solution_bound_; - ret.total_solve_time_ = total_solve_time_; - ret.presolve_time_ = presolve_time_; - ret.max_constraint_violation_ = max_constraint_violation_; - ret.max_int_violation_ = max_int_violation_; - ret.max_variable_bound_violation_ = max_variable_bound_violation_; - ret.nodes_ = num_nodes_; - ret.simplex_iterations_ = num_simplex_iterations_; - - return ret; -} - // Explicit template instantiations template CUOPT_EXPORT cuopt::cython::linear_programming_ret_t gpu_lp_solution_t::to_linear_programming_ret_t(); template CUOPT_EXPORT cuopt::cython::mip_ret_t gpu_mip_solution_t::to_mip_ret_t(); -template CUOPT_EXPORT cuopt::cython::linear_programming_ret_t -cpu_lp_solution_t::to_cpu_linear_programming_ret_t(); -template CUOPT_EXPORT cuopt::cython::mip_ret_t cpu_mip_solution_t::to_cpu_mip_ret_t(); } // namespace cuopt::mathematical_optimization diff --git a/cpp/src/pdlp/solution_conversion_cpu.cpp b/cpp/src/pdlp/solution_conversion_cpu.cpp new file mode 100644 index 0000000000..242df4c824 --- /dev/null +++ b/cpp/src/pdlp/solution_conversion_cpu.cpp @@ -0,0 +1,112 @@ +/* clang-format off */ +/* + * SPDX-FileCopyrightText: Copyright (c) 2025-2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved. + * SPDX-License-Identifier: Apache-2.0 + */ +/* clang-format on */ + +// Host-side solution conversions, split out of solution_conversion.cu. +// +// cpu_lp_solution_t / cpu_mip_solution_t hold std::vector data and simply move it into +// the cython ret structs -- no device memory involved. Keeping them in a .cu TU forced +// the gRPC client to depend on libcuopt.so purely to resolve these two symbols, so they +// live in cuopt_client instead. The GPU counterparts stay in solution_conversion.cu. + +#include +#include +#include + +#include +#include + +namespace cuopt::mathematical_optimization { + +// CPU LP Solution Conversion +// =========================== + +template +cuopt::cython::linear_programming_ret_t +cpu_lp_solution_t::to_cpu_linear_programming_ret_t() +{ + using cpu_solutions_t = cuopt::cython::linear_programming_ret_t::cpu_solutions_t; + cuopt::cython::linear_programming_ret_t ret; + + cpu_solutions_t cpu; + cpu.primal_solution_ = std::move(primal_solution_); + cpu.dual_solution_ = std::move(dual_solution_); + cpu.reduced_cost_ = std::move(reduced_cost_); + + if (!pdlp_warm_start_data_.current_primal_solution_.empty()) { + cpu.current_primal_solution_ = std::move(pdlp_warm_start_data_.current_primal_solution_); + cpu.current_dual_solution_ = std::move(pdlp_warm_start_data_.current_dual_solution_); + cpu.initial_primal_average_ = std::move(pdlp_warm_start_data_.initial_primal_average_); + cpu.initial_dual_average_ = std::move(pdlp_warm_start_data_.initial_dual_average_); + cpu.current_ATY_ = std::move(pdlp_warm_start_data_.current_ATY_); + cpu.sum_primal_solutions_ = std::move(pdlp_warm_start_data_.sum_primal_solutions_); + cpu.sum_dual_solutions_ = std::move(pdlp_warm_start_data_.sum_dual_solutions_); + cpu.last_restart_duality_gap_primal_solution_ = + std::move(pdlp_warm_start_data_.last_restart_duality_gap_primal_solution_); + cpu.last_restart_duality_gap_dual_solution_ = + std::move(pdlp_warm_start_data_.last_restart_duality_gap_dual_solution_); + + ret.initial_primal_weight_ = pdlp_warm_start_data_.initial_primal_weight_; + ret.initial_step_size_ = pdlp_warm_start_data_.initial_step_size_; + ret.total_pdlp_iterations_ = pdlp_warm_start_data_.total_pdlp_iterations_; + ret.total_pdhg_iterations_ = pdlp_warm_start_data_.total_pdhg_iterations_; + ret.last_candidate_kkt_score_ = pdlp_warm_start_data_.last_candidate_kkt_score_; + ret.last_restart_kkt_score_ = pdlp_warm_start_data_.last_restart_kkt_score_; + ret.sum_solution_weight_ = pdlp_warm_start_data_.sum_solution_weight_; + ret.iterations_since_last_restart_ = pdlp_warm_start_data_.iterations_since_last_restart_; + } + + ret.solutions_ = std::move(cpu); + + ret.termination_status_ = termination_status_; + ret.error_status_ = error_status_.get_error_type(); + ret.error_message_ = std::string(error_status_.what()); + ret.l2_primal_residual_ = l2_primal_residual_; + ret.l2_dual_residual_ = l2_dual_residual_; + ret.primal_objective_ = primal_objective_; + ret.dual_objective_ = dual_objective_; + ret.gap_ = gap_; + ret.nb_iterations_ = num_iterations_; + ret.solve_time_ = solve_time_; + ret.solved_by_ = solved_by_; + + return ret; +} + +// =========================== +// CPU MIP Solution Conversion +// =========================== + +template +cuopt::cython::mip_ret_t cpu_mip_solution_t::to_cpu_mip_ret_t() +{ + cuopt::cython::mip_ret_t ret; + + ret.solution_ = std::move(solution_); + + ret.termination_status_ = termination_status_; + ret.error_status_ = error_status_.get_error_type(); + ret.error_message_ = std::string(error_status_.what()); + ret.objective_ = objective_; + ret.mip_gap_ = mip_gap_; + ret.solution_bound_ = solution_bound_; + ret.total_solve_time_ = total_solve_time_; + ret.presolve_time_ = presolve_time_; + ret.max_constraint_violation_ = max_constraint_violation_; + ret.max_int_violation_ = max_int_violation_; + ret.max_variable_bound_violation_ = max_variable_bound_violation_; + ret.nodes_ = num_nodes_; + ret.simplex_iterations_ = num_simplex_iterations_; + + return ret; +} + +// Explicit template instantiations +template CUOPT_EXPORT cuopt::cython::linear_programming_ret_t +cpu_lp_solution_t::to_cpu_linear_programming_ret_t(); +template CUOPT_EXPORT cuopt::cython::mip_ret_t cpu_mip_solution_t::to_cpu_mip_ret_t(); + +} // namespace cuopt::mathematical_optimization From 386b883ba92a4f6ef4bff5052d0379c1c291fb01 Mon Sep 17 00:00:00 2001 From: Ramakrishna Prabhu Date: Tue, 25 Aug 2026 15:26:21 -0500 Subject: [PATCH 2/9] refactor: make to_optimization_problem a free function cpu_optimization_problem_t::to_optimization_problem() was a virtual member on optimization_problem_interface_t. That put it in the vtable of every implementer, including the CPU one -- so cpu_optimization_problem_t's vtable held an entry that only libcuopt can define. Vtable relocations are resolved eagerly at load time, unlike ordinary function calls, so this cannot be deferred or hidden behind lazy binding. Any library carrying that vtable is unloadable without libcuopt.so present. It is now a free function declared in optimization_problem.hpp and defined in cpu_optimization_problem_to_gpu.cpp, dispatching on the concrete type: auto gpu = to_optimization_problem(problem, &handle); The GPU override was a one-line `return nullptr` ("already a GPU problem"), so the dispatch is a single dynamic_cast and the semantics are unchanged -- a GPU-backed problem still yields nullptr. cpu_optimization_problem_t befriends the function to reach its host-side storage. Six call sites updated across pdlp/solve.cu, mip_heuristics/solve.cu, grpc/server/grpc_worker.cpp and solution_interface_test.cu. Splitting the definition into its own translation unit also keeps and the raft handle out of cpu_optimization_problem.cpp, which is otherwise pure host code. Co-Authored-By: Claude Opus 5 Signed-off-by: Ramakrishna Prabhu --- .../cpu_optimization_problem.hpp | 13 +- .../optimization_problem.hpp | 29 +++- .../optimization_problem_interface.hpp | 23 +-- cpp/src/grpc/server/grpc_worker.cpp | 4 +- cpp/src/mip_heuristics/solve.cu | 2 +- cpp/src/pdlp/CMakeLists.txt | 1 + cpp/src/pdlp/cpu_optimization_problem.cpp | 95 ----------- .../pdlp/cpu_optimization_problem_to_gpu.cpp | 159 ++++++++++++++++++ cpp/src/pdlp/optimization_problem.cu | 8 - cpp/src/pdlp/solve.cu | 2 +- .../unit_tests/solution_interface_test.cu | 4 +- 11 files changed, 207 insertions(+), 133 deletions(-) create mode 100644 cpp/src/pdlp/cpu_optimization_problem_to_gpu.cpp diff --git a/cpp/include/cuopt/mathematical_optimization/cpu_optimization_problem.hpp b/cpp/include/cuopt/mathematical_optimization/cpu_optimization_problem.hpp index 28aa91a82f..97d53b2d28 100644 --- a/cpp/include/cuopt/mathematical_optimization/cpu_optimization_problem.hpp +++ b/cpp/include/cuopt/mathematical_optimization/cpu_optimization_problem.hpp @@ -173,9 +173,11 @@ class cpu_optimization_problem_t : public optimization_problem_interface_t> to_optimization_problem( - raft::handle_t const* handle_ptr = nullptr) override; /** * @brief Write the optimization problem to an MPS file. @@ -207,6 +209,13 @@ class cpu_optimization_problem_t : public optimization_problem_interface_t + friend std::unique_ptr> to_optimization_problem( + optimization_problem_interface_t&, raft::handle_t const*); + problem_category_t problem_category_ = problem_category_t::LP; bool maximize_{false}; i_t n_vars_{0}; diff --git a/cpp/include/cuopt/mathematical_optimization/optimization_problem.hpp b/cpp/include/cuopt/mathematical_optimization/optimization_problem.hpp index bdfc2ffbd4..355a317da1 100644 --- a/cpp/include/cuopt/mathematical_optimization/optimization_problem.hpp +++ b/cpp/include/cuopt/mathematical_optimization/optimization_problem.hpp @@ -352,12 +352,8 @@ class optimization_problem_t : public optimization_problem_interface_t template optimization_problem_t convert_to_other_prec(rmm::cuda_stream_view stream) const; - /** - * @brief Returns nullptr since this is already a GPU problem. - * @return nullptr - */ - std::unique_ptr> to_optimization_problem( - raft::handle_t const* handle_ptr = nullptr) override; + // to_optimization_problem() is a free function declared at the bottom of this header, + // not a virtual member -- see the note in optimization_problem_interface.hpp. // ============================================================================ // C API support: Copy to host (polymorphic) @@ -427,5 +423,26 @@ class optimization_problem_t : public optimization_problem_interface_t std::vector row_names_{}; }; +/** + * @brief Convert a problem to a GPU-backed optimization_problem_t. + * + * For optimization_problem_t (GPU): returns nullptr (already is one). + * For cpu_optimization_problem_t: creates a new GPU problem, copies data, returns it. + * + * Usage pattern: + * auto temp = to_optimization_problem(problem_interface, &handle); + * optimization_problem_t& op = temp ? *temp : static_cast(problem); + * + * A free function rather than a virtual member so that cpu_optimization_problem_t's vtable + * carries no GPU-defined entry; see optimization_problem_interface.hpp. + * + * @param problem The problem to convert. + * @param handle_ptr RAFT handle with CUDA resources. Required for CPU->GPU conversion. + * @return unique_ptr to a new GPU problem, or nullptr if it already is one. + */ +template +std::unique_ptr> to_optimization_problem( + optimization_problem_interface_t& problem, raft::handle_t const* handle_ptr = nullptr); + } // namespace CUOPT_EXPORT mathematical_optimization } // namespace cuopt diff --git a/cpp/include/cuopt/mathematical_optimization/optimization_problem_interface.hpp b/cpp/include/cuopt/mathematical_optimization/optimization_problem_interface.hpp index 5927703f03..51796aa60d 100644 --- a/cpp/include/cuopt/mathematical_optimization/optimization_problem_interface.hpp +++ b/cpp/include/cuopt/mathematical_optimization/optimization_problem_interface.hpp @@ -478,22 +478,13 @@ class optimization_problem_interface_t { // Conversion // ============================================================================ - /** - * @brief Convert to a GPU-backed optimization_problem_t. - * - * For optimization_problem_t (GPU): returns nullptr (already is one). - * For cpu_optimization_problem_t: creates new GPU problem, copies data, returns owned pointer. - * - * Usage pattern: - * auto temp = problem_interface->to_optimization_problem(&handle); - * optimization_problem_t& op = temp ? *temp : static_cast(*this); - * - * @param handle_ptr RAFT handle with CUDA resources for GPU memory allocation. - * Required for CPU->GPU conversion. Ignored for GPU problems. - * @return unique_ptr to new GPU problem, or nullptr if already a GPU problem - */ - virtual std::unique_ptr> to_optimization_problem( - raft::handle_t const* handle_ptr = nullptr) = 0; + // NOTE: CPU -> GPU conversion is deliberately NOT a virtual member here. + // + // As a virtual, it occupied a slot in cpu_optimization_problem_t's vtable, and vtable + // relocations are resolved eagerly at load time. That made every library containing + // the vtable -- including the CUDA-free cuopt_client -- unable to load without + // libcuopt.so present. It is now the free function to_optimization_problem() declared + // in optimization_problem.hpp, which lives in libcuopt where the GPU types do. }; } // namespace cuopt::mathematical_optimization diff --git a/cpp/src/grpc/server/grpc_worker.cpp b/cpp/src/grpc/server/grpc_worker.cpp index 250b640031..aa048f34bb 100644 --- a/cpp/src/grpc/server/grpc_worker.cpp +++ b/cpp/src/grpc/server/grpc_worker.cpp @@ -425,7 +425,7 @@ static SolveResult run_mip_solve(DeserializedJob& dj, } SERVER_LOG_INFO("[Worker] Converting CPU problem to GPU problem..."); - auto gpu_problem = dj.problem.to_optimization_problem(&handle); + auto gpu_problem = to_optimization_problem(dj.problem, &handle); SERVER_LOG_INFO("[Worker] Calling solve_mip..."); auto gpu_solution = cuopt::mathematical_optimization::solve_mip(*gpu_problem, dj.mip_settings); @@ -486,7 +486,7 @@ static SolveResult run_lp_solve(DeserializedJob& dj, dj.lp_settings.log_to_console = config.log_to_console; SERVER_LOG_INFO("[Worker] Converting CPU problem to GPU problem..."); - auto gpu_problem = dj.problem.to_optimization_problem(&handle); + auto gpu_problem = to_optimization_problem(dj.problem, &handle); SERVER_LOG_INFO("[Worker] Calling solve_lp..."); auto gpu_solution = cuopt::mathematical_optimization::solve_lp(*gpu_problem, dj.lp_settings); diff --git a/cpp/src/mip_heuristics/solve.cu b/cpp/src/mip_heuristics/solve.cu index 162a5ba291..9b53f2a06f 100644 --- a/cpp/src/mip_heuristics/solve.cu +++ b/cpp/src/mip_heuristics/solve.cu @@ -894,7 +894,7 @@ std::unique_ptr> solve_mip( raft::handle_t handle(stream); // Convert CPU problem to GPU problem - auto gpu_problem = cpu_problem.to_optimization_problem(&handle); + auto gpu_problem = to_optimization_problem(cpu_problem, &handle); // Synchronize before solving to ensure conversion is complete stream.synchronize(); diff --git a/cpp/src/pdlp/CMakeLists.txt b/cpp/src/pdlp/CMakeLists.txt index 44dced14bc..b6a1f8a46d 100644 --- a/cpp/src/pdlp/CMakeLists.txt +++ b/cpp/src/pdlp/CMakeLists.txt @@ -8,6 +8,7 @@ set(LP_CORE_FILES ${CMAKE_CURRENT_SOURCE_DIR}/solver_settings.cu ${CMAKE_CURRENT_SOURCE_DIR}/optimization_problem.cu ${CMAKE_CURRENT_SOURCE_DIR}/cpu_optimization_problem.cpp + ${CMAKE_CURRENT_SOURCE_DIR}/cpu_optimization_problem_to_gpu.cpp ${CMAKE_CURRENT_SOURCE_DIR}/backend_selection.cpp ${CMAKE_CURRENT_SOURCE_DIR}/utilities/problem_checking.cu ${CMAKE_CURRENT_SOURCE_DIR}/solve.cu diff --git a/cpp/src/pdlp/cpu_optimization_problem.cpp b/cpp/src/pdlp/cpu_optimization_problem.cpp index 4b970eb6ec..8e310b287b 100644 --- a/cpp/src/pdlp/cpu_optimization_problem.cpp +++ b/cpp/src/pdlp/cpu_optimization_problem.cpp @@ -10,7 +10,6 @@ #include #include #include -#include #include #include @@ -634,100 +633,6 @@ std::vector cpu_optimization_problem_t::get_variable_types_host return variable_types_; } -// ============================================================================== -// Conversion to optimization_problem_t -// ============================================================================== - -template -std::unique_ptr> -cpu_optimization_problem_t::to_optimization_problem(raft::handle_t const* handle_ptr) -{ - if (handle_ptr == nullptr) { - throw std::runtime_error( - "cpu_optimization_problem_t::to_optimization_problem(): " - "handle_ptr is null. A RAFT handle with CUDA resources is required to convert " - "a CPU-backed problem to a GPU-backed optimization_problem_t."); - } - - auto gpu_problem = std::make_unique>(handle_ptr); - - // Set scalar values - gpu_problem->set_maximize(maximize_); - gpu_problem->set_objective_scaling_factor(objective_scaling_factor_); - gpu_problem->set_objective_offset(objective_offset_); - gpu_problem->set_problem_category(problem_category_); - - // Set string values - if (!objective_name_.empty()) gpu_problem->set_objective_name(objective_name_); - if (!problem_name_.empty()) gpu_problem->set_problem_name(problem_name_); - if (!var_names_.empty()) gpu_problem->set_variable_names(var_names_); - if (!row_names_.empty()) gpu_problem->set_row_names(row_names_); - - // Set CSR constraint matrix (data will be copied to GPU by optimization_problem_t setters) - // Use A_offsets_ presence as the guard: a valid CSR can have zero non-zeros but still - // needs row offsets to define the number of constraints. - if (!A_offsets_.empty()) { - gpu_problem->set_csr_constraint_matrix(A_.data(), - A_.size(), - A_indices_.data(), - A_indices_.size(), - A_offsets_.data(), - A_offsets_.size()); - } - - // Set constraint bounds - if (!b_.empty()) { gpu_problem->set_constraint_bounds(b_.data(), b_.size()); } - - // Set objective coefficients - if (!c_.empty()) { gpu_problem->set_objective_coefficients(c_.data(), c_.size()); } - - // Set quadratic objective if present (GPU setter symmetrizes once: H = Q + Q^T) - if (!Q_values_.empty()) { - gpu_problem->set_quadratic_objective_matrix(Q_values_.data(), - Q_values_.size(), - Q_indices_.data(), - Q_indices_.size(), - Q_offsets_.data(), - Q_offsets_.size()); - } - - if (!quadratic_constraints_.empty()) { - gpu_problem->set_quadratic_constraints( - std::vector::quadratic_constraint_t>( - quadratic_constraints_)); - } - - // Set variable bounds - if (!variable_lower_bounds_.empty()) { - gpu_problem->set_variable_lower_bounds(variable_lower_bounds_.data(), - variable_lower_bounds_.size()); - } - if (!variable_upper_bounds_.empty()) { - gpu_problem->set_variable_upper_bounds(variable_upper_bounds_.data(), - variable_upper_bounds_.size()); - } - - // Set variable types - if (!variable_types_.empty()) { - gpu_problem->set_variable_types(variable_types_.data(), variable_types_.size()); - } - - // Set constraint bounds - if (!constraint_lower_bounds_.empty()) { - gpu_problem->set_constraint_lower_bounds(constraint_lower_bounds_.data(), - constraint_lower_bounds_.size()); - } - if (!constraint_upper_bounds_.empty()) { - gpu_problem->set_constraint_upper_bounds(constraint_upper_bounds_.data(), - constraint_upper_bounds_.size()); - } - - // Set row types - if (!row_types_.empty()) { gpu_problem->set_row_types(row_types_.data(), row_types_.size()); } - - return gpu_problem; -} - // ============================================================================== // File I/O // ============================================================================== diff --git a/cpp/src/pdlp/cpu_optimization_problem_to_gpu.cpp b/cpp/src/pdlp/cpu_optimization_problem_to_gpu.cpp new file mode 100644 index 0000000000..5f31dc9a6e --- /dev/null +++ b/cpp/src/pdlp/cpu_optimization_problem_to_gpu.cpp @@ -0,0 +1,159 @@ +/* clang-format off */ +/* + * SPDX-FileCopyrightText: Copyright (c) 2022-2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved. + * SPDX-License-Identifier: Apache-2.0 + */ +/* clang-format on */ + +// CPU -> GPU conversion for cpu_optimization_problem_t. +// +// Split out of cpu_optimization_problem.cpp so that the rest of that class -- which is +// pure host code -- can be compiled into the CUDA-free cuopt_client library. This is the +// only member that constructs an optimization_problem_t, so it is the only one that needs +// and a raft handle. It stays in cuopt_objs (libcuopt). +// +// The explicit instantiations at the bottom are required: this is a free function, so no +// `template class` anywhere emits it. + +#include +#include +#include +#include + +// Required: the explicit instantiations below are guarded on MIP_INSTANTIATE_*. +// Without this header those macros are undefined and this TU emits no symbols. +#include + +#include +#include +#include + +namespace cuopt::mathematical_optimization { + +// Free function (was a virtual member; see optimization_problem_interface.hpp). +// Dispatches on the concrete type: a GPU problem is already what the caller wants, so it +// yields nullptr, matching the previous optimization_problem_t override. +// +// Unrecognised implementations are rejected rather than yielding nullptr. As a pure virtual +// this was enforced at compile time -- every implementer had to provide an override. A free +// function cannot enforce that, and returning nullptr would be actively unsafe: the +// documented fallback static_casts the original reference to optimization_problem_t&, which +// is undefined behaviour for any other type. +template +std::unique_ptr> to_optimization_problem( + optimization_problem_interface_t& problem, raft::handle_t const* handle_ptr) +{ + auto* cpu_problem = dynamic_cast*>(&problem); + if (cpu_problem == nullptr) { + cuopt_expects(dynamic_cast*>(&problem) != nullptr, + error_type_t::ValidationError, + "to_optimization_problem(): unsupported optimization_problem_interface_t " + "implementation. Only optimization_problem_t and cpu_optimization_problem_t " + "are supported."); + // Already a GPU-backed problem; nothing to convert. + return nullptr; + } + auto& self = *cpu_problem; + + if (handle_ptr == nullptr) { + throw std::runtime_error( + "cpu_optimization_problem_t::to_optimization_problem(): " + "handle_ptr is null. A RAFT handle with CUDA resources is required to convert " + "a CPU-backed problem to a GPU-backed optimization_problem_t."); + } + + auto gpu_problem = std::make_unique>(handle_ptr); + + // Set scalar values + gpu_problem->set_maximize(self.maximize_); + gpu_problem->set_objective_scaling_factor(self.objective_scaling_factor_); + gpu_problem->set_objective_offset(self.objective_offset_); + gpu_problem->set_problem_category(self.problem_category_); + + // Set string values + if (!self.objective_name_.empty()) gpu_problem->set_objective_name(self.objective_name_); + if (!self.problem_name_.empty()) gpu_problem->set_problem_name(self.problem_name_); + if (!self.var_names_.empty()) gpu_problem->set_variable_names(self.var_names_); + if (!self.row_names_.empty()) gpu_problem->set_row_names(self.row_names_); + + // Set CSR constraint matrix (data will be copied to GPU by optimization_problem_t setters) + // Use A_offsets_ presence as the guard: a valid CSR can have zero non-zeros but still + // needs row offsets to define the number of constraints. + if (!self.A_offsets_.empty()) { + gpu_problem->set_csr_constraint_matrix(self.A_.data(), + self.A_.size(), + self.A_indices_.data(), + self.A_indices_.size(), + self.A_offsets_.data(), + self.A_offsets_.size()); + } + + // Set constraint bounds + if (!self.b_.empty()) { gpu_problem->set_constraint_bounds(self.b_.data(), self.b_.size()); } + + // Set objective coefficients + if (!self.c_.empty()) { gpu_problem->set_objective_coefficients(self.c_.data(), self.c_.size()); } + + // Set quadratic objective if present (GPU setter symmetrizes once: H = Q + Q^T) + if (!self.Q_values_.empty()) { + gpu_problem->set_quadratic_objective_matrix(self.Q_values_.data(), + self.Q_values_.size(), + self.Q_indices_.data(), + self.Q_indices_.size(), + self.Q_offsets_.data(), + self.Q_offsets_.size()); + } + + if (!self.quadratic_constraints_.empty()) { + gpu_problem->set_quadratic_constraints( + std::vector::quadratic_constraint_t>( + self.quadratic_constraints_)); + } + + // Set variable bounds + if (!self.variable_lower_bounds_.empty()) { + gpu_problem->set_variable_lower_bounds(self.variable_lower_bounds_.data(), + self.variable_lower_bounds_.size()); + } + if (!self.variable_upper_bounds_.empty()) { + gpu_problem->set_variable_upper_bounds(self.variable_upper_bounds_.data(), + self.variable_upper_bounds_.size()); + } + + // Set variable types + if (!self.variable_types_.empty()) { + gpu_problem->set_variable_types(self.variable_types_.data(), self.variable_types_.size()); + } + + // Set constraint bounds + if (!self.constraint_lower_bounds_.empty()) { + gpu_problem->set_constraint_lower_bounds(self.constraint_lower_bounds_.data(), + self.constraint_lower_bounds_.size()); + } + if (!self.constraint_upper_bounds_.empty()) { + gpu_problem->set_constraint_upper_bounds(self.constraint_upper_bounds_.data(), + self.constraint_upper_bounds_.size()); + } + + // Set row types + if (!self.row_types_.empty()) { + gpu_problem->set_row_types(self.row_types_.data(), self.row_types_.size()); + } + + return gpu_problem; +} + +// ============================================================================== +// Template instantiations matching cpu_optimization_problem.cpp +// ============================================================================== + +#if MIP_INSTANTIATE_FLOAT +template CUOPT_EXPORT std::unique_ptr> +to_optimization_problem(optimization_problem_interface_t&, raft::handle_t const*); +#endif +#if MIP_INSTANTIATE_DOUBLE +template CUOPT_EXPORT std::unique_ptr> +to_optimization_problem(optimization_problem_interface_t&, raft::handle_t const*); +#endif + +} // namespace cuopt::mathematical_optimization diff --git a/cpp/src/pdlp/optimization_problem.cu b/cpp/src/pdlp/optimization_problem.cu index 95457e2556..31f8a315d4 100644 --- a/cpp/src/pdlp/optimization_problem.cu +++ b/cpp/src/pdlp/optimization_problem.cu @@ -639,14 +639,6 @@ raft::handle_t const* optimization_problem_t::get_handle_ptr() const n // Conversion // ============================================================================== -template -std::unique_ptr> -optimization_problem_t::to_optimization_problem(raft::handle_t const* /*handle_ptr*/) -{ - // Already a GPU problem, return nullptr - return nullptr; -} - // ============================================================================== // Host Getters (copy from GPU to CPU) // ============================================================================== diff --git a/cpp/src/pdlp/solve.cu b/cpp/src/pdlp/solve.cu index 5f49efdf0e..4c1b996e49 100644 --- a/cpp/src/pdlp/solve.cu +++ b/cpp/src/pdlp/solve.cu @@ -2681,7 +2681,7 @@ std::unique_ptr> solve_lp( raft::handle_t handle(stream); // Convert CPU problem to GPU problem - auto gpu_problem = cpu_problem.to_optimization_problem(&handle); + auto gpu_problem = to_optimization_problem(cpu_problem, &handle); // Synchronize before solving to ensure conversion is complete stream.synchronize(); diff --git a/cpp/tests/linear_programming/unit_tests/solution_interface_test.cu b/cpp/tests/linear_programming/unit_tests/solution_interface_test.cu index b34faa88b4..85dba0227b 100644 --- a/cpp/tests/linear_programming/unit_tests/solution_interface_test.cu +++ b/cpp/tests/linear_programming/unit_tests/solution_interface_test.cu @@ -305,7 +305,7 @@ TEST_F(SolutionInterfaceTest, gpu_problem_to_optimization_problem) EXPECT_EQ(problem->get_n_constraints(), kNCons); // GPU problem's to_optimization_problem() returns nullptr (already a GPU problem) - auto concrete = problem->to_optimization_problem(&handle); + auto concrete = to_optimization_problem(*problem, &handle); EXPECT_EQ(concrete, nullptr); // Verify the data is still accessible directly on the problem @@ -340,7 +340,7 @@ TEST_F(SolutionInterfaceTest, cpu_problem_to_optimization_problem) EXPECT_EQ(problem->get_n_variables(), kNVars); EXPECT_EQ(problem->get_n_constraints(), kNCons); - auto concrete = problem->to_optimization_problem(&handle); + auto concrete = to_optimization_problem(*problem, &handle); ASSERT_NE(concrete, nullptr); EXPECT_EQ(concrete->get_n_variables(), kNVars); EXPECT_EQ(concrete->get_n_constraints(), kNCons); From 7e5737f4d82d47aedfbc047185f9e2172a683b0f Mon Sep 17 00:00:00 2001 From: Ramakrishna Prabhu Date: Fri, 28 Aug 2026 09:03:23 -0500 Subject: [PATCH 3/9] test: add gtest coverage for the host/device solver-settings and CPU solution-conversion split Addresses the three CodeRabbit review comments on #1801 that had no C++ regression coverage: the semi-continuous callback-disabling predicate in solve_mip_remote() (extracted into should_disable_semi_continuous_callbacks() so it's testable without a live gRPC connection), the solver_settings_t wrapper members moved into solver_settings_gpu.cu (set_initial_pdlp_*, set_pdlp_warm_start_data, add_initial_mip_solution -- previously only reachable through Cython, which is how the missing-instantiation bug in this PR went unnoticed by C++ tests), and the CPU conversion methods in solution_conversion_cpu.cpp (extended to assert every field, including the warm-start-populated branch the prior tests never exercised). Co-Authored-By: Claude Sonnet 5 Signed-off-by: Ramakrishna Prabhu --- cpp/src/grpc/client/grpc_client.hpp | 5 + cpp/src/grpc/client/solve_remote.cpp | 15 +- .../grpc/grpc_client_test.cpp | 40 ++++++ .../unit_tests/solution_interface_test.cu | 89 ++++++++++++ .../unit_tests/solver_settings_test.cu | 128 ++++++++++++++++++ 5 files changed, 274 insertions(+), 3 deletions(-) diff --git a/cpp/src/grpc/client/grpc_client.hpp b/cpp/src/grpc/client/grpc_client.hpp index 87af44c00d..084217acce 100644 --- a/cpp/src/grpc/client/grpc_client.hpp +++ b/cpp/src/grpc/client/grpc_client.hpp @@ -43,6 +43,11 @@ namespace cuopt::mathematical_optimization { void grpc_test_inject_mock_stub(class grpc_client_t& client, std::shared_ptr stub); void grpc_test_mark_as_connected(class grpc_client_t& client); +// Implemented in solve_remote.cpp; declared here so unit tests can exercise the +// semi-continuous callback-disabling predicate without a live gRPC connection. +bool should_disable_semi_continuous_callbacks(const std::vector& var_types, + bool has_callbacks); + /** * @brief Configuration options for the gRPC client * diff --git a/cpp/src/grpc/client/solve_remote.cpp b/cpp/src/grpc/client/solve_remote.cpp index ff808a9fdd..2d8ff05a38 100644 --- a/cpp/src/grpc/client/solve_remote.cpp +++ b/cpp/src/grpc/client/solve_remote.cpp @@ -29,6 +29,17 @@ namespace cuopt::mathematical_optimization { // GPU init, and result pipe transfer. constexpr int kTimeoutBufferSeconds = 120; +// Pulled out of solve_mip_remote() so it can be unit-tested without a live gRPC +// connection: semi-continuous models are not supported together with remote MIP +// get/set callbacks, so callbacks are dropped rather than sent to the server. +bool should_disable_semi_continuous_callbacks(const std::vector& var_types, + bool has_callbacks) +{ + const bool has_sc_variables = + std::count(var_types.begin(), var_types.end(), var_t::SEMI_CONTINUOUS) > 0; + return has_sc_variables && has_callbacks; +} + // ============================================================================ // Helper function to get gRPC server address from environment variables // ============================================================================ @@ -150,9 +161,7 @@ std::unique_ptr> solve_mip_remote( // Check if user has set incumbent callbacks auto mip_callbacks = settings.get_mip_callbacks(); const auto var_types = cpu_problem.get_variable_types_host(); - const bool has_sc_variables = - std::count(var_types.begin(), var_types.end(), var_t::SEMI_CONTINUOUS) > 0; - if (has_sc_variables && !mip_callbacks.empty()) { + if (should_disable_semi_continuous_callbacks(var_types, !mip_callbacks.empty())) { CUOPT_LOG_WARN( "Disabling remote MIP get/set callbacks: semi-continuous models are not " "supported with callbacks"); diff --git a/cpp/tests/linear_programming/grpc/grpc_client_test.cpp b/cpp/tests/linear_programming/grpc/grpc_client_test.cpp index f8fed6eee3..12e5c8a345 100644 --- a/cpp/tests/linear_programming/grpc/grpc_client_test.cpp +++ b/cpp/tests/linear_programming/grpc/grpc_client_test.cpp @@ -2839,3 +2839,43 @@ TEST(MapperRoundtrip, QuadraticConstraintsRowTypeLenient) << "Mismatch at index " << i; } } + +// ============================================================================= +// solve_mip_remote() semi-continuous callback disabling +// ============================================================================= +// +// solve_mip_remote() drops user-provided incumbent get/set callbacks when the model +// has semi-continuous variables, since the remote server does not support that +// combination. should_disable_semi_continuous_callbacks() is the pure predicate behind +// that decision, exposed via grpc_client.hpp so it can be tested without a live +// gRPC connection. + +TEST(SolveMipRemoteCallbacks, NoSemiContinuousNoCallbacksKeepsDisabled) +{ + std::vector var_types = {var_t::CONTINUOUS, var_t::INTEGER}; + EXPECT_FALSE(should_disable_semi_continuous_callbacks(var_types, /*has_callbacks=*/false)); +} + +TEST(SolveMipRemoteCallbacks, NoSemiContinuousWithCallbacksKeepsEnabled) +{ + std::vector var_types = {var_t::CONTINUOUS, var_t::INTEGER}; + EXPECT_FALSE(should_disable_semi_continuous_callbacks(var_types, /*has_callbacks=*/true)); +} + +TEST(SolveMipRemoteCallbacks, SemiContinuousWithoutCallbacksStaysDisabled) +{ + std::vector var_types = {var_t::CONTINUOUS, var_t::SEMI_CONTINUOUS}; + EXPECT_FALSE(should_disable_semi_continuous_callbacks(var_types, /*has_callbacks=*/false)); +} + +TEST(SolveMipRemoteCallbacks, SemiContinuousWithCallbacksGetsDisabled) +{ + std::vector var_types = {var_t::CONTINUOUS, var_t::SEMI_CONTINUOUS, var_t::INTEGER}; + EXPECT_TRUE(should_disable_semi_continuous_callbacks(var_types, /*has_callbacks=*/true)); +} + +TEST(SolveMipRemoteCallbacks, EmptyVariableListKeepsCallbacksEnabled) +{ + std::vector var_types = {}; + EXPECT_FALSE(should_disable_semi_continuous_callbacks(var_types, /*has_callbacks=*/true)); +} diff --git a/cpp/tests/linear_programming/unit_tests/solution_interface_test.cu b/cpp/tests/linear_programming/unit_tests/solution_interface_test.cu index b34faa88b4..7d1fc6b21b 100644 --- a/cpp/tests/linear_programming/unit_tests/solution_interface_test.cu +++ b/cpp/tests/linear_programming/unit_tests/solution_interface_test.cu @@ -548,11 +548,83 @@ TEST_F(SolutionInterfaceTest, lp_solution_to_python_ret) TEST_F(SolutionInterfaceTest, cpu_lp_solution_to_python_ret) { + // Exercises cpu_lp_solution_t::to_cpu_linear_programming_ret_t(), moved into + // pdlp/solution_conversion_cpu.cpp -- assert every field it populates when there + // is no warm-start data. auto cpu_sol = make_cpu_lp_solution(/*with_warmstart=*/false); auto python_ret = cpu_sol->to_python_lp_ret(); EXPECT_FALSE(python_ret.is_gpu()); + ASSERT_TRUE(std::holds_alternative( + python_ret.solutions_)); + const auto& cpu = + std::get(python_ret.solutions_); + EXPECT_EQ(cpu.primal_solution_, (std::vector{1.0, 2.0, 3.0})); + EXPECT_EQ(cpu.dual_solution_, (std::vector{0.5, 0.6})); + EXPECT_EQ(cpu.reduced_cost_, (std::vector{0.1, 0.2, 0.3})); + // No warm-start data was set, so the warm-start buffers must stay empty and the + // warm-start scalars must stay at their default-constructed values. + EXPECT_TRUE(cpu.current_primal_solution_.empty()); + EXPECT_TRUE(cpu.current_dual_solution_.empty()); + EXPECT_TRUE(cpu.initial_primal_average_.empty()); + EXPECT_TRUE(cpu.initial_dual_average_.empty()); + EXPECT_TRUE(cpu.current_ATY_.empty()); + EXPECT_TRUE(cpu.sum_primal_solutions_.empty()); + EXPECT_TRUE(cpu.sum_dual_solutions_.empty()); + EXPECT_TRUE(cpu.last_restart_duality_gap_primal_solution_.empty()); + EXPECT_TRUE(cpu.last_restart_duality_gap_dual_solution_.empty()); + EXPECT_DOUBLE_EQ(python_ret.initial_primal_weight_, 0.0); + EXPECT_DOUBLE_EQ(python_ret.initial_step_size_, 0.0); + EXPECT_EQ(python_ret.total_pdlp_iterations_, 0); + EXPECT_EQ(python_ret.total_pdhg_iterations_, 0); + EXPECT_DOUBLE_EQ(python_ret.last_candidate_kkt_score_, 0.0); + EXPECT_DOUBLE_EQ(python_ret.last_restart_kkt_score_, 0.0); + EXPECT_DOUBLE_EQ(python_ret.sum_solution_weight_, 0.0); + EXPECT_EQ(python_ret.iterations_since_last_restart_, 0); + + EXPECT_EQ(python_ret.termination_status_, pdlp_termination_status_t::Optimal); + EXPECT_EQ(python_ret.error_status_, error_type_t::Success); + EXPECT_NEAR(python_ret.l2_primal_residual_, 1e-8, 1e-15); + EXPECT_NEAR(python_ret.l2_dual_residual_, 2e-8, 1e-15); EXPECT_NEAR(python_ret.primal_objective_, -42.0, 1e-9); + EXPECT_NEAR(python_ret.dual_objective_, -42.5, 1e-9); + EXPECT_NEAR(python_ret.gap_, 0.5, 1e-9); + EXPECT_EQ(python_ret.nb_iterations_, 100); + EXPECT_NEAR(python_ret.solve_time_, 1.23, 1e-9); + EXPECT_EQ(python_ret.solved_by_, method_t::PDLP); +} + +TEST_F(SolutionInterfaceTest, cpu_lp_solution_to_python_ret_with_warmstart) +{ + // Same conversion, exercising the branch that copies pdlp_warm_start_data_ into the + // cpu_solutions_t buffers and the warm-start scalars -- the part of + // to_cpu_linear_programming_ret_t() the previous test cannot reach. + auto cpu_sol = make_cpu_lp_solution(/*with_warmstart=*/true); + auto python_ret = cpu_sol->to_python_lp_ret(); + + EXPECT_FALSE(python_ret.is_gpu()); + const auto& cpu = + std::get(python_ret.solutions_); + EXPECT_EQ(cpu.current_primal_solution_, (std::vector(kNVars, 0.1))); + EXPECT_EQ(cpu.current_dual_solution_, (std::vector(kNCons, 0.2))); + EXPECT_EQ(cpu.initial_primal_average_, (std::vector(kNVars, 0.3))); + EXPECT_EQ(cpu.initial_dual_average_, (std::vector(kNCons, 0.4))); + EXPECT_EQ(cpu.current_ATY_, (std::vector(kNVars, 0.5))); + EXPECT_EQ(cpu.sum_primal_solutions_, (std::vector(kNVars, 0.6))); + EXPECT_EQ(cpu.sum_dual_solutions_, (std::vector(kNCons, 0.7))); + EXPECT_EQ(cpu.last_restart_duality_gap_primal_solution_, (std::vector(kNVars, 0.8))); + EXPECT_EQ(cpu.last_restart_duality_gap_dual_solution_, (std::vector(kNCons, 0.9))); + + EXPECT_DOUBLE_EQ(python_ret.initial_primal_weight_, 1.0); + EXPECT_DOUBLE_EQ(python_ret.initial_step_size_, 0.01); + EXPECT_EQ(python_ret.total_pdlp_iterations_, 100); + EXPECT_EQ(python_ret.total_pdhg_iterations_, 200); + EXPECT_NEAR(python_ret.last_candidate_kkt_score_, 1e-4, 1e-12); + EXPECT_NEAR(python_ret.last_restart_kkt_score_, 1e-5, 1e-12); + EXPECT_DOUBLE_EQ(python_ret.sum_solution_weight_, 50.0); + EXPECT_EQ(python_ret.iterations_since_last_restart_, 10); + + EXPECT_EQ(python_ret.termination_status_, pdlp_termination_status_t::IterationLimit); } TEST_F(SolutionInterfaceTest, mip_solution_to_python_ret) @@ -566,11 +638,28 @@ TEST_F(SolutionInterfaceTest, mip_solution_to_python_ret) TEST_F(SolutionInterfaceTest, cpu_mip_solution_to_python_ret) { + // Exercises cpu_mip_solution_t::to_cpu_mip_ret_t(), moved into + // pdlp/solution_conversion_cpu.cpp -- assert every field it populates. auto cpu_sol = make_cpu_mip_solution(); auto python_ret = cpu_sol->to_python_mip_ret(); EXPECT_FALSE(python_ret.is_gpu()); + ASSERT_TRUE(std::holds_alternative(python_ret.solution_)); + EXPECT_EQ(std::get(python_ret.solution_), + (std::vector{1.0, 0.0, 1.0})); + + EXPECT_EQ(python_ret.termination_status_, mip_termination_status_t::Optimal); + EXPECT_EQ(python_ret.error_status_, error_type_t::Success); EXPECT_NEAR(python_ret.objective_, -99.0, 1e-9); + EXPECT_DOUBLE_EQ(python_ret.mip_gap_, 0.0); + EXPECT_NEAR(python_ret.solution_bound_, -99.0, 1e-9); + EXPECT_NEAR(python_ret.total_solve_time_, 2.34, 1e-9); + EXPECT_NEAR(python_ret.presolve_time_, 0.1, 1e-9); + EXPECT_DOUBLE_EQ(python_ret.max_constraint_violation_, 0.0); + EXPECT_DOUBLE_EQ(python_ret.max_int_violation_, 0.0); + EXPECT_DOUBLE_EQ(python_ret.max_variable_bound_violation_, 0.0); + EXPECT_EQ(python_ret.nodes_, 42); + EXPECT_EQ(python_ret.simplex_iterations_, 500); } // ============================================================================= diff --git a/cpp/tests/linear_programming/unit_tests/solver_settings_test.cu b/cpp/tests/linear_programming/unit_tests/solver_settings_test.cu index d6b5fa0da7..c4480fceb9 100644 --- a/cpp/tests/linear_programming/unit_tests/solver_settings_test.cu +++ b/cpp/tests/linear_programming/unit_tests/solver_settings_test.cu @@ -6,6 +6,7 @@ /* clang-format on */ #include +#include #include #include @@ -282,4 +283,131 @@ TEST(SolverSettingsTest, warm_start_bigger_vector) EXPECT_EQ(h_last_restart_duality_gap_dual_solution, dual_expected); } +// ============================================================================= +// solver_settings_t (the CUDA-free wrapper split across +// math_optimization/solver_settings.cpp and solver_settings_gpu.cu) +// ============================================================================= +// +// These exercise every member that solver_settings_gpu.cu explicitly instantiates. +// A member with a missing explicit instantiation compiles and links this test binary +// fine (cuopt_static resolves it internally), but disappears from libcuopt.so's +// exported symbols -- the failure mode described in the PR that introduced this split. +// See ci/checks or `nm -D --defined-only libcuopt.so` for the linkage-level check. + +TEST(SolverSettingsWrapperTest, InitialPdlpPrimalAndDualSolution) +{ + const raft::handle_t handle_{}; + auto stream = handle_.get_stream(); + + cuopt::mathematical_optimization::solver_settings_t settings{}; + + std::vector primal = {1.0, 2.0, 3.0}; + std::vector dual = {4.0, 5.0}; + + rmm::device_uvector d_primal = cuopt::device_copy(primal, stream); + rmm::device_uvector d_dual = cuopt::device_copy(dual, stream); + + settings.set_initial_pdlp_primal_solution( + d_primal.data(), static_cast(primal.size()), stream); + settings.set_initial_pdlp_dual_solution(d_dual.data(), static_cast(dual.size()), stream); + + EXPECT_EQ(cuopt::host_copy(settings.get_initial_pdlp_primal_solution(), stream), primal); + EXPECT_EQ(cuopt::host_copy(settings.get_initial_pdlp_dual_solution(), stream), dual); +} + +TEST(SolverSettingsWrapperTest, AddInitialMipSolution) +{ + const raft::handle_t handle_{}; + auto stream = handle_.get_stream(); + + cuopt::mathematical_optimization::solver_settings_t settings{}; + + std::vector initial_solution = {1.0, 0.0, 1.0}; + rmm::device_uvector d_solution = cuopt::device_copy(initial_solution, stream); + + settings.add_initial_mip_solution( + d_solution.data(), static_cast(initial_solution.size()), stream); + + ASSERT_EQ(settings.get_mip_settings().initial_solutions.size(), 1u); + EXPECT_EQ(cuopt::host_copy(*settings.get_mip_settings().initial_solutions[0], stream), + initial_solution); +} + +TEST(SolverSettingsWrapperTest, SetPdlpWarmStartDataRawPointers) +{ + cuopt::mathematical_optimization::solver_settings_t settings{}; + + std::vector current_primal_solution = {0.1, 0.2, 0.3}; + std::vector current_dual_solution = {0.4, 0.5}; + std::vector initial_primal_average = {0.6, 0.7, 0.8}; + std::vector initial_dual_average = {0.9, 1.0}; + std::vector current_ATY = {1.1, 1.2, 1.3}; + std::vector sum_primal_solutions = {1.4, 1.5, 1.6}; + std::vector sum_dual_solutions = {1.7, 1.8}; + std::vector last_restart_duality_gap_primal_sol = {1.9, 2.0, 2.1}; + std::vector last_restart_duality_gap_dual_sol = {2.2, 2.3}; + + settings.set_pdlp_warm_start_data( + current_primal_solution.data(), + current_dual_solution.data(), + initial_primal_average.data(), + initial_dual_average.data(), + current_ATY.data(), + sum_primal_solutions.data(), + sum_dual_solutions.data(), + last_restart_duality_gap_primal_sol.data(), + last_restart_duality_gap_dual_sol.data(), + /*primal_size=*/static_cast(current_primal_solution.size()), + /*dual_size=*/static_cast(current_dual_solution.size()), + /*initial_primal_weight=*/1.5, + /*initial_step_size=*/0.01, + /*total_pdlp_iterations=*/10, + /*total_pdhg_iterations=*/20, + /*last_candidate_kkt_score=*/1e-3, + /*last_restart_kkt_score=*/1e-4, + /*sum_solution_weight=*/5.0, + /*iterations_since_last_restart=*/7); + + const auto& view = settings.get_pdlp_warm_start_data_view(); + + auto as_vector = [](auto span) { return std::vector(span.begin(), span.end()); }; + EXPECT_EQ(as_vector(view.current_primal_solution_), current_primal_solution); + EXPECT_EQ(as_vector(view.current_dual_solution_), current_dual_solution); + EXPECT_EQ(as_vector(view.initial_primal_average_), initial_primal_average); + EXPECT_EQ(as_vector(view.initial_dual_average_), initial_dual_average); + EXPECT_EQ(as_vector(view.current_ATY_), current_ATY); + EXPECT_EQ(as_vector(view.sum_primal_solutions_), sum_primal_solutions); + EXPECT_EQ(as_vector(view.sum_dual_solutions_), sum_dual_solutions); + EXPECT_EQ(as_vector(view.last_restart_duality_gap_primal_solution_), + last_restart_duality_gap_primal_sol); + EXPECT_EQ(as_vector(view.last_restart_duality_gap_dual_solution_), + last_restart_duality_gap_dual_sol); + + EXPECT_DOUBLE_EQ(view.initial_primal_weight_, 1.5); + EXPECT_DOUBLE_EQ(view.initial_step_size_, 0.01); + EXPECT_EQ(view.total_pdlp_iterations_, 10); + EXPECT_EQ(view.total_pdhg_iterations_, 20); + EXPECT_DOUBLE_EQ(view.last_candidate_kkt_score_, 1e-3); + EXPECT_DOUBLE_EQ(view.last_restart_kkt_score_, 1e-4); + EXPECT_DOUBLE_EQ(view.sum_solution_weight_, 5.0); + EXPECT_EQ(view.iterations_since_last_restart_, 7); +} + +TEST(SolverSettingsWrapperTest, MipCallbackRegistrationAndTolerances) +{ + cuopt::mathematical_optimization::solver_settings_t settings{}; + + EXPECT_TRUE(settings.get_mip_callbacks().empty()); + + internals::get_solution_callback_t* null_callback = nullptr; + settings.set_mip_callback(null_callback, nullptr); + EXPECT_TRUE(settings.get_mip_callbacks().empty()) << "A null callback must not be registered"; + + // get_tolerances() default-constructs a tolerances_t; verify it round-trips through + // the wrapper -> mip_solver_settings_t split introduced by the host/device separation. + auto tolerances = settings.get_mip_settings().get_tolerances(); + EXPECT_DOUBLE_EQ(tolerances.absolute_tolerance, + mip_solver_settings_t::tolerances_t{}.absolute_tolerance); +} + } // namespace cuopt::mathematical_optimization From e55ebec8282afb973bbacccc79baa34f02cf2676 Mon Sep 17 00:00:00 2001 From: Ramakrishna Prabhu Date: Fri, 28 Aug 2026 09:29:44 -0500 Subject: [PATCH 4/9] test: cover the solver_settings_t GPU-facing instantiations CodeRabbit follow-up on the prior commit: the new SolverSettingsWrapperTest cases only exercised solver_settings_t, leaving the explicit instantiations in solver_settings_gpu.cu (guarded by MIP_INSTANTIATE_FLOAT) with no C++ regression coverage. Co-Authored-By: Claude Sonnet 5 Signed-off-by: Ramakrishna Prabhu --- .../unit_tests/solver_settings_test.cu | 90 +++++++++++++++++++ 1 file changed, 90 insertions(+) diff --git a/cpp/tests/linear_programming/unit_tests/solver_settings_test.cu b/cpp/tests/linear_programming/unit_tests/solver_settings_test.cu index c4480fceb9..3873a9a1bc 100644 --- a/cpp/tests/linear_programming/unit_tests/solver_settings_test.cu +++ b/cpp/tests/linear_programming/unit_tests/solver_settings_test.cu @@ -410,4 +410,94 @@ TEST(SolverSettingsWrapperTest, MipCallbackRegistrationAndTolerances) mip_solver_settings_t::tolerances_t{}.absolute_tolerance); } +// solver_settings_gpu.cu explicitly instantiates these members for both and +// (guarded by MIP_INSTANTIATE_FLOAT / MIP_INSTANTIATE_DOUBLE). The tests above +// only exercise ; repeat the linkage-relevant ones for so a missing +// float instantiation is caught the same way a missing double one would be. +TEST(SolverSettingsWrapperTest, InitialPdlpPrimalAndDualSolutionFloat) +{ + const raft::handle_t handle_{}; + auto stream = handle_.get_stream(); + + cuopt::mathematical_optimization::solver_settings_t settings{}; + + std::vector primal = {1.0f, 2.0f, 3.0f}; + std::vector dual = {4.0f, 5.0f}; + + rmm::device_uvector d_primal = cuopt::device_copy(primal, stream); + rmm::device_uvector d_dual = cuopt::device_copy(dual, stream); + + settings.set_initial_pdlp_primal_solution( + d_primal.data(), static_cast(primal.size()), stream); + settings.set_initial_pdlp_dual_solution(d_dual.data(), static_cast(dual.size()), stream); + + EXPECT_EQ(cuopt::host_copy(settings.get_initial_pdlp_primal_solution(), stream), primal); + EXPECT_EQ(cuopt::host_copy(settings.get_initial_pdlp_dual_solution(), stream), dual); +} + +TEST(SolverSettingsWrapperTest, AddInitialMipSolutionFloat) +{ + const raft::handle_t handle_{}; + auto stream = handle_.get_stream(); + + cuopt::mathematical_optimization::solver_settings_t settings{}; + + std::vector initial_solution = {1.0f, 0.0f, 1.0f}; + rmm::device_uvector d_solution = cuopt::device_copy(initial_solution, stream); + + settings.add_initial_mip_solution( + d_solution.data(), static_cast(initial_solution.size()), stream); + + ASSERT_EQ(settings.get_mip_settings().initial_solutions.size(), 1u); + EXPECT_EQ(cuopt::host_copy(*settings.get_mip_settings().initial_solutions[0], stream), + initial_solution); +} + +TEST(SolverSettingsWrapperTest, SetPdlpWarmStartDataRawPointersFloat) +{ + cuopt::mathematical_optimization::solver_settings_t settings{}; + + std::vector current_primal_solution = {0.1f, 0.2f, 0.3f}; + std::vector current_dual_solution = {0.4f, 0.5f}; + std::vector initial_primal_average = {0.6f, 0.7f, 0.8f}; + std::vector initial_dual_average = {0.9f, 1.0f}; + std::vector current_ATY = {1.1f, 1.2f, 1.3f}; + std::vector sum_primal_solutions = {1.4f, 1.5f, 1.6f}; + std::vector sum_dual_solutions = {1.7f, 1.8f}; + std::vector last_restart_duality_gap_primal_sol = {1.9f, 2.0f, 2.1f}; + std::vector last_restart_duality_gap_dual_sol = {2.2f, 2.3f}; + + settings.set_pdlp_warm_start_data( + current_primal_solution.data(), + current_dual_solution.data(), + initial_primal_average.data(), + initial_dual_average.data(), + current_ATY.data(), + sum_primal_solutions.data(), + sum_dual_solutions.data(), + last_restart_duality_gap_primal_sol.data(), + last_restart_duality_gap_dual_sol.data(), + /*primal_size=*/static_cast(current_primal_solution.size()), + /*dual_size=*/static_cast(current_dual_solution.size()), + /*initial_primal_weight=*/1.5f, + /*initial_step_size=*/0.01f, + /*total_pdlp_iterations=*/10, + /*total_pdhg_iterations=*/20, + /*last_candidate_kkt_score=*/1e-3f, + /*last_restart_kkt_score=*/1e-4f, + /*sum_solution_weight=*/5.0f, + /*iterations_since_last_restart=*/7); + + const auto& view = settings.get_pdlp_warm_start_data_view(); + + auto as_vector = [](auto span) { return std::vector(span.begin(), span.end()); }; + EXPECT_EQ(as_vector(view.current_primal_solution_), current_primal_solution); + EXPECT_EQ(as_vector(view.current_dual_solution_), current_dual_solution); + EXPECT_EQ(as_vector(view.last_restart_duality_gap_dual_solution_), + last_restart_duality_gap_dual_sol); + EXPECT_FLOAT_EQ(view.initial_primal_weight_, 1.5f); + EXPECT_EQ(view.total_pdlp_iterations_, 10); + EXPECT_EQ(view.iterations_since_last_restart_, 7); +} + } // namespace cuopt::mathematical_optimization From c92008a108c45e988f233686e551f4ad963c272f Mon Sep 17 00:00:00 2001 From: Ramakrishna Prabhu Date: Fri, 28 Aug 2026 10:42:44 -0500 Subject: [PATCH 5/9] fix: unbreak conda-cpp-build after macro-comma break in solver_settings_test.cu EXPECT_DOUBLE_EQ(tolerances.absolute_tolerance, mip_solver_settings_t::tolerances_t{}.absolute_tolerance) The comma inside is not inside real parentheses, so the preprocessor parses it as a third macro argument -- EXPECT_DOUBLE_EQ only takes two. Same class of gotcha the file already documents for pdlp_solver_mode_t a few lines up. Fixed by hoisting the template instantiation to a local before the macro call, all 4 conda-cpp-build matrix jobs failed on this in CI. Co-Authored-By: Claude Sonnet 5 Signed-off-by: Ramakrishna Prabhu --- .../linear_programming/unit_tests/solver_settings_test.cu | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/cpp/tests/linear_programming/unit_tests/solver_settings_test.cu b/cpp/tests/linear_programming/unit_tests/solver_settings_test.cu index 3873a9a1bc..3754d9315e 100644 --- a/cpp/tests/linear_programming/unit_tests/solver_settings_test.cu +++ b/cpp/tests/linear_programming/unit_tests/solver_settings_test.cu @@ -406,8 +406,10 @@ TEST(SolverSettingsWrapperTest, MipCallbackRegistrationAndTolerances) // get_tolerances() default-constructs a tolerances_t; verify it round-trips through // the wrapper -> mip_solver_settings_t split introduced by the host/device separation. auto tolerances = settings.get_mip_settings().get_tolerances(); - EXPECT_DOUBLE_EQ(tolerances.absolute_tolerance, - mip_solver_settings_t::tolerances_t{}.absolute_tolerance); + // To avoid the "," inside the macro being interpreted as an extra parameter + using tolerances_t = mip_solver_settings_t::tolerances_t; + double default_absolute_tol = tolerances_t{}.absolute_tolerance; + EXPECT_DOUBLE_EQ(tolerances.absolute_tolerance, default_absolute_tol); } // solver_settings_gpu.cu explicitly instantiates these members for both and From 4cd9ff13683b3d94f278caf3baaba082103cf1d5 Mon Sep 17 00:00:00 2001 From: Ramakrishna Prabhu Date: Fri, 28 Aug 2026 11:20:09 -0500 Subject: [PATCH 6/9] fix: revert solver_settings_t tests, float is never instantiated CI (conda-cpp-build, arm64) failed with undefined references to solver_settings_t::* -- CUOPT_INSTANTIATE_FLOAT is hardcoded to 0 in cpp/include/cuopt/mathematical_optimization/constants.h, so nothing gated by MIP_INSTANTIATE_FLOAT is ever compiled into libcuopt, on any target. CodeRabbit's premise (float is instantiated alongside double) does not hold for this codebase; there is no float coverage to add. Removes the three float-typed SolverSettingsWrapperTest cases added in a prior commit. Co-Authored-By: Claude Sonnet 5 Signed-off-by: Ramakrishna Prabhu --- .../unit_tests/solver_settings_test.cu | 90 ------------------- 1 file changed, 90 deletions(-) diff --git a/cpp/tests/linear_programming/unit_tests/solver_settings_test.cu b/cpp/tests/linear_programming/unit_tests/solver_settings_test.cu index 3754d9315e..3f1edcf06d 100644 --- a/cpp/tests/linear_programming/unit_tests/solver_settings_test.cu +++ b/cpp/tests/linear_programming/unit_tests/solver_settings_test.cu @@ -412,94 +412,4 @@ TEST(SolverSettingsWrapperTest, MipCallbackRegistrationAndTolerances) EXPECT_DOUBLE_EQ(tolerances.absolute_tolerance, default_absolute_tol); } -// solver_settings_gpu.cu explicitly instantiates these members for both and -// (guarded by MIP_INSTANTIATE_FLOAT / MIP_INSTANTIATE_DOUBLE). The tests above -// only exercise ; repeat the linkage-relevant ones for so a missing -// float instantiation is caught the same way a missing double one would be. -TEST(SolverSettingsWrapperTest, InitialPdlpPrimalAndDualSolutionFloat) -{ - const raft::handle_t handle_{}; - auto stream = handle_.get_stream(); - - cuopt::mathematical_optimization::solver_settings_t settings{}; - - std::vector primal = {1.0f, 2.0f, 3.0f}; - std::vector dual = {4.0f, 5.0f}; - - rmm::device_uvector d_primal = cuopt::device_copy(primal, stream); - rmm::device_uvector d_dual = cuopt::device_copy(dual, stream); - - settings.set_initial_pdlp_primal_solution( - d_primal.data(), static_cast(primal.size()), stream); - settings.set_initial_pdlp_dual_solution(d_dual.data(), static_cast(dual.size()), stream); - - EXPECT_EQ(cuopt::host_copy(settings.get_initial_pdlp_primal_solution(), stream), primal); - EXPECT_EQ(cuopt::host_copy(settings.get_initial_pdlp_dual_solution(), stream), dual); -} - -TEST(SolverSettingsWrapperTest, AddInitialMipSolutionFloat) -{ - const raft::handle_t handle_{}; - auto stream = handle_.get_stream(); - - cuopt::mathematical_optimization::solver_settings_t settings{}; - - std::vector initial_solution = {1.0f, 0.0f, 1.0f}; - rmm::device_uvector d_solution = cuopt::device_copy(initial_solution, stream); - - settings.add_initial_mip_solution( - d_solution.data(), static_cast(initial_solution.size()), stream); - - ASSERT_EQ(settings.get_mip_settings().initial_solutions.size(), 1u); - EXPECT_EQ(cuopt::host_copy(*settings.get_mip_settings().initial_solutions[0], stream), - initial_solution); -} - -TEST(SolverSettingsWrapperTest, SetPdlpWarmStartDataRawPointersFloat) -{ - cuopt::mathematical_optimization::solver_settings_t settings{}; - - std::vector current_primal_solution = {0.1f, 0.2f, 0.3f}; - std::vector current_dual_solution = {0.4f, 0.5f}; - std::vector initial_primal_average = {0.6f, 0.7f, 0.8f}; - std::vector initial_dual_average = {0.9f, 1.0f}; - std::vector current_ATY = {1.1f, 1.2f, 1.3f}; - std::vector sum_primal_solutions = {1.4f, 1.5f, 1.6f}; - std::vector sum_dual_solutions = {1.7f, 1.8f}; - std::vector last_restart_duality_gap_primal_sol = {1.9f, 2.0f, 2.1f}; - std::vector last_restart_duality_gap_dual_sol = {2.2f, 2.3f}; - - settings.set_pdlp_warm_start_data( - current_primal_solution.data(), - current_dual_solution.data(), - initial_primal_average.data(), - initial_dual_average.data(), - current_ATY.data(), - sum_primal_solutions.data(), - sum_dual_solutions.data(), - last_restart_duality_gap_primal_sol.data(), - last_restart_duality_gap_dual_sol.data(), - /*primal_size=*/static_cast(current_primal_solution.size()), - /*dual_size=*/static_cast(current_dual_solution.size()), - /*initial_primal_weight=*/1.5f, - /*initial_step_size=*/0.01f, - /*total_pdlp_iterations=*/10, - /*total_pdhg_iterations=*/20, - /*last_candidate_kkt_score=*/1e-3f, - /*last_restart_kkt_score=*/1e-4f, - /*sum_solution_weight=*/5.0f, - /*iterations_since_last_restart=*/7); - - const auto& view = settings.get_pdlp_warm_start_data_view(); - - auto as_vector = [](auto span) { return std::vector(span.begin(), span.end()); }; - EXPECT_EQ(as_vector(view.current_primal_solution_), current_primal_solution); - EXPECT_EQ(as_vector(view.current_dual_solution_), current_dual_solution); - EXPECT_EQ(as_vector(view.last_restart_duality_gap_dual_solution_), - last_restart_duality_gap_dual_sol); - EXPECT_FLOAT_EQ(view.initial_primal_weight_, 1.5f); - EXPECT_EQ(view.total_pdlp_iterations_, 10); - EXPECT_EQ(view.iterations_since_last_restart_, 7); -} - } // namespace cuopt::mathematical_optimization From 020cf6ff581656fb8ae4d8e5789db45756216281 Mon Sep 17 00:00:00 2001 From: Ramakrishna Prabhu Date: Mon, 31 Aug 2026 12:51:40 -0500 Subject: [PATCH 7/9] refactor: move the unsupported-feature predicate out of the client's public header grpc_client.hpp is the gRPC client's public interface, but should_disable_semi_continuous_callbacks is an implementation detail of solve_remote.cpp -- it only appeared there so a unit test could reach it without standing up a live connection. Wrong home. Moved to solve_remote_impl.hpp, mirroring the existing cython_grpc_client_impl.hpp, and renamed to should_disable_unsupported per review: the concern is "is this feature combination something the server cannot honour", not specifically semi-continuous. Documented that semi-continuous + MIP callbacks is currently the only such rule, and that further rules belong inside the predicate rather than as new branches at the call site. No behaviour change. Co-Authored-By: Claude Opus 5 Signed-off-by: Ramakrishna Prabhu --- cpp/src/grpc/client/grpc_client.hpp | 5 --- cpp/src/grpc/client/solve_remote.cpp | 10 +++--- cpp/src/grpc/client/solve_remote_impl.hpp | 33 +++++++++++++++++++ .../grpc/grpc_client_test.cpp | 13 ++++---- 4 files changed, 44 insertions(+), 17 deletions(-) create mode 100644 cpp/src/grpc/client/solve_remote_impl.hpp diff --git a/cpp/src/grpc/client/grpc_client.hpp b/cpp/src/grpc/client/grpc_client.hpp index 084217acce..87af44c00d 100644 --- a/cpp/src/grpc/client/grpc_client.hpp +++ b/cpp/src/grpc/client/grpc_client.hpp @@ -43,11 +43,6 @@ namespace cuopt::mathematical_optimization { void grpc_test_inject_mock_stub(class grpc_client_t& client, std::shared_ptr stub); void grpc_test_mark_as_connected(class grpc_client_t& client); -// Implemented in solve_remote.cpp; declared here so unit tests can exercise the -// semi-continuous callback-disabling predicate without a live gRPC connection. -bool should_disable_semi_continuous_callbacks(const std::vector& var_types, - bool has_callbacks); - /** * @brief Configuration options for the gRPC client * diff --git a/cpp/src/grpc/client/solve_remote.cpp b/cpp/src/grpc/client/solve_remote.cpp index 2d8ff05a38..64f4703bc9 100644 --- a/cpp/src/grpc/client/solve_remote.cpp +++ b/cpp/src/grpc/client/solve_remote.cpp @@ -13,6 +13,7 @@ #include #include #include "grpc_client.hpp" +#include "solve_remote_impl.hpp" #include #include @@ -29,11 +30,8 @@ namespace cuopt::mathematical_optimization { // GPU init, and result pipe transfer. constexpr int kTimeoutBufferSeconds = 120; -// Pulled out of solve_mip_remote() so it can be unit-tested without a live gRPC -// connection: semi-continuous models are not supported together with remote MIP -// get/set callbacks, so callbacks are dropped rather than sent to the server. -bool should_disable_semi_continuous_callbacks(const std::vector& var_types, - bool has_callbacks) +// See solve_remote_impl.hpp for the contract. +bool should_disable_unsupported(const std::vector& var_types, bool has_callbacks) { const bool has_sc_variables = std::count(var_types.begin(), var_types.end(), var_t::SEMI_CONTINUOUS) > 0; @@ -161,7 +159,7 @@ std::unique_ptr> solve_mip_remote( // Check if user has set incumbent callbacks auto mip_callbacks = settings.get_mip_callbacks(); const auto var_types = cpu_problem.get_variable_types_host(); - if (should_disable_semi_continuous_callbacks(var_types, !mip_callbacks.empty())) { + if (should_disable_unsupported(var_types, !mip_callbacks.empty())) { CUOPT_LOG_WARN( "Disabling remote MIP get/set callbacks: semi-continuous models are not " "supported with callbacks"); diff --git a/cpp/src/grpc/client/solve_remote_impl.hpp b/cpp/src/grpc/client/solve_remote_impl.hpp new file mode 100644 index 0000000000..0b29f68c26 --- /dev/null +++ b/cpp/src/grpc/client/solve_remote_impl.hpp @@ -0,0 +1,33 @@ +/* clang-format off */ +/* + * SPDX-FileCopyrightText: Copyright (c) 2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved. + * SPDX-License-Identifier: Apache-2.0 + */ +/* clang-format on */ + +#pragma once + +#include + +#include + +namespace cuopt::mathematical_optimization { + +/** + * @brief Whether a feature combination the remote server cannot honour must be dropped. + * + * Some client-side requests cannot be forwarded to cuopt_grpc_server. Rather than failing + * the solve, solve_mip_remote() drops the unsupported part and warns. This predicate is the + * decision, kept separate from the RPC plumbing so it is unit-testable without a live + * connection. + * + * Currently one rule: MIP get/set callbacks are not supported for semi-continuous models. + * Additional rules belong here as further arguments rather than as new call-site branches. + * + * @param var_types Problem variable types (host). + * @param has_callbacks Whether the caller registered MIP incumbent callbacks. + * @return true when the callbacks must be dropped before submitting. + */ +bool should_disable_unsupported(const std::vector& var_types, bool has_callbacks); + +} // namespace cuopt::mathematical_optimization diff --git a/cpp/tests/linear_programming/grpc/grpc_client_test.cpp b/cpp/tests/linear_programming/grpc/grpc_client_test.cpp index 12e5c8a345..54354cae9e 100644 --- a/cpp/tests/linear_programming/grpc/grpc_client_test.cpp +++ b/cpp/tests/linear_programming/grpc/grpc_client_test.cpp @@ -30,6 +30,7 @@ #include "grpc_settings_mapper.hpp" #include "grpc_solution_mapper.hpp" #include "server/grpc_field_element_size.hpp" +#include "solve_remote_impl.hpp" #include #include @@ -2846,36 +2847,36 @@ TEST(MapperRoundtrip, QuadraticConstraintsRowTypeLenient) // // solve_mip_remote() drops user-provided incumbent get/set callbacks when the model // has semi-continuous variables, since the remote server does not support that -// combination. should_disable_semi_continuous_callbacks() is the pure predicate behind +// combination. should_disable_unsupported() is the pure predicate behind // that decision, exposed via grpc_client.hpp so it can be tested without a live // gRPC connection. TEST(SolveMipRemoteCallbacks, NoSemiContinuousNoCallbacksKeepsDisabled) { std::vector var_types = {var_t::CONTINUOUS, var_t::INTEGER}; - EXPECT_FALSE(should_disable_semi_continuous_callbacks(var_types, /*has_callbacks=*/false)); + EXPECT_FALSE(should_disable_unsupported(var_types, /*has_callbacks=*/false)); } TEST(SolveMipRemoteCallbacks, NoSemiContinuousWithCallbacksKeepsEnabled) { std::vector var_types = {var_t::CONTINUOUS, var_t::INTEGER}; - EXPECT_FALSE(should_disable_semi_continuous_callbacks(var_types, /*has_callbacks=*/true)); + EXPECT_FALSE(should_disable_unsupported(var_types, /*has_callbacks=*/true)); } TEST(SolveMipRemoteCallbacks, SemiContinuousWithoutCallbacksStaysDisabled) { std::vector var_types = {var_t::CONTINUOUS, var_t::SEMI_CONTINUOUS}; - EXPECT_FALSE(should_disable_semi_continuous_callbacks(var_types, /*has_callbacks=*/false)); + EXPECT_FALSE(should_disable_unsupported(var_types, /*has_callbacks=*/false)); } TEST(SolveMipRemoteCallbacks, SemiContinuousWithCallbacksGetsDisabled) { std::vector var_types = {var_t::CONTINUOUS, var_t::SEMI_CONTINUOUS, var_t::INTEGER}; - EXPECT_TRUE(should_disable_semi_continuous_callbacks(var_types, /*has_callbacks=*/true)); + EXPECT_TRUE(should_disable_unsupported(var_types, /*has_callbacks=*/true)); } TEST(SolveMipRemoteCallbacks, EmptyVariableListKeepsCallbacksEnabled) { std::vector var_types = {}; - EXPECT_FALSE(should_disable_semi_continuous_callbacks(var_types, /*has_callbacks=*/true)); + EXPECT_FALSE(should_disable_unsupported(var_types, /*has_callbacks=*/true)); } From 7de9056ce00dd324c3e92032a74a8339acc06bf5 Mon Sep 17 00:00:00 2001 From: Ramakrishna Prabhu Date: Mon, 31 Aug 2026 17:13:25 -0500 Subject: [PATCH 8/9] refactor: pass the problem and settings into should_disable_unsupported Per review, the predicate now takes cpu_optimization_problem_t and mip_solver_settings_t directly instead of a pre-extracted var_types vector and a has_callbacks bool. This is what makes the generalized name honest: a new unsupported-feature rule can consult anything either object exposes without changing the signature, threading another argument through, or adding a branch at the call site. It also moves the get_variable_types_host() copy inside the predicate, so it is skipped entirely when no callbacks are registered -- the common case. The predicate is a template now, so it carries an explicit instantiation. Without one the test's translation unit cannot generate it from the declaration alone, and the symbol goes missing at link time. Tests updated to build real problem/settings objects rather than raw vectors, which also exercises the actual set_mip_callback() path. All 5 cases still pass. Co-Authored-By: Claude Opus 5 Signed-off-by: Ramakrishna Prabhu --- cpp/src/grpc/client/solve_remote.cpp | 20 +++--- cpp/src/grpc/client/solve_remote_impl.hpp | 25 ++++---- .../grpc/grpc_client_test.cpp | 61 ++++++++++++++----- 3 files changed, 72 insertions(+), 34 deletions(-) diff --git a/cpp/src/grpc/client/solve_remote.cpp b/cpp/src/grpc/client/solve_remote.cpp index 64f4703bc9..d6ce8b508d 100644 --- a/cpp/src/grpc/client/solve_remote.cpp +++ b/cpp/src/grpc/client/solve_remote.cpp @@ -31,13 +31,20 @@ namespace cuopt::mathematical_optimization { constexpr int kTimeoutBufferSeconds = 120; // See solve_remote_impl.hpp for the contract. -bool should_disable_unsupported(const std::vector& var_types, bool has_callbacks) +template +bool should_disable_unsupported(const cpu_optimization_problem_t& problem, + const mip_solver_settings_t& settings) { - const bool has_sc_variables = - std::count(var_types.begin(), var_types.end(), var_t::SEMI_CONTINUOUS) > 0; - return has_sc_variables && has_callbacks; + if (settings.get_mip_callbacks().empty()) { return false; } + const auto var_types = problem.get_variable_types_host(); + return std::count(var_types.begin(), var_types.end(), var_t::SEMI_CONTINUOUS) > 0; } +// Instantiated explicitly: the definition lives here, so the unit test's translation unit +// cannot generate it from the declaration alone. +template CUOPT_EXPORT bool should_disable_unsupported( + const cpu_optimization_problem_t&, const mip_solver_settings_t&); + // ============================================================================ // Helper function to get gRPC server address from environment variables // ============================================================================ @@ -157,9 +164,8 @@ std::unique_ptr> solve_mip_remote( } // Check if user has set incumbent callbacks - auto mip_callbacks = settings.get_mip_callbacks(); - const auto var_types = cpu_problem.get_variable_types_host(); - if (should_disable_unsupported(var_types, !mip_callbacks.empty())) { + auto mip_callbacks = settings.get_mip_callbacks(); + if (should_disable_unsupported(cpu_problem, settings)) { CUOPT_LOG_WARN( "Disabling remote MIP get/set callbacks: semi-continuous models are not " "supported with callbacks"); diff --git a/cpp/src/grpc/client/solve_remote_impl.hpp b/cpp/src/grpc/client/solve_remote_impl.hpp index 0b29f68c26..588b3ab526 100644 --- a/cpp/src/grpc/client/solve_remote_impl.hpp +++ b/cpp/src/grpc/client/solve_remote_impl.hpp @@ -7,9 +7,8 @@ #pragma once -#include - -#include +#include +#include namespace cuopt::mathematical_optimization { @@ -17,17 +16,21 @@ namespace cuopt::mathematical_optimization { * @brief Whether a feature combination the remote server cannot honour must be dropped. * * Some client-side requests cannot be forwarded to cuopt_grpc_server. Rather than failing - * the solve, solve_mip_remote() drops the unsupported part and warns. This predicate is the - * decision, kept separate from the RPC plumbing so it is unit-testable without a live - * connection. + * the solve, solve_mip_remote() drops the unsupported part and warns. This predicate is that + * decision, kept out of the RPC plumbing so it is unit-testable without a live connection. + * + * Takes the problem and settings themselves rather than pre-extracted fields, so new rules + * can consult anything either object exposes without changing this signature or adding + * branches at the call site. * * Currently one rule: MIP get/set callbacks are not supported for semi-continuous models. - * Additional rules belong here as further arguments rather than as new call-site branches. * - * @param var_types Problem variable types (host). - * @param has_callbacks Whether the caller registered MIP incumbent callbacks. - * @return true when the callbacks must be dropped before submitting. + * @param problem The problem being submitted. + * @param settings The MIP settings the caller configured. + * @return true when the unsupported request (today, the callbacks) must be dropped. */ -bool should_disable_unsupported(const std::vector& var_types, bool has_callbacks); +template +bool should_disable_unsupported(const cpu_optimization_problem_t& problem, + const mip_solver_settings_t& settings); } // namespace cuopt::mathematical_optimization diff --git a/cpp/tests/linear_programming/grpc/grpc_client_test.cpp b/cpp/tests/linear_programming/grpc/grpc_client_test.cpp index d5462619de..7fead6176e 100644 --- a/cpp/tests/linear_programming/grpc/grpc_client_test.cpp +++ b/cpp/tests/linear_programming/grpc/grpc_client_test.cpp @@ -2844,41 +2844,70 @@ TEST(MapperRoundtrip, QuadraticConstraintsRowTypeLenient) } // ============================================================================= -// solve_mip_remote() semi-continuous callback disabling +// solve_mip_remote() unsupported-feature disabling // ============================================================================= // -// solve_mip_remote() drops user-provided incumbent get/set callbacks when the model -// has semi-continuous variables, since the remote server does not support that -// combination. should_disable_unsupported() is the pure predicate behind -// that decision, exposed via grpc_client.hpp so it can be tested without a live -// gRPC connection. +// solve_mip_remote() drops user-provided incumbent get/set callbacks when the model has +// semi-continuous variables, since the remote server does not support that combination. +// should_disable_unsupported() is the predicate behind that decision, declared in +// solve_remote_impl.hpp so it can be tested without a live gRPC connection. + +namespace { + +// Minimal concrete callback: the predicate only asks whether any callback is registered. +class test_get_callback_t : public cuopt::internals::get_solution_callback_t { + public: + void get_solution(void*, void*, void*, void*) override {} +}; + +cpu_optimization_problem_t make_problem(const std::vector& var_types) +{ + cpu_optimization_problem_t problem; + if (!var_types.empty()) { + problem.set_variable_types(var_types.data(), static_cast(var_types.size())); + } + return problem; +} + +} // namespace TEST(SolveMipRemoteCallbacks, NoSemiContinuousNoCallbacksKeepsDisabled) { - std::vector var_types = {var_t::CONTINUOUS, var_t::INTEGER}; - EXPECT_FALSE(should_disable_unsupported(var_types, /*has_callbacks=*/false)); + auto problem = make_problem({var_t::CONTINUOUS, var_t::INTEGER}); + mip_solver_settings_t settings; + EXPECT_FALSE(should_disable_unsupported(problem, settings)); } TEST(SolveMipRemoteCallbacks, NoSemiContinuousWithCallbacksKeepsEnabled) { - std::vector var_types = {var_t::CONTINUOUS, var_t::INTEGER}; - EXPECT_FALSE(should_disable_unsupported(var_types, /*has_callbacks=*/true)); + auto problem = make_problem({var_t::CONTINUOUS, var_t::INTEGER}); + mip_solver_settings_t settings; + test_get_callback_t callback; + settings.set_mip_callback(&callback, nullptr); + EXPECT_FALSE(should_disable_unsupported(problem, settings)); } TEST(SolveMipRemoteCallbacks, SemiContinuousWithoutCallbacksStaysDisabled) { - std::vector var_types = {var_t::CONTINUOUS, var_t::SEMI_CONTINUOUS}; - EXPECT_FALSE(should_disable_unsupported(var_types, /*has_callbacks=*/false)); + auto problem = make_problem({var_t::CONTINUOUS, var_t::SEMI_CONTINUOUS}); + mip_solver_settings_t settings; + EXPECT_FALSE(should_disable_unsupported(problem, settings)); } TEST(SolveMipRemoteCallbacks, SemiContinuousWithCallbacksGetsDisabled) { - std::vector var_types = {var_t::CONTINUOUS, var_t::SEMI_CONTINUOUS, var_t::INTEGER}; - EXPECT_TRUE(should_disable_unsupported(var_types, /*has_callbacks=*/true)); + auto problem = make_problem({var_t::CONTINUOUS, var_t::SEMI_CONTINUOUS, var_t::INTEGER}); + mip_solver_settings_t settings; + test_get_callback_t callback; + settings.set_mip_callback(&callback, nullptr); + EXPECT_TRUE(should_disable_unsupported(problem, settings)); } TEST(SolveMipRemoteCallbacks, EmptyVariableListKeepsCallbacksEnabled) { - std::vector var_types = {}; - EXPECT_FALSE(should_disable_unsupported(var_types, /*has_callbacks=*/true)); + auto problem = make_problem({}); + mip_solver_settings_t settings; + test_get_callback_t callback; + settings.set_mip_callback(&callback, nullptr); + EXPECT_FALSE(should_disable_unsupported(problem, settings)); } From 63172d01a99333183cc1b1f09f0de4748ddeeac8 Mon Sep 17 00:00:00 2001 From: Ramakrishna Prabhu Date: Wed, 2 Sep 2026 09:53:01 -0500 Subject: [PATCH 9/9] docs: trim redundant comments and drop the orphaned doxygen block The vtable rationale was restated in three places; it now lives once, in optimization_problem_interface.hpp where the virtual was removed, with short pointers elsewhere. Also removes the doxygen block left behind on cpu_optimization_problem.hpp when the declaration was deleted. Two consecutive /** */ blocks preceded write_to_mps(), so doxygen silently discarded the first -- the @param/@return/@throws documentation was already being lost from generated docs. The free function carries those docs. Fixes an inconsistent doc snippet that named the same variable two ways. Co-Authored-By: Claude Opus 5 Signed-off-by: Ramakrishna Prabhu --- .../cpu_optimization_problem.hpp | 19 +++----------- .../optimization_problem.hpp | 5 +--- .../pdlp/cpu_optimization_problem_to_gpu.cpp | 26 ++++++------------- 3 files changed, 12 insertions(+), 38 deletions(-) diff --git a/cpp/include/cuopt/mathematical_optimization/cpu_optimization_problem.hpp b/cpp/include/cuopt/mathematical_optimization/cpu_optimization_problem.hpp index 97d53b2d28..191eac62e8 100644 --- a/cpp/include/cuopt/mathematical_optimization/cpu_optimization_problem.hpp +++ b/cpp/include/cuopt/mathematical_optimization/cpu_optimization_problem.hpp @@ -30,6 +30,7 @@ class mps_data_model_t; // Forward declarations template class optimization_problem_t; + template class pdlp_solver_settings_t; template @@ -166,19 +167,6 @@ class cpu_optimization_problem_t : public optimization_problem_interface_t get_row_types_host() const override; std::vector get_variable_types_host() const override; - /** - * @brief Convert this CPU optimization problem to an optimization_problem_t - * by copying CPU data to GPU (requires GPU memory transfer). - * - * @param handle_ptr RAFT handle with CUDA resources for GPU memory allocation. - * @return unique_ptr to new optimization_problem_t with all data copied to GPU - * @throws std::runtime_error if handle_ptr is null - * - * Provided as the free function to_optimization_problem() in optimization_problem.hpp, - * not as a member: keeping it out of this class's vtable is what lets cuopt_client - * load without libcuopt.so. - */ - /** * @brief Write the optimization problem to an MPS file. * @param[in] mps_file_path Path to the output MPS file @@ -209,9 +197,8 @@ class cpu_optimization_problem_t : public optimization_problem_interface_t friend std::unique_ptr> to_optimization_problem( optimization_problem_interface_t&, raft::handle_t const*); diff --git a/cpp/include/cuopt/mathematical_optimization/optimization_problem.hpp b/cpp/include/cuopt/mathematical_optimization/optimization_problem.hpp index 355a317da1..5363cfe812 100644 --- a/cpp/include/cuopt/mathematical_optimization/optimization_problem.hpp +++ b/cpp/include/cuopt/mathematical_optimization/optimization_problem.hpp @@ -352,9 +352,6 @@ class optimization_problem_t : public optimization_problem_interface_t template optimization_problem_t convert_to_other_prec(rmm::cuda_stream_view stream) const; - // to_optimization_problem() is a free function declared at the bottom of this header, - // not a virtual member -- see the note in optimization_problem_interface.hpp. - // ============================================================================ // C API support: Copy to host (polymorphic) // ============================================================================ @@ -430,7 +427,7 @@ class optimization_problem_t : public optimization_problem_interface_t * For cpu_optimization_problem_t: creates a new GPU problem, copies data, returns it. * * Usage pattern: - * auto temp = to_optimization_problem(problem_interface, &handle); + * auto temp = to_optimization_problem(problem, &handle); * optimization_problem_t& op = temp ? *temp : static_cast(problem); * * A free function rather than a virtual member so that cpu_optimization_problem_t's vtable diff --git a/cpp/src/pdlp/cpu_optimization_problem_to_gpu.cpp b/cpp/src/pdlp/cpu_optimization_problem_to_gpu.cpp index 5f31dc9a6e..da0b305064 100644 --- a/cpp/src/pdlp/cpu_optimization_problem_to_gpu.cpp +++ b/cpp/src/pdlp/cpu_optimization_problem_to_gpu.cpp @@ -7,21 +7,16 @@ // CPU -> GPU conversion for cpu_optimization_problem_t. // -// Split out of cpu_optimization_problem.cpp so that the rest of that class -- which is -// pure host code -- can be compiled into the CUDA-free cuopt_client library. This is the -// only member that constructs an optimization_problem_t, so it is the only one that needs -// and a raft handle. It stays in cuopt_objs (libcuopt). -// -// The explicit instantiations at the bottom are required: this is a free function, so no -// `template class` anywhere emits it. +// Split out of cpu_optimization_problem.cpp: this is the only part of that class needing +// and a raft handle, so keeping it here leaves the rest as pure +// host code. #include #include #include #include -// Required: the explicit instantiations below are guarded on MIP_INSTANTIATE_*. -// Without this header those macros are undefined and this TU emits no symbols. +// Required: without it MIP_INSTANTIATE_* are undefined and this TU emits no symbols. #include #include @@ -30,15 +25,10 @@ namespace cuopt::mathematical_optimization { -// Free function (was a virtual member; see optimization_problem_interface.hpp). -// Dispatches on the concrete type: a GPU problem is already what the caller wants, so it -// yields nullptr, matching the previous optimization_problem_t override. -// -// Unrecognised implementations are rejected rather than yielding nullptr. As a pure virtual -// this was enforced at compile time -- every implementer had to provide an override. A free -// function cannot enforce that, and returning nullptr would be actively unsafe: the -// documented fallback static_casts the original reference to optimization_problem_t&, which -// is undefined behaviour for any other type. +// Dispatches on the concrete type; a GPU-backed problem yields nullptr, matching the +// previous override. Unrecognised implementations throw rather than returning nullptr: +// the documented fallback static_casts the reference to optimization_problem_t&, which +// would be undefined behaviour for any other type. template std::unique_ptr> to_optimization_problem( optimization_problem_interface_t& problem, raft::handle_t const* handle_ptr)