-
Notifications
You must be signed in to change notification settings - Fork 182
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
cuda.parallel: invoke pytest directly rather than via python -m pytest
#3523
cuda.parallel: invoke pytest directly rather than via python -m pytest
#3523
Conversation
Yeah in cuda-python we teach QA about the difference between |
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.
@shwina do you know what change led to the failure? When/why did it stop working? — But I think this is definitely a good change regardless.
I tried to dig into this but I could find nothing significant that has changed. We're not picking up any new versions of our dependencies. I have no idea how we have not seen this before. |
🟨 CI finished in 1h 33m: Pass: 99%/157 | Total: 1d 11h | Avg: 13m 37s | Max: 53m 39s | Hits: 523%/23359
|
Project | |
---|---|
+/- | CCCL Infrastructure |
libcu++ | |
CUB | |
Thrust | |
CUDA Experimental | |
python | |
CCCL C Parallel Library | |
Catch2Helper |
Modifications in project or dependencies?
Project | |
---|---|
+/- | CCCL Infrastructure |
+/- | libcu++ |
+/- | CUB |
+/- | Thrust |
+/- | CUDA Experimental |
+/- | python |
+/- | CCCL C Parallel Library |
+/- | Catch2Helper |
🏃 Runner counts (total jobs: 157)
# | Runner |
---|---|
110 | linux-amd64-cpu16 |
21 | linux-amd64-gpu-v100-latest-1 |
15 | windows-amd64-cpu16 |
10 | linux-arm64-cpu16 |
1 | linux-amd64-gpu-h100-latest-1-testing |
🟩 CI finished in 3h 41m: Pass: 100%/157 | Total: 1d 11h | Avg: 13m 43s | Max: 53m 39s | Hits: 523%/23359
|
Project | |
---|---|
+/- | CCCL Infrastructure |
libcu++ | |
CUB | |
Thrust | |
CUDA Experimental | |
python | |
CCCL C Parallel Library | |
Catch2Helper |
Modifications in project or dependencies?
Project | |
---|---|
+/- | CCCL Infrastructure |
+/- | libcu++ |
+/- | CUB |
+/- | Thrust |
+/- | CUDA Experimental |
+/- | python |
+/- | CCCL C Parallel Library |
+/- | Catch2Helper |
🏃 Runner counts (total jobs: 157)
# | Runner |
---|---|
110 | linux-amd64-cpu16 |
21 | linux-amd64-gpu-v100-latest-1 |
15 | windows-amd64-cpu16 |
10 | linux-arm64-cpu16 |
1 | linux-amd64-gpu-h100-latest-1-testing |
Description
The problem
Recent
cuda.parallel
tests on CI have been failing:Click to see what the failure looks like
Why is this happening?
In
test_python.sh
, we invokepytest
as follows:Note that the working directory for the invocation above is
cuda_parallel
directory, which contains a subdirectory namedcuda
.According to the
pytest
docs, invokingpytest
in this way adds the working directory (cuda_parallel
) to thesys.path
. This means that imports of subpackages likecuda.cccl
will be attempted from thecuda_parallel
directory, rather than the site-packages (which is wherecuda.cccl
lives). This is the problem -cuda_parallel/cuda
doesn't contain thecccl
subpackage.How to fix it?
There are a few possible solutions here:
pytest
directly rather than viapython -m pytest
. This explicitly does not add the current working directory to the Python search path.pytest
(in whatever way) from a directory other thancuda_parallel
- e.g.,cd .. && python -m pytest cuda_parallel/tests
. This way imports ofcuda.cccl
will still be resolved from the site-packages directory.cuda_parallel
directory will not contain acuda
subdirectory.I felt (1) was the simplest solution. I believe that
cuda.core
also needed to recently make a similar change.FYI: @rwgk @leofang
Checklist