From 0f5ae25068c19f8bdb032c92330f5fee51af5e27 Mon Sep 17 00:00:00 2001 From: Ramakrishna Prabhu Date: Tue, 25 Aug 2026 15:25:52 -0500 Subject: [PATCH 1/7] 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 7e5737f4d82d47aedfbc047185f9e2172a683b0f Mon Sep 17 00:00:00 2001 From: Ramakrishna Prabhu Date: Fri, 28 Aug 2026 09:03:23 -0500 Subject: [PATCH 2/7] 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 3/7] 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 4/7] 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 5/7] 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 6/7] 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 7/7] 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)); }