[REVIEW] HDBSCAN #3821

cjnolet · 2021-05-03T20:30:20Z

We had originally tried to ease the review process by separating the Python and CUDA/C++ layer code for HDBSCAN (see #3773), and for the most part we have been separating the commits, however this made it very cumbersome to keep cherry-picking commits backwards so we abandoned #3773 and continued working on this PR.

…0-hdbscan

tfeher

Thanks for the updates, the PR looks good to me!

dantegd

Everything looks good from me too, just requested adding an item to the follow ups issue (if it’s not already there). Once hdbscan python package is in integration we should be good to merge.

dantegd · 2021-05-29T17:32:49Z

python/cuml/experimental/cluster/hdbscan.pyx

+                                         dtype=dtype,
+                                         memptr=mem_ptr)).to_output('numpy')
+
+    def _construct_condensed_tree_attribute(self, ptr, dtype="int32"):


Can you add the item of trying this as a property, I think it can be a follow up nice to have but not indispensable (and need to think about the getattr…)

dantegd

Totally missed the conda change, which goes hand in hand with adding hdbscan to the rapids-build meta package

conda/recipes/cuml/meta.yaml

cjnolet · 2021-05-29T18:27:31Z

Can you add the item of trying this as a property, I think it can be a follow up nice to have but not indispensable (and need to think about the getattr…)

@dantegd, absolutely I'd like to use a lazy-loaded property here. I initially tried this as a lazy-populated property (using @property) but I kept getting an error in __hasattr__ of the base class and I couldn't figure out a good way to make it work. It is very possible that I was doing something wrong, though, and I would prefer to use that pattern if we can get it to work.

cjnolet · 2021-06-01T14:56:35Z

Added item to our follow-on task to revisit the property / lazy-loaded members for plotting objects: #3879 (comment)

cjnolet · 2021-06-01T15:10:28Z

PR to add hdbscan package to rapids-build-env for tests: rapidsai/integration#288

ajschmidt8 · 2021-06-01T20:00:44Z

Removing ops-codeowners from the required reviews since it doesn't seem there are any file changes that we're responsible for. Feel free to add us back if necessary.

dantegd · 2021-06-02T16:09:49Z

rerun tests

dantegd · 2021-06-02T16:10:04Z

rerun tests

dantegd · 2021-06-02T16:10:53Z

@gpucibot merge

cjnolet · 2021-06-02T19:52:20Z

Going to try again in hopes the changes to the integration repo just weren't picked up in time.

cjnolet · 2021-06-02T19:52:27Z

rerun tests

cjnolet · 2021-06-02T20:42:56Z

rerun tests

codecov-commenter · 2021-06-03T22:03:12Z

Codecov Report

❗ No coverage uploaded for pull request base (branch-21.06@92484fb). Click here to learn what that means.
The diff coverage is n/a.

@@               Coverage Diff               @@
##             branch-21.06    #3821   +/-   ##
===============================================
  Coverage                ?   85.13%           
===============================================
  Files                   ?      230           
  Lines                   ?    18138           
  Branches                ?        0           
===============================================
  Hits                    ?    15441           
  Misses                  ?     2697           
  Partials                ?        0

Flag	Coverage Δ
dask	`47.80% <0.00%> (?)`
non-dask	`77.49% <0.00%> (?)`

Flags with carried forward coverage won't be shown. Click here to find out more.

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 92484fb...1519478. Read the comment docs.

We had originally tried to ease the review process by separating the Python and CUDA/C++ layer code for HDBSCAN (see rapidsai#3773), and for the most part we have been separating the commits, however this made it very cumbersome to keep cherry-picking commits backwards so we abandoned rapidsai#3773 and continued working on this PR. Authors: - Corey J. Nolet (https://github.com/cjnolet) - Divye Gala (https://github.com/divyegala) Approvers: - Tamas Bela Feher (https://github.com/tfeher) - Dante Gama Dessavre (https://github.com/dantegd) URL: rapidsai#3821

cjnolet and others added 30 commits April 20, 2021 17:17

HDBSCAN

c0a1b8b

Fixing style

3800f76

Progress

362d429

Checking in

abf2267

Checking in todo stubs of remaining work for initial implementation

c68bd8d

Creating output condensed hierarchy

78a8432

Resizing rmm arrays

b502e1d

Adding missing semicolon

146ac00

initial ideas to compute stabilities

3cee98b

Checking in

60253ce

Updating

e4668d3

separating kernels into detail

1e9f557

Merge branch 'fea-020-hdbscan' of github.com:cjnolet/cuml into fea-02…

0331697

…0-hdbscan

allocating stabilities through caller

fd77b48

cleaning stabilities a bit

7edb4a3

Initial stub for eom is done.

0efd58b

Adding data to epilogue

bbb35db

COuple small updates

83d8ac3

Adding get_stability scores

f563aca

tests building

bbe1ffe

cleaning up some comments

d135645

Fixing style

0fce1bb

merging

7052836

merge and compile

b6048ce

Updates to get hdbscan to compile

f05d545

Getting remaining stuff in extract_clusters to compile

1628a36

Using zip iterators for copy_if

463f757

compiling compute stabilities

dcc83db

stabilities and merge building

4829b21

Sorry!

8814181

tfeher approved these changes May 28, 2021

View reviewed changes

Adding hdbscan to build dependencies (for pytests)

d143f05

cjnolet requested a review from a team as a code owner May 29, 2021 00:14

github-actions bot added the conda conda issue label May 29, 2021

dantegd approved these changes May 29, 2021

View reviewed changes

dantegd requested changes May 29, 2021

View reviewed changes

conda/recipes/cuml/meta.yaml Outdated Show resolved Hide resolved

Removing hdbscan change from HDBSCAN PR

13dca59

github-actions bot removed the conda conda issue label Jun 1, 2021

cjnolet added 5 - Ready to Merge Testing and reviews complete, ready to merge and removed 3 - Ready for Review Ready for review by team labels Jun 1, 2021

ajschmidt8 removed the request for review from a team June 1, 2021 20:00

dantegd approved these changes Jun 2, 2021

View reviewed changes

cjnolet added 3 commits June 2, 2021 18:36

Fixing docs and addig param

095dcea

Fixing base test

444cffb

Fixing doxygen

1519478

rapids-bot bot merged commit 870c7ee into rapidsai:branch-21.06 Jun 3, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[REVIEW] HDBSCAN #3821

[REVIEW] HDBSCAN #3821

cjnolet commented May 3, 2021 •

edited

Loading

tfeher left a comment

dantegd left a comment

dantegd May 29, 2021

dantegd left a comment

cjnolet commented May 29, 2021 •

edited

Loading

cjnolet commented Jun 1, 2021

cjnolet commented Jun 1, 2021

ajschmidt8 commented Jun 1, 2021

dantegd commented Jun 2, 2021

dantegd commented Jun 2, 2021

dantegd commented Jun 2, 2021

cjnolet commented Jun 2, 2021

cjnolet commented Jun 2, 2021

cjnolet commented Jun 2, 2021

codecov-commenter commented Jun 3, 2021

[REVIEW] HDBSCAN #3821

[REVIEW] HDBSCAN #3821

Conversation

cjnolet commented May 3, 2021 • edited Loading

tfeher left a comment

Choose a reason for hiding this comment

dantegd left a comment

Choose a reason for hiding this comment

dantegd May 29, 2021

Choose a reason for hiding this comment

dantegd left a comment

Choose a reason for hiding this comment

cjnolet commented May 29, 2021 • edited Loading

cjnolet commented Jun 1, 2021

cjnolet commented Jun 1, 2021

ajschmidt8 commented Jun 1, 2021

dantegd commented Jun 2, 2021

dantegd commented Jun 2, 2021

dantegd commented Jun 2, 2021

cjnolet commented Jun 2, 2021

cjnolet commented Jun 2, 2021

cjnolet commented Jun 2, 2021

codecov-commenter commented Jun 3, 2021

Codecov Report

cjnolet commented May 3, 2021 •

edited

Loading

cjnolet commented May 29, 2021 •

edited

Loading