-
Notifications
You must be signed in to change notification settings - Fork 222
refactor: split host-only members out of CUDA translation units #1801
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,135 @@ | ||
| /* 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 <cuopt/mathematical_optimization/solver_settings.hpp> | ||
|
|
||
| #include <rmm/cuda_stream_view.hpp> | ||
| #include <rmm/device_uvector.hpp> | ||
|
|
||
| #include <mip_heuristics/mip_constants.hpp> | ||
|
|
||
| namespace cuopt { | ||
| namespace CUOPT_EXPORT mathematical_optimization { | ||
|
|
||
| template <typename i_t, typename f_t> | ||
| void solver_settings_t<i_t, f_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 <typename i_t, typename f_t> | ||
| void solver_settings_t<i_t, f_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 <typename i_t, typename f_t> | ||
| void solver_settings_t<i_t, f_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); | ||
|
Comment on lines
+45
to
+84
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win Instantiate This member is defined only in this CUDA translation unit. Lines 108-132 instantiate the other moved members but omit this one. A caller of Add explicit float and double instantiations in the matching 🤖 Prompt for AI Agents |
||
| } | ||
|
|
||
| template <typename i_t, typename f_t> | ||
| const rmm::device_uvector<f_t>& solver_settings_t<i_t, f_t>::get_initial_pdlp_primal_solution() | ||
| const | ||
| { | ||
| return pdlp_settings.get_initial_primal_solution(); | ||
| } | ||
|
|
||
| template <typename i_t, typename f_t> | ||
| const rmm::device_uvector<f_t>& solver_settings_t<i_t, f_t>::get_initial_pdlp_dual_solution() const | ||
| { | ||
| return pdlp_settings.get_initial_dual_solution(); | ||
| } | ||
|
|
||
| template <typename i_t, typename f_t> | ||
| void solver_settings_t<i_t, f_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); | ||
|
Comment on lines
+28
to
+105
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift Add unit tests for the moved solver-settings members. This split creates new CPU/CUDA linkage boundaries. Add gtest coverage under
📍 Affects 2 files
🤖 Prompt for AI AgentsSource: Coding guidelines |
||
| } | ||
|
|
||
| #if MIP_INSTANTIATE_FLOAT | ||
| template CUOPT_EXPORT void solver_settings_t<int, float>::set_initial_pdlp_primal_solution( | ||
| const float*, int, rmm::cuda_stream_view); | ||
| template CUOPT_EXPORT void solver_settings_t<int, float>::set_initial_pdlp_dual_solution( | ||
| const float*, int, rmm::cuda_stream_view); | ||
| template CUOPT_EXPORT const rmm::device_uvector<float>& | ||
| solver_settings_t<int, float>::get_initial_pdlp_primal_solution() const; | ||
| template CUOPT_EXPORT const rmm::device_uvector<float>& | ||
| solver_settings_t<int, float>::get_initial_pdlp_dual_solution() const; | ||
| template CUOPT_EXPORT void solver_settings_t<int, float>::add_initial_mip_solution( | ||
| const float*, int, rmm::cuda_stream_view); | ||
| #endif | ||
|
|
||
| #if MIP_INSTANTIATE_DOUBLE | ||
| template CUOPT_EXPORT void solver_settings_t<int, double>::set_initial_pdlp_primal_solution( | ||
| const double*, int, rmm::cuda_stream_view); | ||
| template CUOPT_EXPORT void solver_settings_t<int, double>::set_initial_pdlp_dual_solution( | ||
| const double*, int, rmm::cuda_stream_view); | ||
| template CUOPT_EXPORT const rmm::device_uvector<double>& | ||
| solver_settings_t<int, double>::get_initial_pdlp_primal_solution() const; | ||
| template CUOPT_EXPORT const rmm::device_uvector<double>& | ||
| solver_settings_t<int, double>::get_initial_pdlp_dual_solution() const; | ||
| template CUOPT_EXPORT void solver_settings_t<int, double>::add_initial_mip_solution( | ||
| const double*, int, rmm::cuda_stream_view); | ||
| #endif | ||
|
|
||
| } // namespace CUOPT_EXPORT mathematical_optimization | ||
| } // namespace cuopt | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 <cuopt/export.hpp> | ||
| #include <cuopt/mathematical_optimization/mip/solver_settings.hpp> | ||
| #include <mip_heuristics/mip_constants.hpp> | ||
|
|
||
| #include <vector> | ||
|
|
||
| namespace cuopt::mathematical_optimization { | ||
|
|
||
| template <typename i_t, typename f_t> | ||
| void mip_solver_settings_t<i_t, f_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 <typename i_t, typename f_t> | ||
| const std::vector<internals::base_solution_callback_t*> | ||
| mip_solver_settings_t<i_t, f_t>::get_mip_callbacks() const | ||
| { | ||
| return mip_callbacks_; | ||
| } | ||
|
|
||
| template <typename i_t, typename f_t> | ||
| typename mip_solver_settings_t<i_t, f_t>::tolerances_t | ||
| mip_solver_settings_t<i_t, f_t>::get_tolerances() const noexcept | ||
| { | ||
| return tolerances; | ||
| } | ||
|
|
||
| #if MIP_INSTANTIATE_FLOAT | ||
| template CUOPT_EXPORT void mip_solver_settings_t<int, float>::set_mip_callback( | ||
| internals::base_solution_callback_t*, void*); | ||
| template CUOPT_EXPORT const std::vector<internals::base_solution_callback_t*> | ||
| mip_solver_settings_t<int, float>::get_mip_callbacks() const; | ||
| template CUOPT_EXPORT mip_solver_settings_t<int, float>::tolerances_t | ||
| mip_solver_settings_t<int, float>::get_tolerances() const noexcept; | ||
| #endif | ||
|
|
||
| #if MIP_INSTANTIATE_DOUBLE | ||
| template CUOPT_EXPORT void mip_solver_settings_t<int, double>::set_mip_callback( | ||
| internals::base_solution_callback_t*, void*); | ||
| template CUOPT_EXPORT const std::vector<internals::base_solution_callback_t*> | ||
| mip_solver_settings_t<int, double>::get_mip_callbacks() const; | ||
| template CUOPT_EXPORT mip_solver_settings_t<int, double>::tolerances_t | ||
| mip_solver_settings_t<int, double>::get_tolerances() const noexcept; | ||
| #endif | ||
|
|
||
| } // namespace cuopt::mathematical_optimization |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
Add unit coverage for the callback-disabling branch.
Add a gtest under
cpp/src/teststhat covers host variable types both with and withoutvar_t::SEMI_CONTINUOUS, and verifies that callbacks are cleared only for semi-continuous models.As per coding guidelines, “
**/*.{cpp,cc,cxx,h,hpp,cu,cuh}: Add unit tests. Please refer tocpp/src/testsfor examples of unit tests on C and C++ using gtest.”🤖 Prompt for AI Agents
Source: Coding guidelines