-
Notifications
You must be signed in to change notification settings - Fork 532
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
CI test speed improvement #3851
CI test speed improvement #3851
Conversation
Summary of top 40 tests times in CI before/after those improvements Before
After
|
@lowener there seems to still be one particular test that is on the slower side, do you think it could be tackled in this PR?
|
I moved this test to a stress test and introduced a unit test that is much faster (with parameters |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Had one quick question
@@ -230,7 +230,7 @@ def test_umap_fit_transform_trust(name, target_metric): | |||
data = wine.data | |||
labels = wine.target | |||
else: | |||
data, labels = make_blobs(n_samples=5000, n_features=10, | |||
data, labels = make_blobs(n_samples=500, n_features=10, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just had one quick question, have you seen any issues of reducing the data so much? i.e. it would be nice to run the test a number of times to be sure no flakiness is being introduced
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good point
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I saw no failures with 100 retries of this test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, but agreed we should run like 100 times in a row (you can use https://pypi.org/project/pytest-repeat/) to make sure they are not flaky.
@@ -230,7 +230,7 @@ def test_umap_fit_transform_trust(name, target_metric): | |||
data = wine.data | |||
labels = wine.target | |||
else: | |||
data, labels = make_blobs(n_samples=5000, n_features=10, | |||
data, labels = make_blobs(n_samples=500, n_features=10, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good point
rerun tests |
@gpucibot merge |
Codecov Report
@@ Coverage Diff @@
## branch-21.06 #3851 +/- ##
===============================================
Coverage ? 85.55%
===============================================
Files ? 226
Lines ? 17357
Branches ? 0
===============================================
Hits ? 14850
Misses ? 2507
Partials ? 0
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report at Codecov.
|
Closes rapidsai#3786. Some redundant combinations are removed and I reduced the number of samples on some unit tests that are run very often. `test_umap.py::test_umap_fit_transform_trust` is the test that takes the most time in CI. Local speed-up: - `test_umap.py`: From `273s` to `79s` - `test_dbscan.py`: From `14s` to `2s` - `dask/test_nearest_neighbors.py::test_compare_skl`: From `113s` to `31s` In total, the expected saved time is `288s` (~5 min) on the pipeline locally. Authors: - Micka (https://github.com/lowener) Approvers: - John Zedlewski (https://github.com/JohnZed) - Dante Gama Dessavre (https://github.com/dantegd) URL: rapidsai#3851
Closes #3786.
Some redundant combinations are removed and I reduced the number of samples on some unit tests that are run very often.
test_umap.py::test_umap_fit_transform_trust
is the test that takes the most time in CI.Local speed-up:
test_umap.py
: From273s
to79s
test_dbscan.py
: From14s
to2s
dask/test_nearest_neighbors.py::test_compare_skl
: From113s
to31s
In total, the expected saved time is
288s
(~5 min) on the pipeline locally.