Skip to content

Require UCX 1.12.1+ - #638

Merged
rapids-bot[bot] merged 1 commit into
NVIDIA:branch-22.06from
jakirkham:ucx_1_12_1
May 5, 2022
Merged

Require UCX 1.12.1+#638
rapids-bot[bot] merged 1 commit into
NVIDIA:branch-22.06from
jakirkham:ucx_1_12_1

Conversation

@jakirkham

Copy link
Copy Markdown
Contributor

@jakirkham
jakirkham requested a review from a team as a code owner May 4, 2022 03:35
@cjnolet cjnolet added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels May 4, 2022
@@ -0,0 +1,2 @@
ucx_version:
- "1.12.1"

Copy link
Copy Markdown
Contributor

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.1

Copy link
Copy Markdown
Contributor Author

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.

Copy link
Copy Markdown
Contributor

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_compatible for the runtime dependency?

Copy link
Copy Markdown
Contributor Author

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 ucx constraints 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.

- libraft-headers {{ version }}
- nccl>=2.9.9
- rmm {{ minor_version }}
- ucx >={{ ucx_version }}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should remove the >= here

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please see comment above

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That said, we can probably drop this line as the feedstock has run_exports, which covers adding and pinning this.

@cjnolet

cjnolet commented May 5, 2022

Copy link
Copy Markdown
Contributor

@gpucibot merge

@rapids-bot
rapids-bot Bot merged commit 24102b8 into NVIDIA:branch-22.06 May 5, 2022
@jakirkham
jakirkham deleted the ucx_1_12_1 branch May 5, 2022 21:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

improvement Improvement / enhancement to an existing function non-breaking Non-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants