Skip to content

Conversation

@esnvidia
Copy link

@esnvidia esnvidia commented Jan 2, 2026

When vertex_labels is std::nullopt, the else branch incorrectly attempted to access vertex_labels_p1->begin() and vertex_labels_p2->begin(), causing cudaErrorIllegalAddress crash.

This fix removes the label iterators from thrust::make_zip_iterator calls in the else branch, since labels are not available when vertex_labels is nullopt.

Fixes temporal neighbor sampling when called without vertex labels, e.g., homogeneous_uniform_temporal_neighbor_sample with temporal_property_name=None.

All 210+ temporal sampling tests pass with this fix.

PR_TEMPORAL_SAMPLING_FIX.md

Pull Request: Fix null optional dereference in temporal_partition_vertices

Summary

Fixes a crash (cudaErrorIllegalAddress) when calling temporal neighbor sampling without vertex labels.

Description

The temporal_partition_vertices function in temporal_partition_vertices_impl.cuh has an else branch that handles the case when vertex_labels is std::nullopt. However, this branch incorrectly attempts to dereference the null optional when constructing thrust::make_zip_iterator, causing an illegal memory access.

Root Cause

The else branch was copy-pasted from the if branch but the label iterators were not removed:

// BUGGY: In else branch where vertex_labels is nullopt
thrust::make_zip_iterator(
    vertices_p1.begin(), 
    vertex_times_p1.begin(), 
    vertex_labels_p1->begin())  // <-- Dereferences nullopt!

The Fix

Remove label iterators from the else branch since labels are not available:

// FIXED: No labels in zip iterator when vertex_labels is nullopt
thrust::make_zip_iterator(
    vertices_p1.begin(), 
    vertex_times_p1.begin())  // Only vertices and times

Testing

C++ Tests (all pass)

100% tests passed, 0 tests failed out of 4

- TEMPORAL_GRAPH_TEST ..................... Passed    3.77 sec
- TEMPORAL_NEIGHBOR_SAMPLING_TEST ......... Passed  514.50 sec (210 tests)
- CAPI_UNIFORM_TEMPORAL_NEIGHBOR_SAMPLE_TEST Passed    0.85 sec

Python Verification

# This was crashing before the fix:
result = homogeneous_uniform_temporal_neighbor_sample(
    rh, G, 
    None,  # temporal_property_name=None triggers the bug
    start_vertices,
    start_times,
    None,  # no label offsets
    fanout,
    with_replacement=True, 
    do_expensive_check=False
)
# Now works: SUCCESS! Sampled 645 edges

Files Changed

  • cpp/src/sampling/detail/temporal_partition_vertices_impl.cuh

Diff

diff --git a/cpp/src/sampling/detail/temporal_partition_vertices_impl.cuh b/cpp/src/sampling/detail/temporal_partition_vertices_impl.cuh
index ea1eb1e6e..97a42730f 100644
--- a/cpp/src/sampling/detail/temporal_partition_vertices_impl.cuh
+++ b/cpp/src/sampling/detail/temporal_partition_vertices_impl.cuh
@@ -141,28 +141,22 @@ temporal_partition_vertices(raft::handle_t const& handle,
       vertex_labels_p1->resize(vertices_p1.size(), handle.get_stream());
       vertex_times_p1.resize(vertices_p1.size(), handle.get_stream());
     } else {
+      // FIXED: When vertex_labels is std::nullopt, don't include labels in zip iterator
       copy_if_mask_unset(
         handle,
-        thrust::make_zip_iterator(
-          vertices_p1.begin(), vertex_times_p1.begin(), vertex_labels_p1->begin()),
-        thrust::make_zip_iterator(
-          vertices_p1.end(), vertex_times_p1.end(), vertex_labels_p1->end()),
+        thrust::make_zip_iterator(vertices_p1.begin(), vertex_times_p1.begin()),
+        thrust::make_zip_iterator(vertices_p1.end(), vertex_times_p1.end()),
         vertex_partition_mask.begin(),
-        thrust::make_zip_iterator(
-          vertices_p2.begin(), vertex_times_p2.begin(), vertex_labels_p2->begin()));
+        thrust::make_zip_iterator(vertices_p2.begin(), vertex_times_p2.begin()));
       vertices_p1.resize(
         thrust::distance(
-          thrust::make_zip_iterator(
-            vertices_p1.begin(), vertex_times_p1.begin(), vertex_labels_p1->begin()),
+          thrust::make_zip_iterator(vertices_p1.begin(), vertex_times_p1.begin()),
           copy_if_mask_set(
             handle,
-            thrust::make_zip_iterator(
-              vertices_p1.begin(), vertex_times_p1.begin(), vertex_labels_p1->begin()),
-            thrust::make_zip_iterator(
-              vertices_p1.end(), vertex_times_p1.end(), vertex_labels_p1->end()),
+            thrust::make_zip_iterator(vertices_p1.begin(), vertex_times_p1.begin()),
+            thrust::make_zip_iterator(vertices_p1.end(), vertex_times_p1.end()),
             vertex_partition_mask.begin(),
-            thrust::make_zip_iterator(
-              vertices_p1.begin(), vertex_times_p1.begin(), vertex_labels_p1->begin()))),
+            thrust::make_zip_iterator(vertices_p1.begin(), vertex_times_p1.begin()))),
         handle.get_stream());
 
       vertex_times_p1.resize(vertices_p1.size(), handle.get_stream());

Checklist

  • Code compiles without errors
  • All existing tests pass (210+ temporal sampling tests)
  • No new warnings introduced
  • Python API verified working with fix
  • Fix is minimal and focused on the specific bug

Impact

  • Before: Any call to temporal neighbor sampling without vertex labels crashes with cudaErrorIllegalAddress
  • After: Temporal sampling works correctly with or without vertex labels

When vertex_labels is std::nullopt, the else branch incorrectly
attempted to access vertex_labels_p1->begin() and vertex_labels_p2->begin(),
causing cudaErrorIllegalAddress crash.

This fix removes the label iterators from thrust::make_zip_iterator
calls in the else branch, since labels are not available when
vertex_labels is nullopt.

Fixes temporal neighbor sampling when called without vertex labels,
e.g., homogeneous_uniform_temporal_neighbor_sample with
temporal_property_name=None.

All 210+ temporal sampling tests pass with this fix.
@esnvidia esnvidia requested a review from a team as a code owner January 2, 2026 15:45
@copy-pr-bot
Copy link

copy-pr-bot bot commented Jan 2, 2026

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@ChuckHastings ChuckHastings added bug Something isn't working non-breaking Non-breaking change labels Jan 6, 2026
@ChuckHastings
Copy link
Collaborator

/ok to test 06a2380

@ChuckHastings
Copy link
Collaborator

You need to update the copyright date to be: 2025-2026. Then we can try and build it in our continuous integration system and get this merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working non-breaking Non-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants