Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[REVIEW] Single-Linkage Hierarchical Clustering Python Wrapper #3631

Merged
merged 118 commits into from
Mar 30, 2021
Merged

[REVIEW] Single-Linkage Hierarchical Clustering Python Wrapper #3631

merged 118 commits into from
Mar 30, 2021

Conversation

cjnolet
Copy link
Member

@cjnolet cjnolet commented Mar 18, 2021 •
edited

Closes #3099

cjnolet and others added 30 commits December 15, 2020 12:33
@cjnolet
Checking in
43a8118
@cjnolet
Getting MST to return results
335e1f9
@cjnolet
Still trying to figure out why MST isn't returning expected results
8dc58ba
@cjnolet
Adding symmetrization to linkage
cef178a
@cjnolet
Fixing style
1f00cbf
@cjnolet
Merge branch 'branch-0.18' into fea-018-hdbscan
209733b
@cjnolet
Test is executing end-to-end, need to verify results
957192f
@cjnolet
Adding new symmetrizaiton
20e32e8
@cjnolet
Checking in
107b34d
@cjnolet
Merge branch 'branch-0.18' into fea-018-hdbscan
5dd72ba 
Conflicts:
	cpp/src_prims/sparse/coo.cuh
@cjnolet
Merge branch 'branch-0.18' into fea-018-hdbscan
3fe27c7
@cjnolet
Adding final cluster extraction
48fbd1a
@cjnolet
Fixing style
98dbf47
@cjnolet
Fixing symmetrizatio bug
567ab06
@cjnolet
Output matches sklearn
7635b25
@cjnolet
Updating include check for test
f3f45eb
@cjnolet
Fixing style
02125d6
@cjnolet
Checking in
c8f1a58
@cjnolet
Cleaning up logging
20f57a4
@cjnolet
Updating raft commit
3b08d79
@cjnolet
Fixing style
3e95e4b
@cjnolet
Fixing style
9525b80
@cjnolet
Cleaning up log statements & fixing small bug in inherit_labels
33b1948
@cjnolet
Adding test for outliers
2ad0580
@cjnolet
Merge branch 'branch-0.18' into fea-018-hdbscan
067ca4b
@cjnolet
Updating test cmakelists
c69d965
@cjnolet
Adding benchmark for linkage
7f78358
@cjnolet
Changes from benchmarking
e608d2b
@cjnolet
Updating changes
0a9cf0d
@cjnolet
Checking in
f012f35
@cjnolet cjnolet changed the title [WIP] Single-Linkage Hierarchical Clustering Python Wrapper [REVIEW] Single-Linkage Hierarchical Clustering Python Wrapper Mar 26, 2021
@cjnolet cjnolet added 3 - Ready for Review Ready for review by team feature request New feature or request non-breaking Non-breaking change and removed 2 - In Progress Currenty a work in progress labels Mar 26, 2021
@cjnolet cjnolet moved this from PR-WIP to PR-Needs review in v0.19 Release Mar 26, 2021
@github-actions github-actions bot added the CMake label Mar 26, 2021
dantegd
dantegd requested changes Mar 26, 2021
View reviewed changes
Copy link
Member

@dantegd dantegd left a comment

Choose a reason for hiding this comment

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

Looks very clean and neat! Just had super minor comments

python/cuml/cluster/__init__.py Show resolved Hide resolved
python/cuml/cluster/agglomerative.pyx Show resolved Hide resolved
python/cuml/cluster/agglomerative.pyx Outdated Show resolved Hide resolved
python/cuml/cluster/agglomerative.pyx Outdated Show resolved Hide resolved
cpp/cmake/Dependencies.cmake Outdated Show resolved Hide resolved
@dantegd dantegd added 4 - Waiting on Author Waiting for author to respond to review and removed 3 - Ready for Review Ready for review by team labels Mar 26, 2021
@codecov-io
Copy link

Codecov Report

Merging #3631 (0883026) into branch-0.19 (0883026) will not change coverage.
The diff coverage is n/a.

❗ Current head 0883026 differs from pull request most recent head 3f6217d. Consider uploading reports for the commit 3f6217d to get more accurate results
Impacted file tree graph

@@             Coverage Diff              @@
##           branch-0.19    #3631   +/-   ##
============================================
  Coverage        73.13%   73.13%           
============================================
  Files              215      215           
  Lines            16987    16987           
============================================
  Hits             12423    12423           
  Misses            4564     4564
Flag Coverage Δ
non-dask 73.13% <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 0883026...3f6217d. Read the comment docs.

@cjnolet cjnolet added 4 - Waiting on Reviewer Waiting for reviewer to review or respond and removed 4 - Waiting on Author Waiting for author to respond to review labels Mar 27, 2021
@cjnolet
Copy link
Member Author

cjnolet commented Mar 29, 2021

rerun tests

v0.19 Release automation moved this from PR-Needs review to PR-Reviewer approved Mar 29, 2021
@cjnolet
Copy link
Member Author

cjnolet commented Mar 30, 2021

rerun tests

@cjnolet cjnolet merged commit 668aea1 into rapidsai:branch-0.19 Mar 30, 2021
v0.19 Release automation moved this from PR-Reviewer approved to Done Mar 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dantegd dantegd dantegd approved these changes

Assignees
No one assigned
Labels
4 - Waiting on Reviewer Waiting for reviewer to review or respond CMake Cython / Python Cython or Python issue feature request New feature or request non-breaking Non-breaking change
Projects
No open projects
v0.19 Release
  
Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[FEA] Single-linkage Hierarchical Clustering
4 participants
@cjnolet @ajschmidt8 @codecov-io @dantegd