Add missing numba-cuda dependency to the conda recipe#481
Conversation
jameslamb
left a comment
There was a problem hiding this comment.
I'm unsure that this is the right fix, left some clarifying questions.
conda/recipes/ucxx/recipe.yaml
Outdated
| run_constraints: | ||
| - cupy >=9.5.0 | ||
| - numba >=0.59.1,<0.62.0a0 | ||
| - numba-cuda >=0.14.0,<0.15.0a00 |
There was a problem hiding this comment.
cudf is updating its pinning to >=0.18.0,<0.19.0 in rapidsai/cudf#19604, so this will need to be updated again when that merges (to ensure all of RAPIDS can solve together).
Would it be acceptible to do something like >=0.14.0,<0.19.0 here, so environments that end up with both cudf and ucxx aren't broken? And then tighten it to >=0.18.0,<0.19.0 once that cudf PR is merged? Or is that range too large?
Also, should it be a run: (required) dependency not run_constraints: (optional)? I see that for wheels we only have this as an optional dependency in the [test] extra:
ucxx/python/ucxx/pyproject.toml
Line 44 in d2d9468
But it seems looks like it's imported unconditionally:
ucxx/python/ucxx/examples/basic.py
Line 15 in d2d9468
There was a problem hiding this comment.
As a recap, we had to rollback numba-cuda version dependency twice before, immediately this is NOT acceptable IMO. I don’t know if you recall, here's how events unfolded:
Since this is a critical issue, that we’ve been seeing segmentation faults in both RAFT and RMM, I think we first need to go the safe route and make sure everything is back to normal, then we can update the pin to a newer version.
jameslamb
left a comment
There was a problem hiding this comment.
This is looking good, thank you! Merge any time.
|
Thanks @jameslamb and @KyleFromNVIDIA , triggering auto-merge. |
|
/merge |
dependencies.yaml
Outdated
| - pynvml>=12.0.0,<13.0.0a0 | ||
| - output_types: [conda] | ||
| packages: | ||
| - &numba-cuda numba-cuda>=0.14.0,<0.15.0a0 |
There was a problem hiding this comment.
Trying to spell this consistently across RAPIDS to make automated updates easier.
| - &numba-cuda numba-cuda>=0.14.0,<0.15.0a0 | |
| - &numba-cuda-dep numba-cuda>=0.14.0,<0.15.0a0 |
There was a problem hiding this comment.
@KyleFromNVIDIA @bdice you should agree here. See rapidsai/dask-cuda#1531 (comment) for the identical discussion in the opposite direction.
There was a problem hiding this comment.
@bdice Most YAML anchors in dependencies.yaml use underscores and don't have -dep at the end. The existing numba-cuda ones are not consistent with everything else. We should fix the existing ones, not make even more inconsistent ones.
|
I applied my changes (minor renames / typos) since I just saw that |
|
Just for the sake of completeness here about #481 (comment), Kyle agreed we should go forward with the current naming and fix with rapids-reviser later if needed. |
The
numba-cudapackage was made a dependency in #462, but the conda recipe was missed.