Skip to content

Drop thrust/functional it is almost not used at all#7823

Merged
rapids-bot[bot] merged 3 commits intorapidsai:mainfrom
miscco:drop_thrust_functional
Feb 24, 2026
Merged

Drop thrust/functional it is almost not used at all#7823
rapids-bot[bot] merged 3 commits intorapidsai:mainfrom
miscco:drop_thrust_functional

Conversation

@miscco
Copy link
Copy Markdown
Contributor

@miscco miscco commented Feb 23, 2026

No description provided.

@miscco miscco requested a review from a team as a code owner February 23, 2026 12:32
@miscco miscco requested review from dantegd and lowener February 23, 2026 12:32
@miscco miscco force-pushed the drop_thrust_functional branch from 879663b to b8b40b7 Compare February 23, 2026 12:33
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Feb 23, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b8b40b7 and 4626336.

📒 Files selected for processing (1)
  • cpp/src/tsne/distances.cuh
🚧 Files skipped from review as they are similar to previous changes (1)
  • cpp/src/tsne/distances.cuh

📝 Walkthrough

Summary by CodeRabbit

  • Chores

    • Updated copyright years to 2026 across multiple files.
    • Streamlined internal header/dependency usage to reduce unused includes.
  • Compatibility

    • Switched a few internal utilities to CUDA-standard equivalents to improve consistency and build robustness.

Walkthrough

Removed unused <thrust/functional.h> includes across multiple CUDA/C++ sources, replaced one thrust::logical_not<bool>() with cuda::std::logical_not<bool>(), and updated several copyright years from 2025 to 2026.

Changes

Cohort / File(s) Summary
DBSCAN vertexdeg
cpp/src/dbscan/vertexdeg/algo.cuh
Removed several Thrust includes (execution_policy, functional, constant_iterator, counting_iterator, transform); updated copyright year 2025→2026.
HDBSCAN & utilities
cpp/src/hdbscan/condensed_hierarchy.cu, cpp/src/hdbscan/detail/select.cuh, cpp/src/hdbscan/detail/utils.h
Removed unused <thrust/functional.h> includes; no other logic or API changes.
GLM & TSNE files
cpp/src/glm/qn/glm_base.cuh, cpp/src/tsne/distances.cuh, cpp/src/tsne/fft_tsne.cuh
Updated copyright years (2025→2026 variants) and removed <thrust/functional.h> includes; replaced thrust::maximum with cuda::maximum in normalize_distances.
Genetic / TimeSeries / Primitives / Tests
cpp/src/genetic/fitness.cuh, cpp/src_prims/timeSeries/stationarity.cuh, cpp/tests/sg/rf_test.cu
Removed unused <thrust/functional.h> includes; no behavioral or API changes.
TSA auto_arima
cpp/src/tsa/auto_arima.cuh
Switched to <cuda/std/functional> and replaced thrust::logical_not<bool>() with cuda::std::logical_not<bool>() in divide_by_mask_build_index.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested reviewers

  • dantegd
  • lowener
  • csadorf
🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Description check ❓ Inconclusive No pull request description was provided by the author. While this is not ideal, the title and changeset context are sufficient to understand the purpose. Add a description explaining why thrust/functional is being removed and its impact on the codebase.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: removing thrust/functional header includes across multiple files. It is specific and directly related to the changeset.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Tip

Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
cpp/src/tsne/distances.cuh (1)

141-148: ⚠️ Potential issue | 🔴 Critical

thrust::maximum<value_t>() requires thrust/functional.h, which was removed in the recent refactoring.

At Line 146, the code uses thrust::maximum<value_t>() from a header that is no longer included. While thrust/transform_reduce.h may transitively pull it in, this is an undocumented implementation detail that can break with future Thrust library changes.

<cuda/functional> is already included (Line 24) and provides the direct replacement: cuda::maximum<value_t>{}. This aligns with the preference for libcudacxx facilities over Thrust equivalents.

Proposed fix
-                                             thrust::maximum<value_t>());
+                                             cuda::maximum<value_t>{});
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@cpp/src/tsne/distances.cuh` around lines 141 - 148, Replace the use of
thrust::maximum<value_t>() in the transform_reduce that computes maxNorm with
cuda::maximum<value_t>{} (since <cuda/functional> is already included) to avoid
relying on the removed thrust/functional header; update the call site in the
block that computes maxNorm (the thrust::transform_reduce over
distances/total_nn that assigns to maxNorm) so it uses cuda::maximum and leaves
the subsequent raft::linalg::scalarMultiply(distances, distances, 1.0f /
maxNorm, total_nn, stream) unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@cpp/src/tsne/distances.cuh`:
- Around line 141-148: Replace the use of thrust::maximum<value_t>() in the
transform_reduce that computes maxNorm with cuda::maximum<value_t>{} (since
<cuda/functional> is already included) to avoid relying on the removed
thrust/functional header; update the call site in the block that computes
maxNorm (the thrust::transform_reduce over distances/total_nn that assigns to
maxNorm) so it uses cuda::maximum and leaves the subsequent
raft::linalg::scalarMultiply(distances, distances, 1.0f / maxNorm, total_nn,
stream) unchanged.

ℹ️ Review info

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 237e589 and b8b40b7.

📒 Files selected for processing (11)
  • cpp/src/dbscan/vertexdeg/algo.cuh
  • cpp/src/genetic/fitness.cuh
  • cpp/src/glm/qn/glm_base.cuh
  • cpp/src/hdbscan/condensed_hierarchy.cu
  • cpp/src/hdbscan/detail/select.cuh
  • cpp/src/hdbscan/detail/utils.h
  • cpp/src/tsa/auto_arima.cuh
  • cpp/src/tsne/distances.cuh
  • cpp/src/tsne/fft_tsne.cuh
  • cpp/src_prims/timeSeries/stationarity.cuh
  • cpp/tests/sg/rf_test.cu
💤 Files with no reviewable changes (6)
  • cpp/src/genetic/fitness.cuh
  • cpp/tests/sg/rf_test.cu
  • cpp/src/hdbscan/detail/utils.h
  • cpp/src_prims/timeSeries/stationarity.cuh
  • cpp/src/hdbscan/detail/select.cuh
  • cpp/src/hdbscan/condensed_hierarchy.cu

@lowener lowener added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Feb 23, 2026
Copy link
Copy Markdown
Contributor

@lowener lowener left a comment

Choose a reason for hiding this comment

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

LGTM

@aamijar
Copy link
Copy Markdown
Member

aamijar commented Feb 24, 2026

/merge

@rapids-bot rapids-bot Bot merged commit 66fe4a8 into rapidsai:main Feb 24, 2026
99 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CUDA/C++ improvement Improvement / enhancement to an existing function non-breaking Non-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants