BUILD: Help cmake find UCX on system #7096

zasdfgbnm-nvidia · 2021-07-13T17:59:15Z

This PR makes the installation script writes cmake config files to ${prefix}/lib/cmake/ucx so that cmake projects can easily find UCX installed on the system. PyTorch uses cmake, so this will be helpful for PyTorch to integrate the ucx/ucc backend to upstream.

This PR is tested by: https://github.com/zasdfgbnm/cmake-ucx-ucc, please let me know if you want cmake testes to be included in this repository as well.

Related: openucx/ucc#255

swx-jenkins3 · 2021-07-13T17:59:33Z

Can one of the admins verify this patch?

yosefe

shall we add it to rpm/deb files as well?

cmake/ucx-config-version.cmake.in

yosefe · 2021-07-14T23:32:44Z

cmake/ucx-config.cmake.in

+# Copyright (C) NVIDIA Corporation. 2021.  ALL RIGHTS RESERVED.
+#
+
+set(UCX_LIBRARIES "@prefix@/lib")


@libdir@ (it could be lib64)

@libdir@ will be replaced by ${exec_prefix}/lib, but in the context of cmake, exec_prefix is not defined. But I can do something like

set(prefix "@prefix@") set(exec_prefix "@exec_prefix@") set_target_properties(ucx::ucs PROPERTIES IMPORTED_LOCATION "@libdir@/libucs.so" INTERFACE_INCLUDE_DIRECTORIES "@includedir@" )

yosefe · 2021-07-14T23:33:26Z

cmake/ucx-targets.cmake.in

+add_library(ucx::ucs SHARED IMPORTED)
+
+set_target_properties(ucx::ucs PROPERTIES
+  IMPORTED_LOCATION "@prefix@/lib/libucs.so"


@libdir@ instead of @prefix/lib in all places

yosefe · 2021-07-14T23:34:58Z

cmake/ucx-targets.cmake.in

+
+set_target_properties(ucx::ucp PROPERTIES
+  IMPORTED_LOCATION "@prefix@/lib/libucp.so"
+  INTERFACE_INCLUDE_DIRECTORIES "@prefix@/include"


I think there is also @includedir

zasdfgbnm · 2021-07-15T15:49:14Z

With this PR it is automatically added to the deb package. I don't know about rpm, but it should be automatic as well.

cmake/ucx-config-version.cmake.in

yosefe · 2021-07-15T16:10:16Z

cmake/ucx-config.cmake.in

+set(prefix "@prefix@")
+set(exec_prefix "@exec_prefix@")


since we include ucx-targets.cmake, can avoid re-defining prefix and exec_prefix?
BTW, why need to separate to 2 cmake files (ucx-config and ucx-targets)?

I have updated this PR to avoid redefinition. There are two separate files because other projects do so. I guess this is how cmake generates these files (See https://cmake.org/cmake/help/git-stage/guide/importing-exporting/index.html#exporting-targets-from-the-build-tree). But I don't know if this separation is required or not.

yosefe · 2021-07-15T16:11:56Z

cmake/Makefile.am

+cmake_DATA =                       \
+    ucx-targets.cmake              \
+    ucx-config.cmake               \
+    ucx-config-version.cmake


pls remove horizontal spaces before \

yosefe · 2021-07-15T16:16:36Z

With this PR it is automatically added to the deb package. I don't know about rpm, but it should be automatic as well.
https://dev.azure.com/ucfconsort/ucx/_build/results?buildId=21404&view=logs&jobId=388a871c-1c79-55fb-9b87-030460a06453&j=8cc70074-80e3-506d-e80d-a542b02430e7&t=da02301d-00be-54d1-aa97-9bf0b310a3f6

There is build error from RPM:

Checking for unpackaged file(s): /usr/lib/rpm/check-files /home/swx-azure-svc_azpcontainer/21404-20210715.10/build/_rpm
error: Installed (but unpackaged) file(s) found:
   /usr/lib64/cmake/ucx/ucx-config-version.cmake
   /usr/lib64/cmake/ucx/ucx-config.cmake
   /usr/lib64/cmake/ucx/ucx-targets.cmake
    Installed (but unpackaged) file(s) found:
   /usr/lib64/cmake/ucx/ucx-config-version.cmake
   /usr/lib64/cmake/ucx/ucx-config.cmake
   /usr/lib64/cmake/ucx/ucx-targets.cmake

zasdfgbnm · 2021-07-15T18:34:44Z

@yosefe The rpm problem should be fixed now.

yosefe · 2021-07-15T19:15:34Z

@zasdfgbnm-nvidia BTW, maybe it's worth adding a small test for cmake?
e.g in buildlib/builds.sh add a step which:

make install (into build-test/installs)
add CMakeLists.txt with find_package(ucx)
run cmake in some dummy folder and make sure UCX is found (from cmake output)?
this will also serve as demo of how to use UCX with Cmake

zasdfgbnm · 2021-07-15T19:19:30Z

@yosefe Yes, I think it is worth it. I will copy-paste https://github.com/zasdfgbnm/cmake-ucx-ucc/tree/master/Tests/ucx to this repository and take a look at how to add it to CI here.

yosefe · 2021-07-15T19:26:48Z

@zasdfgbnm one option is to copy-paste this to examples/cmake and run it from buildlib/builds.sh (which is part of "build-on-all-containers" stage)

yosefe · 2021-07-15T19:46:47Z

buildlib/tools/builds.sh

+	cd /tmp/cmake-ucx
+	cmake --build .


need to return to previous directory after the test

yosefe · 2021-07-15T19:48:26Z

examples/cmake/test_ucp.c

+    ucp_context_h ucp_context;
+    ucp_params_t ucp_params;
+    memset(&ucp_params, 0, sizeof(ucp_params));
+    ucp_params.field_mask = UCP_PARAM_FIELD_FEATURES;
+    ucp_params.features = UCP_FEATURE_AM;
+    ucs_status_t status = ucp_init(&ucp_params, NULL, &ucp_context);
+    assert(status == UCS_OK);


{ ucp_params_t ucp_params = {}; ucp_context_h ucp_context; ucs_status_t status; ucp_params.field_mask = UCP_PARAM_FIELD_FEATURES; ucp_params.features = UCP_FEATURE_AM; status = ucp_init(&ucp_params, NULL, &ucp_context); assert(status == UCS_OK); ucp_cleanup(ucp_context) return 0; }

yosefe · 2021-07-15T19:49:06Z

examples/cmake/test_uct.c

+    assert(UCS_OK == status);
+
+    /* Create a worker object */
+    status = uct_worker_create(async, UCS_THREAD_MODE_SINGLE, &worker);


pls destroy the created objects and return 0

yosefe · 2021-07-15T19:49:26Z

examples/cmake/CMakeLists.txt

@@ -0,0 +1,11 @@
+cmake_minimum_required(VERSION 3.5)


need to add NVIDIA copyright

yosefe · 2021-07-15T19:49:30Z

examples/cmake/test_ucp.c

@@ -0,0 +1,14 @@
+#include <ucp/api/ucp.h>


need to add NVIDIA copyright

yosefe · 2021-07-15T19:49:33Z

examples/cmake/test_uct.c

@@ -0,0 +1,16 @@
+#include <uct/api/uct.h>


need to add NVIDIA copyright

yosefe · 2021-07-15T20:58:19Z

+ mkdir -p /tmp/cmake-ucx
+ cmake -DUCX_DIR=/home/swx-azure-svc_azpcontainer/21448-20210715.19/build/install -S examples/cmake -B /tmp/cmake-ucx
CMake Error: The source directory "/tmp/cmake-ucx" does not appear to contain CMakeLists.txt.
Specify --help for usage, or press the help button on the CMake GUI.

zasdfgbnm · 2021-07-15T22:24:00Z

The cmake in CI is super old (2.8), the latest is 3.21, that's why it fails with

The source directory "/tmp/cmake-ucx" does not appear to contain CMakeLists.txt.

I have changed the

cmake_minimum_required(VERSION 3.5)

in the example to

cmake_minimum_required(VERSION 2.8)

Let's see if it works now. If not, I recommend upgrading the cmake, instead of writing workarounds to support old versions.

yosefe · 2021-07-15T23:09:41Z

The cmake in CI is super old (2.8), the latest is 3.21, that's why it fails with
The source directory "/tmp/cmake-ucx" does not appear to contain CMakeLists.txt.
I have changed the
cmake_minimum_required(VERSION 3.5)
in the example to
cmake_minimum_required(VERSION 2.8)
Let's see if it works now. If not, I recommend upgrading the cmake, instead of writing workarounds to support old versions.

The CI is running on multiple different systems per the required support matrix. In this case, can skip this particular test if cmake version on the system is too old.

yosefe · 2021-07-16T10:30:29Z

buildlib/tools/builds.sh

+build_cmake_examples() {
+	echo "==== Build CMake sample ===="
+
+	${WORKSPACE}/contrib/configure-release --prefix=$ucx_inst


if which cmake then ... (the current test script) else azure_log_warning "cmake executable not found, skipping cmake test" fi

zasdfgbnm · 2021-07-16T21:01:03Z

Test failures seem unrelated

zasdfgbnm-nvidia force-pushed the cmake branch from 4554a13 to 8ec8fe7 Compare July 13, 2021 21:26

yosefe reviewed Jul 14, 2021

View reviewed changes

zasdfgbnm-nvidia force-pushed the cmake branch from 8ec8fe7 to 6c50948 Compare July 15, 2021 15:07

yosefe reviewed Jul 15, 2021

View reviewed changes

zasdfgbnm-nvidia force-pushed the cmake branch 7 times, most recently from 01de486 to 70463b4 Compare July 15, 2021 17:26

yosefe approved these changes Jul 15, 2021

View reviewed changes

zasdfgbnm-nvidia force-pushed the cmake branch from 70463b4 to e6696cb Compare July 15, 2021 19:44

yosefe reviewed Jul 15, 2021

View reviewed changes

zasdfgbnm-nvidia force-pushed the cmake branch from e6696cb to c8b5678 Compare July 15, 2021 20:02

yosefe approved these changes Jul 15, 2021

View reviewed changes

zasdfgbnm-nvidia mentioned this pull request Jul 15, 2021

BUILD: improve cmake support openucx/ucc#265

Merged

zasdfgbnm-nvidia force-pushed the cmake branch 3 times, most recently from 0562bd2 to a9b0fc4 Compare July 15, 2021 22:20

yosefe reviewed Jul 16, 2021

View reviewed changes

BUILD: Help cmake find UCX on system

72a1545

zasdfgbnm-nvidia force-pushed the cmake branch from a9b0fc4 to 72a1545 Compare July 16, 2021 15:48

yosefe merged commit d362596 into openucx:master Jul 17, 2021

brminich mentioned this pull request Dec 7, 2021

NEWS: News update for v1.12.0 rc1 #7741

Merged

zasdfgbnm mentioned this pull request Jun 21, 2022

[PFC] Native UCC process group for Pytorch pytorch/pytorch#79918

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

BUILD: Help cmake find UCX on system #7096

BUILD: Help cmake find UCX on system #7096

zasdfgbnm-nvidia commented Jul 13, 2021

swx-jenkins3 commented Jul 13, 2021

yosefe left a comment

yosefe Jul 14, 2021

zasdfgbnm Jul 15, 2021

yosefe Jul 14, 2021

yosefe Jul 14, 2021

zasdfgbnm commented Jul 15, 2021

yosefe Jul 15, 2021

zasdfgbnm Jul 15, 2021

yosefe Jul 15, 2021

zasdfgbnm Jul 15, 2021

yosefe commented Jul 15, 2021

zasdfgbnm commented Jul 15, 2021

yosefe commented Jul 15, 2021

zasdfgbnm commented Jul 15, 2021

yosefe commented Jul 15, 2021

yosefe Jul 15, 2021

yosefe Jul 15, 2021 •

edited

Loading

yosefe Jul 15, 2021

zasdfgbnm Jul 15, 2021

yosefe Jul 15, 2021

zasdfgbnm Jul 15, 2021

yosefe Jul 15, 2021

zasdfgbnm Jul 15, 2021

yosefe Jul 15, 2021

zasdfgbnm Jul 15, 2021

yosefe commented Jul 15, 2021

zasdfgbnm commented Jul 15, 2021 •

edited

Loading

yosefe commented Jul 15, 2021

yosefe Jul 16, 2021

zasdfgbnm commented Jul 16, 2021

BUILD: Help cmake find UCX on system #7096

BUILD: Help cmake find UCX on system #7096

Conversation

zasdfgbnm-nvidia commented Jul 13, 2021

swx-jenkins3 commented Jul 13, 2021

yosefe left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zasdfgbnm commented Jul 15, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yosefe commented Jul 15, 2021

zasdfgbnm commented Jul 15, 2021

yosefe commented Jul 15, 2021

zasdfgbnm commented Jul 15, 2021

yosefe commented Jul 15, 2021

Choose a reason for hiding this comment

yosefe Jul 15, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yosefe commented Jul 15, 2021

zasdfgbnm commented Jul 15, 2021 • edited Loading

yosefe commented Jul 15, 2021

Choose a reason for hiding this comment

zasdfgbnm commented Jul 16, 2021

yosefe Jul 15, 2021 •

edited

Loading

zasdfgbnm commented Jul 15, 2021 •

edited

Loading