Skip to content

[cudax][cuco] Migrate host contains_if APIs for fixed_capacity_set - #12357

Merged
miscco merged 5 commits into
NVIDIA:mainfrom
PointKernel:cuco-set-contains-if
Nov 11, 2026
Merged

miscco merged 5 commits into
NVIDIA:mainfrom
PointKernel:cuco-set-contains-if

Conversation

@PointKernel

@PointKernel PointKernel commented Oct 30, 2026 •

Copy link
Copy Markdown
Member

Description

Closes #11135.

This PR migrates the host contains_if and contains_if_async APIs from cuCollections static_set to fixed_capacity_set, reusing the shared open-addressing implementation.

Checklist

  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

Add host contains_if and contains_if_async to
cudax::cuco::fixed_capacity_set, forwarding to the shared open-addressing
implementation, and cover them in the insert and contains test with a
stencil distinct from the keys.
@copy-pr-bot

copy-pr-bot Bot commented Oct 30, 2026

Copy link
Copy Markdown
Contributor

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.

@cccl-authenticator-app cccl-authenticator-app Bot moved this from Todo to In Progress in CCCL Oct 30, 2026
@PointKernel PointKernel added cuco cuCollections cudax Feature intended for the cudax experimental library labels Oct 30, 2026
@PointKernel

Copy link
Copy Markdown
Member Author

/ok to test

@github-actions

This comment has been minimized.

@PointKernel
PointKernel marked this pull request as ready for review October 30, 2026 22:54
@PointKernel
PointKernel requested review from a team as code owners October 30, 2026 22:54
@cccl-authenticator-app cccl-authenticator-app Bot moved this from In Progress to In Review in CCCL Oct 30, 2026
@coderabbitai

coderabbitai Bot commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

Explain Change →

No actionable comments were generated in the recent review.

Summary

Summary by CodeRabbit

  • New Features
    • Added conditional membership checks for fixed-capacity sets, with synchronous and asynchronous options. A predicate selects which queries are checked against the set; unselected queries return false.
  • Tests
    • Added coverage for conditional lookups with offset stencil values, including results before and after inserting keys, and verified that only selected queries receive membership results.

Adds synchronous and asynchronous contains_if APIs to fixed_capacity_set. The APIs use a predicate on stencil values to select queries and write membership results. Tests cover both overloads and verify results across the query range.

Changes: Conditional lookup

Layer / File(s) Summary
Add conditional lookup APIs
cudax/include/cuda/experimental/__cuco/fixed_capacity_set.cuh
Adds synchronous and asynchronous contains_if overloads. Both write membership results for selected keys and false for unselected keys. The synchronous overload synchronizes the supplied stream; the asynchronous overload does not.
Test conditional lookup
cudax/test/cuco/fixed_capacity_set/test_insert_and_contains.cu
Adds predicate helpers and tests both overloads with an offset stencil. Checks membership results for even-indexed inserted keys.

Assessment against linked issues

Objective Addressed Explanation
Migrate host contains_if APIs to fixed_capacity_set and add corresponding tests [#11135] ✅

Possibly related PRs

  • NVIDIA/cccl#12355: Adds stencil-filtered lookup APIs that return stored keys or a sentinel rather than membership results.

Suggested reviewers: fbusato

Change: Feature

Merge Risk: Low · up to 41473

The change is mergeable with a test follow-up: the filtered lookup test does not detect every missing write, but no current API failure or build blocker is established.

Security architecture risk: Minimal · up to 41473

The new queries reuse existing lookup execution and preserve caller-managed storage and stream synchronization. The review found no introduced security finding or material expansion of privileges or trust-boundary exposure.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The added API operates with the invoking application's existing CUDA authority over supplied iterators, set storage, and stream. The inspected change adds query selection, not a service, tenant, credential, or sandbox boundary.

Trust Boundaries and Controls

  • inferred — The stencil predicate is caller-supplied query logic, not an authorization control. As with existing membership operations, the caller must provide valid device-accessible ranges and retain storage until queued work completes; the new wrappers do not validate untrusted pointers or transfer storage ownership.

Resilience and Maintainability Implications

  • inferred — The methods preserve the existing borrowed-storage execution model. Repeated queries do not introduce a set-state transition, while overlapping output writes and premature destruction still require caller coordination. Launch and synchronization errors are reported, but callers must not assume atomic output publication or rollback after execution failure; that limitation predates these wrappers.
- [ ] Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Recent review info

Run configuration

  • Source mode: Auto
  • Repository: NVIDIA/cccl
  • Branch: main
  • File: .coderabbit.yaml
  • Commit: 22bbd61
  • Layers: Repository YAML
  • Review profile: CHILL
  • Plan: Enterprise
  • Run ID: 4e351f9f-542f-4f62-8e2d-8784ae5d033b
Commits

Reviewing files that changed from the base of the PR and between de7680b and 414731d.

Files selected for processing: 2

  • cudax/include/cuda/experimental/__cuco/fixed_capacity_set.cuh
  • cudax/test/cuco/fixed_capacity_set/test_insert_and_contains.cu

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.

@PointKernel PointKernel self-assigned this Nov 3, 2026
@github-actions

This comment has been minimized.

# Conflicts:
#	cudax/test/cuco/fixed_capacity_set/test_insert_and_contains.cu

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Nitpick comments: 1

cudax/test/cuco/fixed_capacity_set/test_insert_and_contains.cu: 1

90-99: Trivial · Maintainability & Code Quality · Quick win

suggestion: Initialize the filtered output before the lookup.

Selected positions already contain the expected membership values. A regression that leaves those positions unchanged can pass this assertion.

Suggested fix

-  set.contains_if(context.stream, first, first + num_queries, stencil, is_odd{}, found.begin());
+  auto filtered_found = cuda::make_buffer<int>(context.stream, context.mr, num_queries, 7);
+  set.contains_if(context.stream, first, first + num_queries, stencil, is_odd{}, filtered_found.begin());
   REQUIRE(cuda::std::all_of(
     context.policy(),
     cuda::counting_iterator<int>{0},
     cuda::counting_iterator<int>{num_queries},
-    matches_even_membership{found.data(), num_keys}));
+    matches_even_membership{filtered_found.data(), num_keys}));
Review info

Run configuration

  • Source mode: Auto
  • Repository: NVIDIA/cccl
  • Branch: main
  • File: .coderabbit.yaml
  • Commit: dd063b7
  • Layers: Repository YAML
  • Review profile: CHILL
  • Plan: Enterprise
  • Run ID: 3ccb783e-2a6a-46f6-a52d-636a2e0925bd
Commits

Reviewing files that changed from the base of the PR and between 83458af and de7680b.

Files selected for processing: 2

  • cudax/include/cuda/experimental/__cuco/fixed_capacity_set.cuh
  • cudax/test/cuco/fixed_capacity_set/test_insert_and_contains.cu

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 9 remain after this review.

Comment on lines +90 to +91
// Offset from the keys so that `pred(stencil[i])` and `pred(first[i])` disagree.
const auto stencil = cuda::counting_iterator<int>{1};

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Major · Functional Correctness · Quick win

Supported by static analysis

Script executed:

sed -n '55,150p' cudax/test/cuco/fixed_capacity_set/test_insert_and_contains.cu
rg -n 'test_insert_and_contains|define C2H_TEST' cudax/test/cuco

Repository: NVIDIA/cccl

Length of output: 5337


Script executed:

cat -n cudax/test/cuco/fixed_capacity_set/test_insert_and_contains.cu | sed -n '1,185p'
cat -n cudax/test/cuco/fixed_capacity_set/CMakeLists.txt
cat -n cudax/test/cuco/CMakeLists.txt
rg -n --glob '*.[ch]' --glob '*.cuh' --glob '*.hpp' '#[[:space:]]*define[[:space:]]+C2H_TEST' cudax c2h libcudacxx thrust cub 2>/dev/null
git diff dd063b73e63e18f911182f79879fbf7d558ebe79 de7680b3b25b53bca9e8c3bd99bc3401269de02b -- cudax/test/cuco/fixed_capacity_set/test_insert_and_contains.cu

Repository: NVIDIA/cccl

Length of output: 14218


Script executed:

cat -n c2h/include/c2h/catch2_test_helper.h | sed -n '625,660p'
rg -n 'function\(cudax_add_catch2_test|macro\(cudax_add_catch2_test' cudax

Repository: NVIDIA/cccl

Length of output: 1859


Script executed:

cat -n cudax/test/CMakeLists.txt | sed -n '1,105p'

Repository: NVIDIA/cccl

Length of output: 3712


important: Give the stencil variables distinct names.

The two stencil declarations share a scope, so the test target cannot compile. Rename the later variable and its uses.

Suggested fix

-  const auto stencil = cuda::counting_iterator<int>{-num_keys / 2};
+  const auto insert_stencil = cuda::counting_iterator<int>{-num_keys / 2};
@@
-  REQUIRE(set.insert_if(context.stream, first, first + num_keys, stencil, is_negative{}) == num_keys / 2);
+  REQUIRE(set.insert_if(context.stream, first, first + num_keys, insert_stencil, is_negative{}) == num_keys / 2);
@@
-  set.insert_if_async(context.stream, first, first + num_keys, stencil, is_negative{});
+  set.insert_if_async(context.stream, first, first + num_keys, insert_stencil, is_negative{});

@github-actions

This comment has been minimized.

@github-actions

Copy link
Copy Markdown
Contributor

🥳 CI Workflow Results

🟩 Finished in 1h 22m: Pass: 100%/75 | Total: 13h 08m | Max: 1h 22m | Hits: 94%/52101

See results here.

@miscco
miscco merged commit 24ee635 into NVIDIA:main Nov 11, 2026
95 checks passed
@github-project-automation github-project-automation Bot moved this from In Review to Done in CCCL Nov 11, 2026
@PointKernel
PointKernel deleted the cuco-set-contains-if branch November 11, 2026 17:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cuco cuCollections cudax Feature intended for the cudax experimental library

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

[CUCO][FEA]: Migrate host contains_if APIs for fixed_capacity_set

4 participants