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

Reduce the size of the cuml libraries #3702

Merged

Conversation

robertmaynard
Copy link
Contributor

By explicitly telling nvcc's fatbin pass to always compress device code we can ensure that our binaries are the smallest possible size.

See rapidsai/cudf#7583 for additional context.

By explicitly telling nvcc's fatbin pass to always compress device code
we can ensure that our binaries are the smallest possible size.

See rapidsai/cudf#7583 for additional context.
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.

lgtm, it seems that only updating the copyright year in the cpp/CMakeLists.txt file is missing

@dantegd dantegd added feature request New feature or request non-breaking Non-breaking change labels Apr 1, 2021
@dantegd dantegd added this to PR-WIP in v0.19 Release via automation Apr 1, 2021
@robertmaynard
Copy link
Contributor Author

lgtm, it seems that only updating the copyright year in the cpp/CMakeLists.txt file is missing

What should it be 2019-2021?

@robertmaynard
Copy link
Contributor Author

lgtm, it seems that only updating the copyright year in the cpp/CMakeLists.txt file is missing

On second read it looks like the copyright failure is coming from :

python/cuml/experimental/preprocessing/__init__.py:2 Issue: Current year not included in the copyright header

Which I didn't touch.

@dantegd
Copy link
Member

dantegd commented Apr 1, 2021

lgtm, it seems that only updating the copyright year in the cpp/CMakeLists.txt file is missing

What should it be 2019-2021?

The copyright checker seems to have gone crazy in a few PRs (I should've checked the year in the cmakelists file..), checking what's the issue

@JohnZed
Copy link
Contributor

JohnZed commented Apr 2, 2021

rerun tests

@JohnZed
Copy link
Contributor

JohnZed commented Apr 2, 2021

On another PR, rerunning seemed to fix the copyright checker (which was picking up unchanged files)

@JohnZed
Copy link
Contributor

JohnZed commented Apr 2, 2021

rerun tests

Copy link
Member

@trivialfis trivialfis left a comment

Choose a reason for hiding this comment

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

Out of curiosity, how much did you save?

@trivialfis
Copy link
Member

Never mind, I saw the impressive number on the linked issue.

@dantegd dantegd added the 5 - Ready to Merge Testing and reviews complete, ready to merge label Apr 2, 2021
@codecov-io
Copy link

Codecov Report

Merging #3702 (7e350fe) into branch-0.19 (fd9ec89) will increase coverage by 2.23%.
The diff coverage is 100.00%.

Impacted file tree graph

@@               Coverage Diff               @@
##           branch-0.19    #3702      +/-   ##
===============================================
+ Coverage        80.70%   82.94%   +2.23%     
===============================================
  Files              227      227              
  Lines            17615    17726     +111     
===============================================
+ Hits             14217    14702     +485     
+ Misses            3398     3024     -374     
Flag Coverage Δ
dask 46.40% <100.00%> (+1.41%) ⬆️
non-dask 75.02% <100.00%> (+2.11%) ⬆️

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

Impacted Files Coverage Δ
...cuml/_thirdparty/sklearn/preprocessing/__init__.py 100.00% <ø> (ø)
...on/cuml/_thirdparty/sklearn/preprocessing/_data.py 64.40% <ø> (+1.29%) ⬆️
...hirdparty/sklearn/preprocessing/_discretization.py 83.33% <ø> (-0.88%) ⬇️
...l/_thirdparty/sklearn/preprocessing/_imputation.py 85.54% <ø> (+22.74%) ⬆️
python/cuml/_thirdparty/sklearn/utils/extmath.py 56.89% <ø> (ø)
...cuml/_thirdparty/sklearn/utils/skl_dependencies.py 80.00% <ø> (+26.07%) ⬆️
...ython/cuml/_thirdparty/sklearn/utils/validation.py 18.41% <ø> (-4.04%) ⬇️
python/cuml/cluster/__init__.py 100.00% <ø> (ø)
python/cuml/cluster/agglomerative.pyx 96.47% <ø> (ø)
python/cuml/cluster/dbscan.pyx 98.23% <ø> (-1.77%) ⬇️
... and 176 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 4a946e0...7e350fe. Read the comment docs.

@dantegd
Copy link
Member

dantegd commented Apr 5, 2021

@gpucibot merge

@rapids-bot rapids-bot bot merged commit b45e27b into rapidsai:branch-0.19 Apr 5, 2021
v0.19 Release automation moved this from PR-WIP to Done Apr 5, 2021
@robertmaynard robertmaynard deleted the fea/reduce_cuml_binary_size branch April 5, 2021 15:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
5 - Ready to Merge Testing and reviews complete, ready to merge CMake feature request New feature or request non-breaking Non-breaking change
Projects
No open projects
v0.19 Release
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

5 participants