Fix GB300 and GH200 - #23517
Conversation
There is a bug in the CUDA driver that caused `cudaMemcpyBatchAsync()` to fail when `cudaMemcpyFlagPreferOverlapWithCompute` was passed. Remove this flag and re-enable all GB300 and GH200 testing.
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe workflows no longer exclude GB300 and GH200 GPUs from selected test matrices. The cuDF pandas matrix keeps its existing architecture, CUDA, and Python selection logic. CUDA batch copies now pass zero flags. ChangesTest workflow GPU selection
CUDA batch-copy flags
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
cpp/src/utilities/cuda_memcpy.cu (1)
94-94: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winUpdate the Doxygen contract for the copy attributes.
cpp/include/cudf/detail/utilities/cuda_memcpy.hppstill documentscudaMemcpyFlagPreferOverlapWithCompute, but the wrapper passesflags = 0. Remove the overlap flag from the contract and document the default copy flags.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cpp/src/utilities/cuda_memcpy.cu` at line 94, Update the Doxygen contract for the copy attributes associated with the cudaMemcpy wrapper to remove the cudaMemcpyFlagPreferOverlapWithCompute claim and document that the wrapper uses the default copy flags with flags set to 0. Keep the implementation in the cudaMemcpy attributes initializer unchanged.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@cpp/src/utilities/cuda_memcpy.cu`:
- Line 94: Update the Doxygen contract for the copy attributes associated with
the cudaMemcpy wrapper to remove the cudaMemcpyFlagPreferOverlapWithCompute
claim and document that the wrapper uses the default copy flags with flags set
to 0. Keep the implementation in the cudaMemcpy attributes initializer
unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: cd27a0d6-a851-4869-aafc-7f8dc1865471
📒 Files selected for processing (3)
.github/workflows/pr.yaml.github/workflows/test.yamlcpp/src/utilities/cuda_memcpy.cu
💤 Files with no reviewable changes (1)
- .github/workflows/test.yaml
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
|
/ok to test 8431f9b |
|
/ok to test bf19e35 |
|
/ok to test 0fea12e |
|
/ok to test dd91286 |
|
/ok to test d907c1e |
|
/ok to test b6cbf65 |
|
/ok to test 1f962a5 |
|
/ok to test 9003f7a |
|
/ok to test 3223778 |
|
/ok to test 6bd135e |
|
/ok to test bf8adaf |
|
/ok to test 0637642 |
|
I can repoduce the polars failure: https://github.com/NVIDIA/cudf/actions/runs/32748580160/job/97508667461?pr=23517. It fails CPU-only! I'm working on debugging it now. One option is punting (and filing an issue) and skipping the test to keep forward progress. |
|
The last failing python tests are in https://github.com/NVIDIA/cudf/actions/runs/32753601025/job/97527096857?pr=23517. The first two are hybrid scan tests. CC @mhaseeb123 (for awareness). I'm checking if I can reproduce. If so, I'll have an agent go down the rabbit hole. Details |
|
/ok to test b800d95 |
…23783) Discovered in #23517 Lets skip it for now until the bug is fixed in Polars. Authors: - Matthew Murray (https://github.com/Matt711) Approvers: - Bradley Dice (https://github.com/bdice) URL: #23783
Description
The cudaMemcpyBatchAsync with cudaMemcpySrcAccessOrderStream + cudaMemcpyFlagPreferOverlapWithCompute defers reading the copy source until the stream reaches the copy. cuDF passes local host vectors as sources in many places; they're destroyed before the stream gets there, so the deferred read hits freed memory.
Fixes add
stream.synchronize()where a local host buffer is the copy source and goes out of scope before the copy completes.Checklist