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

Pickling for HBDSCAN #5102

Merged
merged 18 commits into from
Jan 4, 2023
Merged

Pickling for HBDSCAN #5102

merged 18 commits into from
Jan 4, 2023

Conversation

divyegala
Copy link
Member

@divyegala divyegala commented Dec 21, 2022 •
edited
Loading

closes #4986

@divyegala divyegala added feature request New feature or request non-breaking Non-breaking change labels Dec 21, 2022
@github-actions github-actions bot added CMake CUDA/C++ Cython / Python Cython or Python issue labels Dec 21, 2022
@divyegala divyegala marked this pull request as ready for review December 29, 2022 22:01
@divyegala divyegala requested review from a team as code owners December 29, 2022 22:02
@divyegala
Copy link
Member Author

cc @cjnolet @beckernick this is ready for review

cjnolet
cjnolet requested changes Jan 3, 2023
View reviewed changes
Copy link
Member

@cjnolet cjnolet left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this, Divye! Just a couple things.

cpp/include/cuml/cluster/hdbscan.hpp Outdated Show resolved Hide resolved
cpp/include/cuml/cluster/hdbscan.hpp Outdated Show resolved Hide resolved
python/cuml/tests/test_pickle.py Outdated Show resolved Hide resolved
@divyegala divyegala requested a review from cjnolet January 4, 2023 19:48
cjnolet
cjnolet approved these changes Jan 4, 2023
View reviewed changes
Copy link
Member

@cjnolet cjnolet left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for fixing this, Divye!

@github-actions github-actions bot removed the CMake label Jan 4, 2023
@beckernick
Copy link
Member

Thanks for implementing this. This is going to make a huge impact on user workflows

@codecov-commenter
Copy link

Codecov Report

Base: 69.31% // Head: 69.05% // Decreases project coverage by -0.25% ⚠️

Coverage data is based on head (60621c1) compared to base (96c983d).
Patch coverage: 12.50% of modified lines in pull request are covered.

Additional details and impacted files 
@@               Coverage Diff                @@
##           branch-23.02    #5102      +/-   ##
================================================
- Coverage         69.31%   69.05%   -0.26%     
================================================
  Files               192      192              
  Lines             12369    12377       +8     
================================================
- Hits               8573     8547      -26     
- Misses             3796     3830      +34
Impacted Files Coverage Δ
python/cuml/internals/import_utils.py 46.51% <12.50%> (-2.25%) ⬇️
python/cuml/internals/available_devices.py 64.28% <0.00%> (-21.43%) ⬇️
python/cuml/metrics/pairwise_kernels.py 72.58% <0.00%> (-8.07%) ⬇️
python/cuml/neighbors/kernel_density.py 75.49% <0.00%> (-7.29%) ⬇️
python/cuml/testing/utils.py 88.32% <0.00%> (-0.60%) ⬇️
python/cuml/internals/array.py 87.94% <0.00%> (-0.20%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@divyegala
Copy link
Member Author

@gpucibot merge

@rapids-bot
Copy link

rapids-bot bot commented Jan 4, 2023

@divyegala, the @gpucibot merge command has been replaced with /merge.

Please re-comment this PR with /merge and use this new command in the future.

@divyegala
Copy link
Member Author

/merge

@rapids-bot rapids-bot bot merged commit 0303a93 into rapidsai:branch-23.02 Jan 4, 2023
jakirkham pushed a commit to jakirkham/cuml that referenced this pull request Feb 27, 2023
@divyegala
Pickling for HBDSCAN (rapidsai#5102)
d2e7b8e 
closes rapidsai#4986

Authors:
  - Divye Gala (https://github.com/divyegala)

Approvers:
  - Corey J. Nolet (https://github.com/cjnolet)

URL: rapidsai#5102
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cjnolet cjnolet cjnolet approved these changes

Assignees

@divyegala divyegala

Labels
CUDA/C++ Cython / Python Cython or Python issue feature request New feature or request non-breaking Non-breaking change
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[BUG] Persisting HDBScan model with Pickle/joblib and approximate_predict results in core dump
4 participants
@divyegala @beckernick @codecov-commenter @cjnolet