Update to UCX v1.14.0 and configure/build for release #111

pentschev · 2022-10-17T17:17:55Z

Checklist

Used a personal fork of the feedstock to propose changes
Bumped the build number (if the version is unchanged)
Reset the build number to 0 (if the version changed)
Re-rendered with the latest conda-smithy (Use the phrase @conda-forge-admin, please rerender in a comment in this PR for automated rerendering)
Ensured the license file is being packaged.

Changes:

Upgrade to UCX 1.14.0
Add libnuma dependency from conda-forge
Switch to release builds
Move to single build compatible for CPU-only and CUDA
Remove verbs support

Closes #26
Closes #66
Closes #102
Closes #110
Closes #112
Closes #115

conda-forge-linter · 2022-10-17T17:17:59Z

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

pentschev · 2022-10-17T17:18:08Z

@conda-forge-admin, please rerender

pentschev · 2022-10-17T17:19:05Z

cc @leofang @jakirkham

github-actions · 2022-10-17T17:19:37Z

Hi! This is the friendly automated conda-forge-webservice.

I tried to rerender for you, but it looks like there was nothing to do.

This message was generated by GitHub actions workflow run https://github.com/conda-forge/ucx-split-feedstock/actions/runs/3267176097.

leofang · 2022-10-17T17:26:07Z

Thanks, Peter! LGTM but I'd like to have @jakirkham or someone else to take another look 🙂

jakirkham · 2022-10-18T08:44:41Z

Does this result in ABI changes? In particular were there debug symbols that would have been linked against before that are now removed?

conda-forge-webservices · 2023-03-14T12:34:57Z

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

pentschev · 2023-03-14T12:37:25Z

As per our previous discussion, we wanted to wait for the change to build for release until the new UCX release was out, now v1.14.0 has been released and I've updated this PR to use v1.14.0.

@jakirkham @leofang would you mind taking a look?

pentschev · 2023-03-14T15:06:30Z

It seems that the CentOS 6 builds fail with UCX 1.14 because IBV_ACCESS_ON_DEMAND is missing as it was previously defined in https://github.com/openucx/ucx/blob/c0aa76d0c121445b8665dbb16e1577ef5ebc6200/src/uct/ib/base/ib_verbs.h#L160-L169 and now is expected to be defined as part of rdma-core. Similar to

ucx-split-feedstock/recipe/install_ucx.sh

Line 11 in dd05d5a

# --with-rdmacm requires rdma-core v23+, while CentOS 7 only offers v22.

, I believe we should be building against rdma-core v28 for MOFED 5.x compatibility, which we can't because CentOS 7 goes only up to v22. My suggestion would be to remove --with-verbs entirely until we can guarantee rdma-core v28 or higher is available as part of the conda-forge packaging process.

pentschev · 2023-03-14T15:09:30Z

To be clear, MOFED 5.x is now the only tested UCX version, I know MOFED 4.x was still "supported" by UCX until last year or so but not actively tested, thus I was constantly finding bugs. As a result of that we decided to stop supporting MOFED 4.x entirely for RAPIDS, both internally at NVIDIA and for customers. I would recommend the same be done here, as MOFED 4.x support (rdma-core < v28) is going to be hit or miss.

jakirkham · 2023-03-14T18:47:08Z

Thanks Peter! 🙏

Do we also want to add libnuma ( #112 )?

Wonder if we can also combine these into a single build for both CPU and GPU (just moving cudatoolkit to run_constrained). This would resolve issue ( #66 ) and also issues ( #26 ), ( #66 ), ( #102 ) & ( #115 )

cc @kkraus14 (for awareness)

kkraus14 · 2023-03-14T18:53:49Z

recipe/meta.yaml


 build:
  skip: true  # [not linux]
-  number: 0
+  number: 1


If we're bumping the version we don't need to bump the build number

This was an oversight, this PR was only changing to release with no bump on the UCX version, so this was left by accident. We also have the number variable above, is there a reason we can't use it here @jakirkham ? It is currently only used by outputs.name=ucx.build.number, wondering whether we can use for both.

Fixed in 50a5dc2 .

Yeah we should use the top-level number and template it everywhere else

jakirkham · 2023-03-14T19:01:49Z

A separate question would be other we want to backport the CUDA 12 fixes ourselves ( #116 (comment) ) to ensure they are in 1.14.0

kkraus14 · 2023-03-14T21:17:30Z

To be clear, MOFED 5.x is now the only tested UCX version, I know MOFED 4.x was still "supported" by UCX until last year or so but not actively tested, thus I was constantly finding bugs. As a result of that we decided to stop supporting MOFED 4.x entirely for RAPIDS, both internally at NVIDIA and for customers. I would recommend the same be done here, as MOFED 4.x support (rdma-core < v28) is going to be hit or miss.

Now that there's a virtual package plugin system for conda we in theory could have a virtual package for MOFED version and constrain UCX packages based on that MOFED version if desired.

Is there a path forward for an updated rdma-core CDT or are we limited to CentOS 7 for CDTs?

jakirkham · 2023-03-16T19:11:44Z

It would need to be provided by the user somehow

Anyways if this is not helpful, we can skip it. Was just thinking the option might be useful for local builds

pentschev · 2023-03-16T19:15:07Z

It would need to be provided by the user somehow

Anyways if this is not helpful, we can skip it. Was just thinking the option might be useful for local builds

I agree this would be a good idea if it could be provided by the user, but unfortunately I don't think this is the case. Sometime ago I even went through the rabbit hole of trying to link against the system's rdma-core/MOFED, but due to the way conda sets sysroot that was impossible, or at least cumbersome enough to prevent that from working after spending about a day on it. With that said, I think this isn't helpful, but if someone can point to a path forward I do not oppose to doing that.

jakirkham · 2023-03-16T19:36:52Z

Gotcha. If we think it is too complicated, let's punt.

pentschev · 2023-03-16T19:38:15Z

Gotcha. If we think it is too complicated, let's punt.

Agreed. In that case, I think this PR is ready for review and merging (if nothing needs changing). Would you mind taking another look, John?

recipe/meta.yaml

jakirkham

Thanks Peter! 🙏

Looks good. One minor suggestion below.

recipe/meta.yaml

Co-authored-by: jakirkham <jakirkham@gmail.com>

jakirkham · 2023-03-16T22:35:47Z

Thanks Peter! 🙏

LGTM

Will let this sit for a bit to see if anyone else has feedback. Planning on merging EOD tomorrow if no comments

jakirkham · 2023-03-18T00:18:10Z

Thanks Peter for the PR and Keith for help reviewing! 🙏

conda-forge/ucx-split-feedstock#111 was merged and UCX v1.14.0 is now available on conda-forge which we should support. Authors: - Peter Andreas Entschev (https://github.com/pentschev) Approvers: - Ray Douglass (https://github.com/raydouglass) URL: #935

UCX 1.14.0 was recently released and conda-forge package was updated in conda-forge/ucx-split-feedstock#111 with several packaging improvements. Relax the pin to allow installing UCX v1.14.x as well. Authors: - Peter Andreas Entschev (https://github.com/pentschev) - AJ Schmidt (https://github.com/ajschmidt8) - Corey J. Nolet (https://github.com/cjnolet) Approvers: - AJ Schmidt (https://github.com/ajschmidt8) URL: #1366

jakirkham · 2023-04-12T18:59:38Z

recipe/meta.yaml

    requirements:
      build:
        - {{ compiler("c") }}
        - {{ compiler("cxx") }}
        - {{ compiler("cuda") }}         # [cuda_compiler_version != "None"]
        - {{ cdt("libnl") }}             # [cdt_name == "cos6"]
        - {{ cdt("libnl3") }}            # [cdt_name == "cos7"]
-        - {{ cdt("libibcm-devel") }}     # [cdt_name == "cos6"]
-        - {{ cdt("libibverbs-devel") }}  # [cdt_name == "cos6"]
-        - {{ cdt("librdmacm-devel") }}   # [cdt_name == "cos6"]
        - {{ cdt("numactl-devel") }}


Just noticed we still have this CDT. Though we added libnuma below. Should we drop the CDT then?

Addressing in PR ( #118 )

Configure/build UCX for release

9e35b3f

pentschev requested review from Akshay-Venkatesh, TomAugspurger, jakirkham, leofang, matthiasdiener and quasiben as code owners October 17, 2022 17:17

pentschev mentioned this pull request Oct 17, 2022

Disable debugging features #110

Closed

pentschev mentioned this pull request Jan 27, 2023

ucx should depend on libnuma #112

Closed

1 task

pentschev added 3 commits March 14, 2023 05:31

Merge remote-tracking branch 'upstream/main' into use-configure-release

a28f3da

Update to UCX v1.14.0

9aed59a

Update build number

8da0d81

pentschev changed the title ~~Configure/build UCX for release~~ Update to UCX v1.14.0 and configure/build for release Mar 14, 2023

Fix sha256

f870eee

kkraus14 reviewed Mar 14, 2023

View reviewed changes

pentschev added 2 commits March 15, 2023 01:18

Fix build number

50a5dc2

Add libnuma dependency

c828466

kkraus14 reviewed Mar 16, 2023

View reviewed changes

recipe/meta.yaml Outdated Show resolved Hide resolved

Move libnuma to host section

f161ce9

kkraus14 approved these changes Mar 16, 2023

View reviewed changes

jakirkham reviewed Mar 16, 2023

View reviewed changes

recipe/meta.yaml Outdated Show resolved Hide resolved

Remove unnecessary libnuma from run section

ef0cfe2

Co-authored-by: jakirkham <jakirkham@gmail.com>

jakirkham approved these changes Mar 16, 2023

View reviewed changes

kkraus14 mentioned this pull request Mar 17, 2023

Package request: rdma-core libraries conda-forge/staged-recipes#22306

Closed

2 tasks

jakirkham merged commit 09499c8 into conda-forge:main Mar 18, 2023

This was referenced Mar 18, 2023

add ucx migration conda-forge/conda-forge-pinning-feedstock#4256

Merged

Reinstate ucx support conda-forge/arrow-cpp-feedstock#1011

Merged

Remove ucx dependency for now conda-forge/arrow-cpp-feedstock#963

Merged

pentschev mentioned this pull request Mar 20, 2023

Update UCX pin to allow 1.14 rapidsai/ucx-py#935

Merged

pentschev mentioned this pull request Mar 22, 2023

Relax UCX pin to allow 1.14 rapidsai/raft#1366

Merged

jakirkham mentioned this pull request Mar 25, 2023

UCX 1.14.0 packages make CUDA optional conda-forge/openmpi-feedstock#119

Closed

pentschev deleted the use-configure-release branch April 12, 2023 07:37

jakirkham reviewed Apr 12, 2023

View reviewed changes

jakirkham mentioned this pull request Apr 12, 2023

Drop NUMA CDT #118

Merged

5 tasks

jakirkham mentioned this pull request May 22, 2023

Building with CMA #72

Closed

pentschev mentioned this pull request Jun 26, 2023

Add CUDA 12.0 build matrix rapidsai/ucx-py#967

Merged

This was referenced Mar 6, 2024

Remove libnuma installations rapidsai/miniforge-cuda#22

Closed

Require UCX 1.14.0+ rapidsai/build-planning#27

Closed

Drop unused libnl and libnuma & always build rdma & ibverbs support #160

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update to UCX v1.14.0 and configure/build for release #111

Update to UCX v1.14.0 and configure/build for release #111

pentschev commented Oct 17, 2022 •

edited by jakirkham

Loading

conda-forge-linter commented Oct 17, 2022

pentschev commented Oct 17, 2022

pentschev commented Oct 17, 2022

github-actions bot commented Oct 17, 2022

leofang commented Oct 17, 2022

jakirkham commented Oct 18, 2022

conda-forge-webservices bot commented Mar 14, 2023

pentschev commented Mar 14, 2023

pentschev commented Mar 14, 2023

pentschev commented Mar 14, 2023

jakirkham commented Mar 14, 2023

kkraus14 Mar 14, 2023

pentschev Mar 15, 2023

pentschev Mar 15, 2023

jakirkham Mar 15, 2023

jakirkham commented Mar 14, 2023

kkraus14 commented Mar 14, 2023

jakirkham commented Mar 16, 2023

pentschev commented Mar 16, 2023

jakirkham commented Mar 16, 2023

pentschev commented Mar 16, 2023

jakirkham left a comment

jakirkham commented Mar 16, 2023

jakirkham commented Mar 18, 2023

jakirkham Apr 12, 2023

jakirkham Apr 12, 2023

Update to UCX v1.14.0 and configure/build for release #111

Update to UCX v1.14.0 and configure/build for release #111

Conversation

pentschev commented Oct 17, 2022 • edited by jakirkham Loading

conda-forge-linter commented Oct 17, 2022

pentschev commented Oct 17, 2022

pentschev commented Oct 17, 2022

github-actions bot commented Oct 17, 2022

leofang commented Oct 17, 2022

jakirkham commented Oct 18, 2022

conda-forge-webservices bot commented Mar 14, 2023

pentschev commented Mar 14, 2023

pentschev commented Mar 14, 2023

pentschev commented Mar 14, 2023

jakirkham commented Mar 14, 2023

kkraus14 Mar 14, 2023

Choose a reason for hiding this comment

pentschev Mar 15, 2023

Choose a reason for hiding this comment

pentschev Mar 15, 2023

Choose a reason for hiding this comment

jakirkham Mar 15, 2023

Choose a reason for hiding this comment

jakirkham commented Mar 14, 2023

kkraus14 commented Mar 14, 2023

jakirkham commented Mar 16, 2023

pentschev commented Mar 16, 2023

jakirkham commented Mar 16, 2023

pentschev commented Mar 16, 2023

jakirkham left a comment

Choose a reason for hiding this comment

jakirkham commented Mar 16, 2023

jakirkham commented Mar 18, 2023

jakirkham Apr 12, 2023

Choose a reason for hiding this comment

jakirkham Apr 12, 2023

Choose a reason for hiding this comment

pentschev commented Oct 17, 2022 •

edited by jakirkham

Loading