Support recent hdbscan - #7956
Conversation
The signature of `CondensedTree` changed in 0.8.41, this fixes that.
This test only asserted things if the number of labels were identical between the sklearn and cuml versions. In past versions of hdbscan this was never true, leading the tests to not assert anything. In new versions this is _sometimes_ true, leading tests to sometimes fail. We could have xfailed this, but given it was providing no signal before (and we have other tests for this functionality) I'm opting to just rip it out for now.
We now test across a few versions of `hdbscan`, ensuring we keep up with upstream changes.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughLoosened hdbscan version pins across conda and project configs, moved hdbscan version control into CI matrices, added runtime hdbscan version detection in the Cython HDBSCAN wrapper to support pre-/post-0.8.41 CondensedTree signatures, and adjusted tests (guarded skip; removed one test). Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@python/cuml/cuml/cluster/hdbscan/hdbscan.pyx`:
- Around line 868-872: The code passes a CumlArray (self.labels_) directly into
hdbscan.plots.CondensedTree which expects a NumPy array; convert/normalize
labels_ to host NumPy before constructing the CondensedTree (use the existing
pattern via self._get_raw_data_cpu(self.labels_) or equivalent conversion) so
call hdbscan.plots.CondensedTree(self._condensed_tree,
self._get_raw_data_cpu(self.labels_)).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 481ca140-678a-47c8-8c69-84e805343222
📒 Files selected for processing (8)
conda/environments/all_cuda-129_arch-aarch64.yamlconda/environments/all_cuda-129_arch-x86_64.yamlconda/environments/all_cuda-131_arch-aarch64.yamlconda/environments/all_cuda-131_arch-x86_64.yamldependencies.yamlpython/cuml/cuml/cluster/hdbscan/hdbscan.pyxpython/cuml/pyproject.tomlpython/cuml/tests/test_hdbscan.py
|
/merge |
This:
hdbscanin our tests. Previously we'd pinned to test against version 0.8.39, which means we'd missed several recent hdbscan releases. Now our "latest" deps tests will always use the latest release, ensuring we notice upstream breaking changes. We also test against the previous pinned version (in "oldest" test runs), as well as one intermediate version.condensed_tree_to work with recent hdbscanhdbscanandsklearnFixes #7955.