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.

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

Small cleanup. #3575

Closed
wants to merge 4 commits into from
Closed

Conversation

trivialfis
Copy link
Member

@trivialfis trivialfis commented Mar 3, 2021

  • Remove unused code.
  • Fix comment.

* Remove unused code.
* Fix comment.
* Add missing header.
@trivialfis trivialfis requested a review from a team as a code owner March 3, 2021 13:46
@trivialfis trivialfis added non-breaking Non-breaking change improvement Improvement / enhancement to an existing function and removed CUDA/C++ labels Mar 3, 2021
@cjnolet
Copy link
Member

cjnolet commented Mar 3, 2021

@trivialfis, really happy to see these prims being cleaned up a bit. Some of them haven't been touched much since the original version of UMAP was written back in 2019 and can really use some TLC.

As a side note- the sparse prims have all been moved to RAFT and I've had #3554 open to remove them from cuml and adjust the includes to use the versions from RAFT. Unfortunately, it hasn't been merged yet because it's been waiting on updates to other prims, which I need to update in RAFT again. It would be helpful if any sparse prims changes going forward are made directly in RAFT so that we can get #3554 merged quickly.

@codecov-io
Copy link

Codecov Report

Merging #3575 (ad34ffc) into branch-0.19 (9fa6e17) will increase coverage by 35.80%.
The diff coverage is n/a.

Impacted file tree graph

@@               Coverage Diff                @@
##           branch-0.19    #3575       +/-   ##
================================================
+ Coverage        44.99%   80.80%   +35.80%     
================================================
  Files              223      227        +4     
  Lines            16921    17735      +814     
================================================
+ Hits              7614    14331     +6717     
+ Misses            9307     3404     -5903     
Flag Coverage Δ
dask 45.30% <ø> (+0.31%) ⬆️
non-dask 73.06% <ø> (?)

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

Impacted Files Coverage Δ
python/cuml/dask/solvers/cd.py 100.00% <0.00%> (ø)
python/cuml/common/numba_utils.py 0.00% <0.00%> (ø)
python/cuml/internals/global_settings.py 100.00% <0.00%> (ø)
python/cuml/dask/preprocessing/encoders.py 100.00% <0.00%> (ø)
...n/cuml/experimental/explainer/permutation_shap.pyx 98.82% <0.00%> (ø)
python/cuml/experimental/explainer/base.pyx 67.06% <0.00%> (ø)
python/cuml/experimental/explainer/kernel_shap.pyx 97.75% <0.00%> (ø)
python/cuml/experimental/linear_model/lars.pyx 96.53% <0.00%> (ø)
python/cuml/dask/common/dask_arr_utils.py 95.83% <0.00%> (+0.05%) ⬆️
python/cuml/dask/cluster/dbscan.py 97.29% <0.00%> (+0.07%) ⬆️
... and 129 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 9fa6e17...ad34ffc. Read the comment docs.

@trivialfis
Copy link
Member Author

@cjnolet Thanks, let me look into raft tomorrow. Closing this.

@trivialfis trivialfis closed this Mar 3, 2021
@trivialfis trivialfis deleted the enh-cleanup branch March 3, 2021 19:51
rapids-bot bot pushed a commit to rapidsai/raft that referenced this pull request Apr 10, 2021
* Remove unused code.
* Fix comments.

Ported from rapidsai/cuml#3575 .  Rebased onto 0.20.

Authors:
  - Jiaming Yuan (https://github.com/trivialfis)

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

URL: #203
hlinsen pushed a commit to hlinsen/raft that referenced this pull request Apr 12, 2021
* Remove unused code.
* Fix comments.

Ported from rapidsai/cuml#3575 .  Rebased onto 0.20.

Authors:
  - Jiaming Yuan (https://github.com/trivialfis)

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

URL: rapidsai#203
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Improvement / enhancement to an existing function non-breaking Non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants