-
Notifications
You must be signed in to change notification settings - Fork 225
Migrate stream APIs from rmm::cuda_stream_view to cuda::stream_ref #1828
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
adb4f65
4e37290
214d34a
b32e29e
6b416d5
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 |
|---|---|---|
|
|
@@ -10,9 +10,9 @@ | |
| #include <cuopt/export.hpp> | ||
| #include <cuopt/mathematical_optimization/pdlp/pdlp_warm_start_data.hpp> | ||
|
|
||
| #include <cuda/stream> | ||
| #include <raft/core/device_span.hpp> | ||
|
|
||
| #include <rmm/cuda_stream_view.hpp> | ||
| #include <rmm/device_uvector.hpp> | ||
|
|
||
| #include <cuopt/mathematical_optimization/constants.h> | ||
|
|
@@ -52,10 +52,12 @@ class solver_settings_t { | |
|
|
||
| void set_initial_pdlp_primal_solution(const f_t* initial_primal_solution, | ||
| i_t size, | ||
| rmm::cuda_stream_view stream = rmm::cuda_stream_default); | ||
| cuda::stream_ref stream = cuda::stream_ref{ | ||
| cudaStream_t{cudaStreamDefault}}); | ||
| void set_initial_pdlp_dual_solution(const f_t* initial_dual_solution, | ||
| i_t size, | ||
| rmm::cuda_stream_view stream = rmm::cuda_stream_default); | ||
| cuda::stream_ref stream = cuda::stream_ref{ | ||
| cudaStream_t{cudaStreamDefault}}); | ||
|
Comment on lines
+55
to
+60
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 | 🔴 Critical | ⚡ Quick win 🔎 Supported by static analysis🏁 Script executed: #!/bin/bash
set -euo pipefail
rg -n --glob '*.{hpp,cu}' 'cudaStream_t\s*\{\s*cudaStreamDefault\s*\}' \
cpp/include/cuopt/mathematical_optimization/solver_settings.hpp \
cpp/src/pdlp/pdlp_warm_start_data.cuRepository: NVIDIA/cuopt Length of output: 1700 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- solver_settings.hpp ---'
sed -n '1,105p' cpp/include/cuopt/mathematical_optimization/solver_settings.hpp
printf '%s\n' '--- pdlp_warm_start_data.cu ---'
sed -n '1,100p' cpp/src/pdlp/pdlp_warm_start_data.cu
printf '%s\n' '--- bound stream_ref and CUDA stream symbols ---'
rg -n -S --glob '*.{h,hpp,hh,cuh,cpp,cu,cc,cxx}' \
'stream_ref|cudaStream_t|cudaStreamDefault|cudaStreamCreate' \
cpp | head -240Repository: NVIDIA/cuopt Length of output: 36187 🏁 Script executed: #!/bin/bash
set -euo pipefail
cat > /tmp/cuda_stream_initializer_probe.cpp <<'CPP'
struct CUstream_st;
using cudaStream_t = CUstream_st*;
enum cudaStreamFlags {
cudaStreamDefault = 0,
cudaStreamNonBlocking = 1
};
cudaStream_t from_flag{cudaStreamDefault};
cudaStream_t from_value_initialization{};
int main() { return from_flag == from_value_initialization ? 0 : 1; }
CPP
if command -v g++ >/dev/null 2>&1; then
g++ -std=c++17 -Wall -Wextra -pedantic -fsyntax-only \
/tmp/cuda_stream_initializer_probe.cpp
printf '%s\n' 'g++ accepted both initializers'
else
printf '%s\n' 'g++ unavailable'
fiRepository: NVIDIA/cuopt Length of output: 472 Construct the default CUDA stream from a null stream handle.
📍 Affects 2 files
🤖 Prompt for AI Agents |
||
| void set_pdlp_warm_start_data(const f_t* current_primal_solution, | ||
| const f_t* current_dual_solution, | ||
| const f_t* initial_primal_average, | ||
|
|
@@ -82,7 +84,8 @@ class solver_settings_t { | |
| // MIP Settings | ||
| void add_initial_mip_solution(const f_t* initial_solution, | ||
| i_t size, | ||
| rmm::cuda_stream_view stream = rmm::cuda_stream_default); | ||
| cuda::stream_ref stream = cuda::stream_ref{ | ||
| cudaStream_t{cudaStreamDefault}}); | ||
| void set_mip_callback(internals::base_solution_callback_t* callback = nullptr, | ||
| void* user_data = nullptr); | ||
|
|
||
|
|
||
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 | 🟡 Minor | ⚡ Quick win
Correct the public stream contract documentation.
These APIs enqueue allocation and copy work on their
streamparameter. Their documentation instead says that copying uses the RAFT handle stream. It also omits the stream parameter.cpp/include/cuopt/mathematical_optimization/mip/solver_settings.hpp#L94-L95: add@param streamand describe the CUDA default-stream behavior.cpp/include/cuopt/mathematical_optimization/pdlp/solver_settings.hpp#L155-L156: add@param streamand describe the CUDA default-stream behavior.cpp/include/cuopt/mathematical_optimization/pdlp/solver_settings.hpp#L170-L171: add@param streamand describe the CUDA default-stream behavior.As per path instructions, “verify parameter descriptions match actual types/behavior.”
📍 Affects 2 files
cpp/include/cuopt/mathematical_optimization/mip/solver_settings.hpp#L94-L95(this comment)cpp/include/cuopt/mathematical_optimization/pdlp/solver_settings.hpp#L155-L156cpp/include/cuopt/mathematical_optimization/pdlp/solver_settings.hpp#L170-L171🤖 Prompt for AI Agents
Source: Path instructions