-
Notifications
You must be signed in to change notification settings - Fork 249
Require UCX 1.12.1+ #638
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
Require UCX 1.12.1+ #638
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| ucx_version: | ||
| - "1.12.1" | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -34,6 +34,7 @@ requirements: | |
| - cudatoolkit {{ cuda_version }}.* | ||
| - cuda-python >=11.5,<12.0 | ||
| - nccl>=2.9.9 | ||
| - ucx {{ ucx_version }} | ||
| - ucx-py {{ ucx_py_version }} | ||
| - ucx-proc=*=gpu | ||
| run: | ||
|
|
@@ -42,6 +43,7 @@ requirements: | |
| - libraft-headers {{ version }} | ||
| - nccl>=2.9.9 | ||
| - rmm {{ minor_version }} | ||
| - ucx >={{ ucx_version }} | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we should remove the
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please see comment above
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That said, we can probably drop this line as the feedstock has |
||
| - ucx-py {{ ucx_py_version }} | ||
| - ucx-proc=*=gpu | ||
| - dask>=2022.03.0 | ||
|
|
||
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 think this should be
>=1.12.1There 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.
The intent here is to ensure we have a fixed version at build time so that this sets a minimum version at runtime.
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.
Oh, I thought we wanted to build using the latest version available. What's the difference between doing this and using
pin_compatiblefor the runtime dependency?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.
We are using the latest version since 1.12.1 is latest. However we may want more flexibility if a later 1.12.x breaks something and we need to pin an older version in the metapackage. Also we may want to control which version we build against to make it easier to align with other packages that have
ucxconstraints as well.FWIW am proposing something similar in conda-forge ( conda-forge/conda-forge-pinning-feedstock#2847 ) though maybe we can wait to see what others think about that and update if needed.