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

Eliminate unnecessary includes discovered by cppclean #3564

Merged
merged 10 commits into from
Mar 10, 2021

Conversation

wphicks
Copy link
Contributor

@wphicks wphicks commented Feb 26, 2021

Provide forward declarations where possible to reduce unnecessary includes
Eliminate unneeded or redundant includes

@wphicks wphicks requested a review from a team as a code owner February 26, 2021 18:13
@wphicks
Copy link
Contributor Author

wphicks commented Feb 26, 2021

While this PR does not substantively impact build time, it is the first part of a larger effort to reduce build time. The intention here is not to immediately eliminate all unnecessary includes, just to eliminate those discovered by cppclean. The inter-header dependency graphs before and after are available in the following PDFs (png/jpg did not render well here):

Before:
include_only_sfdp-0_19.pdf

After:
include_only_sfdp.pdf

The difference here is not particularly dramatic, in part because this PR does not tackle the more important cases which cannot be immediately addressed with a forward declaration. Those changes are already in progress for the next PR in this series.

@wphicks wphicks added feature request New feature or request non-breaking Non-breaking change labels Feb 26, 2021
@wphicks wphicks added the 3 - Ready for Review Ready for review by team label Feb 26, 2021
@wphicks
Copy link
Contributor Author

wphicks commented Feb 26, 2021

rerun tests

@wphicks wphicks mentioned this pull request Feb 27, 2021
@wphicks
Copy link
Contributor Author

wphicks commented Mar 8, 2021

rerun tests

@codecov-io
Copy link

Codecov Report

Merging #3564 (a2fddfc) into branch-0.19 (6dfff66) will increase coverage by 1.78%.
The diff coverage is 96.39%.

Impacted file tree graph

@@               Coverage Diff               @@
##           branch-0.19    #3564      +/-   ##
===============================================
+ Coverage        79.21%   81.00%   +1.78%     
===============================================
  Files              226      227       +1     
  Lines            17900    17881      -19     
===============================================
+ Hits             14180    14485     +305     
+ Misses            3720     3396     -324     
Flag Coverage Δ
dask 45.30% <76.15%> (+1.55%) ⬆️
non-dask 73.34% <96.39%> (+1.86%) ⬆️

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

Impacted Files Coverage Δ
python/cuml/dask/cluster/dbscan.py 97.29% <ø> (+0.07%) ⬆️
python/cuml/dask/linear_model/elastic_net.py 100.00% <ø> (ø)
python/cuml/datasets/arima.pyx 97.56% <ø> (ø)
python/cuml/datasets/blobs.py 77.27% <ø> (ø)
python/cuml/datasets/regression.pyx 98.11% <ø> (ø)
python/cuml/decomposition/incremental_pca.py 94.70% <ø> (ø)
python/cuml/feature_extraction/_tfidf.py 94.73% <ø> (+0.11%) ⬆️
python/cuml/metrics/pairwise_distances.pyx 98.83% <ø> (ø)
python/cuml/preprocessing/LabelEncoder.py 94.73% <ø> (ø)
python/cuml/preprocessing/encoders.py 95.10% <ø> (+0.02%) ⬆️
... and 95 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 5e874e9...a2fddfc. Read the comment docs.

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, pre-approving, just wanted to leave one question

cpp/include/cuml/cluster/kmeans_mg.hpp Show resolved Hide resolved
@dantegd dantegd added 4 - Waiting on Author Waiting for author to respond to review 5 - Ready to Merge Testing and reviews complete, ready to merge and removed 3 - Ready for Review Ready for review by team 4 - Waiting on Author Waiting for author to respond to review labels Mar 10, 2021
@dantegd dantegd added this to PR-WIP in v0.19 Release via automation Mar 10, 2021
@dantegd
Copy link
Member

dantegd commented Mar 10, 2021

@gpucibot merge

@rapids-bot rapids-bot bot merged commit 0a7a0fa into rapidsai:branch-0.19 Mar 10, 2021
v0.19 Release automation moved this from PR-WIP to Done Mar 10, 2021
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 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

3 participants