Skip to content

Fix KDE score_samples symbol exports - #8173

Merged
rapids-bot[bot] merged 1 commit into
NVIDIA:mainfrom
csadorf:fix-kde-export-symbols
May 27, 2026
Merged

Fix KDE score_samples symbol exports#8173
rapids-bot[bot] merged 1 commit into
NVIDIA:mainfrom
csadorf:fix-kde-export-symbols

Conversation

@csadorf

@csadorf csadorf commented May 27, 2026

Copy link
Copy Markdown
Contributor

Export the KDE score_samples template instantiations and namespace symbols so the Python extension can resolve them at import time.

@csadorf
csadorf requested a review from a team as a code owner May 27, 2026 14:49
@csadorf
csadorf requested review from dantegd and viclafargue May 27, 2026 14:49
@csadorf csadorf added bug Something isn't working non-breaking Non-breaking change labels May 27, 2026

@jcrist jcrist left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

:shipit:

@coderabbitai

coderabbitai Bot commented May 27, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 844e5f8c-850b-449b-800f-f223f78e4d66

📥 Commits

Reviewing files that changed from the base of the PR and between a402255 and d862ab4.

📒 Files selected for processing (2)
  • cpp/include/cuml/neighbors/kde.hpp
  • cpp/src/kde/kde.cu

📝 Walkthrough

Summary by CodeRabbit

  • Chores
    • Updated KDE module symbol export declarations for improved library integration.

Walkthrough

The PR updates the KDE score_samples API to apply CUML_EXPORT macro for explicit symbol visibility. The header namespace is wrapped with CUML_EXPORT, and explicit template instantiations in the source are marked with CUML_EXPORT to ensure consistent visibility of both declarations and compiled specializations.

Changes

KDE API Export Visibility

Layer / File(s) Summary
Header namespace export wrapping
cpp/include/cuml/neighbors/kde.hpp
Adds cuml/common/export.hpp include and wraps ML::KDE namespace with namespace CUML_EXPORT ML { namespace KDE { ... } }, then updates closing braces to match the nested structure.
Explicit template instantiation export
cpp/src/kde/kde.cu
Marks explicit template instantiations of score_samples<float> and score_samples<double> with CUML_EXPORT macro for consistent compiled symbol visibility.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • rapidsai/cuml#7833: Introduces KDE score_samples template and explicit instantiations; this PR adds export visibility to those same APIs.

Suggested labels

CUDA/C++, improvement, Cython / Python

Suggested reviewers

  • jcrist
  • cjnolet
  • betatim
  • viclafargue
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Fix KDE score_samples symbol exports' directly and clearly summarizes the main change: adding export qualifiers to KDE score_samples symbols in the namespace and template instantiations.
Description check ✅ Passed The description accurately relates to the changeset, explaining that the change exports KDE score_samples template instantiations and namespace symbols for Python extension resolution at import time, which matches the actual code modifications.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@dantegd

dantegd commented May 27, 2026

Copy link
Copy Markdown
Contributor

/merge

@rapids-bot
rapids-bot Bot merged commit a8c262b into NVIDIA:main May 27, 2026
105 checks passed
@csadorf
csadorf deleted the fix-kde-export-symbols branch May 27, 2026 16:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working CUDA/C++ non-breaking Non-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants