-
Notifications
You must be signed in to change notification settings - Fork 200
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
Build benchmarks in RMM CI #941
Conversation
Following what is done in other repos, you might need to update the conda build scripts to also build tests if Project Flash is enabled. See https://github.com/rapidsai/cudf/blob/branch-22.02/conda/recipes/libcudf/build.sh as example |
@Ethyling that seems out of scope for this PR. This PR is about building benchmarks, not tests. Also I'm out sick and we are in burndown. so could you approve the present PR and just file an issue for this detail? |
I think this is required to fix this CI issue: Before this PR, the default value of
This change only applies for non Project Flash builds: https://github.com/rapidsai/rmm/pull/941/files#diff-285f239638b8b5c5499ef5fda2e3db69adda7045e81556b4f2df478e99ab6263R71 But @harrism WDYT? |
Ah, I see now. Unfortunately I'm out sick so will have to wait until next week unless you want to commit a fix. |
Signed-off-by: Jordan Jacobelli <[email protected]>
52edeab
to
b1a29d8
Compare
Signed-off-by: Jordan Jacobelli <[email protected]>
Signed-off-by: Jordan Jacobelli <[email protected]>
@harrism The tests and benchmarks are now building on PRs, but we only run the tests currently. Should we also run the benchmarks? |
No, don't run the benchmarks. Benchmarking is a separate effort and would likely not be done on every PR. We just need to make sure PRs don't break the benchmark build. |
Thanks for the help, @Ethyling ! |
Is this ready to merge now? Still needs ops approval. |
We are good to merge |
@gpucibot merge |
Fixes #940
Adds options to build tests and benchmarks to build.sh, and uses them in ci/gpu/build.sh