cuvs-bench-cpu: avoid 'mkl' dependency on aarch64#750
cuvs-bench-cpu: avoid 'mkl' dependency on aarch64#750rapids-bot[bot] merged 4 commits intorapidsai:branch-25.04from jameslamb:fix/cuvs-bench-metadata
Conversation
This comment was marked as resolved.
This comment was marked as resolved.
|
|
||
| # (3) export data | ||
| python -m cuvs_bench.data_export --dataset deep-image-96-inner | ||
| python -m cuvs_bench.run.data_export --dataset deep-image-96-inner |
There was a problem hiding this comment.
I think these tests indirectly caught a mistake in the docs! When I set up the import tests, I did something like this to find which modules were referenced in documentation:
git grep -E 'cuvs_bench\.'That showed me these references to cuvs_bench.data_export.
However, the import tests failed:
Traceback (most recent call last):
File "/opt/conda/conda-bld/test_tmp/run_test.py", line 5, in <module>
import cuvs_bench.data_export
ModuleNotFoundError: No module named 'cuvs_bench.data_export'
...
CondaBuildUserError: TESTS FAILED: cuvs-bench-cpu-25.04.00a95-py310_250305_g0208914_95.conda
...
import: 'cuvs_bench'
import: 'cuvs_bench.data_export'
And it really does look to me like this needs to be cuvs_bench.run.data_export... there's no top-level __init__.py doing remapping in https://github.com/rapidsai/cuvs/tree/branch-25.04/python/cuvs_bench/cuvs_bench, and data_export is down in cuvs_bench/run/: https://github.com/rapidsai/cuvs/blob/89b03493b487910d2125fde6680590adde8e2a95/python/cuvs_bench/cuvs_bench/run/data_export.py
There was a problem hiding this comment.
It is now an option that is true by default. However, for this PR please just update the line to cuvs_bench.run --data-export
There was a problem hiding this comment.
Ah ok, sure! Thanks for the explanation.
Just pushed 669e169 with that change.
gforsyth
left a comment
There was a problem hiding this comment.
Nice catch on the import, @jameslamb ! Yay tests!
divyegala
left a comment
There was a problem hiding this comment.
Approved pending doc update
|
|
||
| # (3) export data | ||
| python -m cuvs_bench.data_export --dataset deep-image-96-inner | ||
| python -m cuvs_bench.run.data_export --dataset deep-image-96-inner |
There was a problem hiding this comment.
It is now an option that is true by default. However, for this PR please just update the line to cuvs_bench.run --data-export
|
Saw one test failure for wheels: Strongly suspect that that's a flaky test. I just triggered a re-run. |
|
/merge |
Fixes #6403 This project publishes a conda package, `cuml-cpu`, which does what it sounds like... allows the use of cuML on systems without a GPU. This proposes some updates to packaging for `cuml-cpu`: * fixes importing in CPU-only environment (broken in 25.04, see #6403) * enables import tests during conda builds, to reduce the risk of such issues going undetected in the future ## Notes for Reviewers ### Why all these changes in Python code? See some of the challenges I faced documented in #6400 (comment). In short, `import cuml` when it was installed via `cuml-cpu` will break at import time whenever modules imported with `cuml.internals.safe_imports.gpu_only_import()` are used in any of the following ways: * type hints * decorators * any other module-level direct use Like this: ```text cuml.internals.safe_imports.UnavailableError: cudf is not installed in non GPU-enabled installations ``` ### How long has this been broken? What's the root cause? It seems like something changed within 25.04... earlier versions of cuML are not affected by these issues: #6403 (comment) I don't know what the root cause is. Maybe some changes to `cuml`'s top-level imports in 25.04 is now pulling in the modules with these problems at runtime, when previously it wasn't? I'm really not sure. ### Benefits of these Changes This adds a bit of test coverage in CI, minimally verifying that `cuml-cpu` is installable and that `import cuml` works in an environment without a GPU. Inspired by: * similar changes in `cuvs`: rapidsai/cuvs#750 * this conversation I recently had with @betatim : rapidsai/cuvs#743 (comment) ### How I tested this Saw stuff like this in `conda-python-build` jobs, confirming that the import tests were running and passing: ```text BUILD START: ['cuml-cpu-25.04.00a137-py310_250312_g153b21870_137.conda'] ... import: 'cuml' ... Resource usage statistics from testing cuml-cpu: ... Time elapsed: 0:00:10.0 ... TEST END: /tmp/conda-bld-output/linux-64/cuml-cpu-25.04.00a137-py310_250312_g153b21870_137.conda ``` Authors: - James Lamb (https://github.com/jameslamb) Approvers: - Gil Forsyth (https://github.com/gforsyth) - Simon Adorf (https://github.com/csadorf) - Tim Head (https://github.com/betatim) URL: #6400
Fixes rapidsai/docker#739 rapidsai#260 introduced a runtime dependency on `mkl` for the `cuvs-bench-cpu` conda package. There are not aarch64 packages for `mkl` on conda-forge, so this makes `cuvs-bench-cpu` impossible to install on aarch64. This fixes that, by applying the same "only add on x86_64" guard used for `mkl` everywhere else in this project, e.g. like this: https://github.com/rapidsai/cuvs/blob/89b03493b487910d2125fde6680590adde8e2a95/conda/recipes/cuvs-bench-cpu/meta.yaml#L51 It also proposes adding import tests to the `cuvs-bench-cpu` conda recipe, so issues like this can be caught in CI in the future. ## Notes for Reviewers I searched for references like this ```shell git grep -i mkl ``` ### How I tested this Saw lines like this in `conda-python-build` logs: ```text BUILD START: ['cuvs-bench-cpu-25.04.00a96-py312_250305_g94340bc_96.conda'] ... TEST START: /tmp/conda-bld-output/linux-aarch64/cuvs-bench-cpu-25.04.00a96-py312_250305_g94340bc_96.conda ... import: 'cuvs_bench' import: 'cuvs_bench.generate_groundtruth' import: 'cuvs_bench.get_dataset' import: 'cuvs_bench.plot' import: 'cuvs_bench.run' import: 'cuvs_bench.run.data_export' import: 'cuvs_bench.split_groundtruth' import: 'cuvs_bench' import: 'cuvs_bench.generate_groundtruth' import: 'cuvs_bench.get_dataset' import: 'cuvs_bench.plot' import: 'cuvs_bench.run' import: 'cuvs_bench.run.data_export' import: 'cuvs_bench.split_groundtruth' ... TEST END: /tmp/conda-bld-output/linux-aarch64/cuvs-bench-cpu-25.04.00a96-py312_250305_g94340bc_96.conda ``` ([build link](https://github.com/rapidsai/cuvs/actions/runs/13687659537/job/38276160494?pr=750#step:9:5961)) Authors: - James Lamb (https://github.com/jameslamb) Approvers: - Gil Forsyth (https://github.com/gforsyth) - Divye Gala (https://github.com/divyegala) URL: rapidsai#750
Fixes rapidsai/docker#739
#260 introduced a runtime dependency on
mklfor thecuvs-bench-cpuconda package. There are not aarch64 packages formklon conda-forge, so this makescuvs-bench-cpuimpossible to install on aarch64.This fixes that, by applying the same "only add on x86_64" guard used for
mkleverywhere else in this project, e.g. like this:cuvs/conda/recipes/cuvs-bench-cpu/meta.yaml
Line 51 in 89b0349
It also proposes adding import tests to the
cuvs-bench-cpuconda recipe, so issues like this can be caught in CI in the future.Notes for Reviewers
I searched for references like this
How I tested this
Saw lines like this in
conda-python-buildlogs:(build link)