Skip to content

add RecordStreamTest.cpp#47

Open
youge325 wants to merge 2 commits intoPFCCLab:masterfrom
youge325:cRecord
Open

add RecordStreamTest.cpp#47
youge325 wants to merge 2 commits intoPFCCLab:masterfrom
youge325:cRecord

Conversation

@youge325
Copy link
Contributor

@youge325 youge325 commented Mar 5, 2026

新增 record_stream 测试

image

Copilot AI review requested due to automatic review settings March 5, 2026 13:35
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a new compatibility test suite for Tensor::record_stream and updates build configuration to include CUDA Toolkit headers when available, improving compilation in CUDA-enabled environments (especially when headers pull in cuda_runtime_api.h).

Changes:

  • Added test/RecordStreamTest.cpp covering multiple record_stream call scenarios (various dtypes/shapes, non-contiguous, CPU tensor cases).
  • Updated CMakeLists.txt to find_package(CUDAToolkit) and append CUDA include directories to Torch/Paddle include paths when found.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
test/RecordStreamTest.cpp Introduces record_stream API coverage and a helper to obtain a CUDA stream across Torch vs Paddle compat builds.
CMakeLists.txt Adds optional CUDA Toolkit discovery and include paths to support CUDA-dependent headers during compilation.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

"${TORCH_DIR}/include/torch/csrc/api/include/")
if(CUDAToolkit_FOUND)
list(APPEND TORCH_INCLUDE_DIR "${CUDAToolkit_INCLUDE_DIRS}")
endif()
Copy link

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

find_package(CUDAToolkit QUIET) + conditional include path append can still leave builds failing later with missing cuda_runtime_api.h (e.g., CUDA-enabled libtorch headers included but CUDA toolkit not found), and the failure will be a compiler error with no actionable CMake message. Consider adding a clear message(FATAL_ERROR ...) (or at least message(WARNING ...)) when CUDA-dependent headers/tests are being built but CUDAToolkit is not found, so the configuration fails early and is easier to debug.

Suggested change
endif()
endif()
if(NOT CUDAToolkit_FOUND)
message(
FATAL_ERROR
"CUDAToolkit was not found, but CUDA-dependent Torch tests are enabled. "
"Please install the CUDA toolkit or disable building these Torch tests.")
endif()

Copilot uses AI. Check for mistakes.
Comment on lines +127 to +129
if(CUDAToolkit_FOUND)
list(APPEND PADDLE_INCLUDE_DIR "${CUDAToolkit_INCLUDE_DIRS}")
endif()
Copy link

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Appending ${CUDAToolkit_INCLUDE_DIRS} to PADDLE_INCLUDE_DIR is guarded on CUDAToolkit_FOUND, but PADDLE_LIBRARIES unconditionally includes GPU libraries (e.g. libphi_gpu.so). If the environment is missing CUDA toolkit headers, the build will fail with confusing compile errors. Consider either (1) making CUDA toolkit a required dependency when linking Paddle GPU libs, or (2) gating the GPU libs and CUDA include dirs behind a single explicit option/check so CPU-only environments fail fast or build a CPU-only variant consistently.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants