From 6019606981c3c6800d7d076e1b1b05792eb0a1ff Mon Sep 17 00:00:00 2001 From: Simon Adorf Date: Mon, 10 Jul 2023 12:23:35 -0700 Subject: [PATCH 01/30] Add gtest to common_build dependencies. Needed by default unless the --nolibcumltest option is provided to the build.sh script. --- conda/environments/all_cuda-118_arch-x86_64.yaml | 1 + conda/environments/cpp_all_cuda-118_arch-x86_64.yaml | 1 + dependencies.yaml | 1 + 3 files changed, 3 insertions(+) diff --git a/conda/environments/all_cuda-118_arch-x86_64.yaml b/conda/environments/all_cuda-118_arch-x86_64.yaml index 15e75cab3c..fc6fb2541b 100644 --- a/conda/environments/all_cuda-118_arch-x86_64.yaml +++ b/conda/environments/all_cuda-118_arch-x86_64.yaml @@ -23,6 +23,7 @@ dependencies: - distributed>=2023.5.1 - doxygen=1.8.20 - gcc_linux-64=11.* +- gmock>=1.13.0 - graphviz - hdbscan - hypothesis>=6.0,<7 diff --git a/conda/environments/cpp_all_cuda-118_arch-x86_64.yaml b/conda/environments/cpp_all_cuda-118_arch-x86_64.yaml index a5ecce6f4a..9e30f250cd 100644 --- a/conda/environments/cpp_all_cuda-118_arch-x86_64.yaml +++ b/conda/environments/cpp_all_cuda-118_arch-x86_64.yaml @@ -12,6 +12,7 @@ dependencies: - cudatoolkit=11.8 - cxx-compiler - gcc_linux-64=11.* +- gmock>=1.13.0 - libcublas-dev=11.11.3.6 - libcublas=11.11.3.6 - libcufft-dev=10.9.0.58 diff --git a/dependencies.yaml b/dependencies.yaml index 3819cc3de2..8db5c2bde6 100644 --- a/dependencies.yaml +++ b/dependencies.yaml @@ -91,6 +91,7 @@ dependencies: packages: - c-compiler - cxx-compiler + - gmock>=1.13.0 - libcumlprims==23.8.* - libraft==23.8.* - libraft-headers==23.8.* From ec724b38db20b7d81a85eccceca10ba080d89dba Mon Sep 17 00:00:00 2001 From: Simon Adorf Date: Mon, 10 Jul 2023 12:28:58 -0700 Subject: [PATCH 02/30] Add nvcc to common_build dependencies. --- conda/environments/all_cuda-118_arch-x86_64.yaml | 1 + conda/environments/cpp_all_cuda-118_arch-x86_64.yaml | 1 + dependencies.yaml | 12 ++++++++++++ 3 files changed, 14 insertions(+) diff --git a/conda/environments/all_cuda-118_arch-x86_64.yaml b/conda/environments/all_cuda-118_arch-x86_64.yaml index fc6fb2541b..46f3a213c6 100644 --- a/conda/environments/all_cuda-118_arch-x86_64.yaml +++ b/conda/environments/all_cuda-118_arch-x86_64.yaml @@ -49,6 +49,7 @@ dependencies: - nltk - numba>=0.57 - numpydoc +- nvcc_linux-64=11.8 - pip - pydata-sphinx-theme - pylibraft==23.8.* diff --git a/conda/environments/cpp_all_cuda-118_arch-x86_64.yaml b/conda/environments/cpp_all_cuda-118_arch-x86_64.yaml index 9e30f250cd..15c123006d 100644 --- a/conda/environments/cpp_all_cuda-118_arch-x86_64.yaml +++ b/conda/environments/cpp_all_cuda-118_arch-x86_64.yaml @@ -28,5 +28,6 @@ dependencies: - libraft==23.8.* - librmm==23.8.* - ninja +- nvcc_linux-64=11.8 - sysroot_linux-64==2.17 name: cpp_all_cuda-118_arch-x86_64 diff --git a/dependencies.yaml b/dependencies.yaml index 8db5c2bde6..475f6e9988 100644 --- a/dependencies.yaml +++ b/dependencies.yaml @@ -109,6 +109,18 @@ dependencies: packages: - gcc_linux-aarch64=11.* - sysroot_linux-aarch64==2.17 + - output_types: conda + matrices: + - matrix: + arch: x86_64 + cuda: "11.8" + packages: + - nvcc_linux-64=11.8 + - matrix: + arch: aarch64 + cuda: "11.8" + packages: + - nvcc_linux-aarch64=11.8 py_build: common: - output_types: [conda, requirements, pyproject] From 900cf1f3d6622808f13688ca6940d7d46cd1784f Mon Sep 17 00:00:00 2001 From: Simon Adorf Date: Mon, 10 Jul 2023 15:13:08 -0700 Subject: [PATCH 03/30] fixup! Add gtest to common_build dependencies. --- conda/environments/all_cuda-118_arch-x86_64.yaml | 1 + conda/environments/cpp_all_cuda-118_arch-x86_64.yaml | 1 + dependencies.yaml | 1 + 3 files changed, 3 insertions(+) diff --git a/conda/environments/all_cuda-118_arch-x86_64.yaml b/conda/environments/all_cuda-118_arch-x86_64.yaml index 46f3a213c6..384f951ceb 100644 --- a/conda/environments/all_cuda-118_arch-x86_64.yaml +++ b/conda/environments/all_cuda-118_arch-x86_64.yaml @@ -25,6 +25,7 @@ dependencies: - gcc_linux-64=11.* - gmock>=1.13.0 - graphviz +- gtest>=1.13.0 - hdbscan - hypothesis>=6.0,<7 - ipykernel diff --git a/conda/environments/cpp_all_cuda-118_arch-x86_64.yaml b/conda/environments/cpp_all_cuda-118_arch-x86_64.yaml index 15c123006d..d868eaca97 100644 --- a/conda/environments/cpp_all_cuda-118_arch-x86_64.yaml +++ b/conda/environments/cpp_all_cuda-118_arch-x86_64.yaml @@ -13,6 +13,7 @@ dependencies: - cxx-compiler - gcc_linux-64=11.* - gmock>=1.13.0 +- gtest>=1.13.0 - libcublas-dev=11.11.3.6 - libcublas=11.11.3.6 - libcufft-dev=10.9.0.58 diff --git a/dependencies.yaml b/dependencies.yaml index 475f6e9988..6bbcb49ab2 100644 --- a/dependencies.yaml +++ b/dependencies.yaml @@ -92,6 +92,7 @@ dependencies: - c-compiler - cxx-compiler - gmock>=1.13.0 + - gtest>=1.13.0 - libcumlprims==23.8.* - libraft==23.8.* - libraft-headers==23.8.* From 836cab37503450a2b4248596a5b32a086132db5d Mon Sep 17 00:00:00 2001 From: Simon Adorf Date: Tue, 14 Feb 2023 05:32:20 -0800 Subject: [PATCH 04/30] Fix bug in cpp/scripts/run-clang-tidy.py script. --- cpp/scripts/run-clang-tidy.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cpp/scripts/run-clang-tidy.py b/cpp/scripts/run-clang-tidy.py index 278caa59bf..6e01790657 100755 --- a/cpp/scripts/run-clang-tidy.py +++ b/cpp/scripts/run-clang-tidy.py @@ -1,4 +1,4 @@ -# Copyright (c) 2020-2021, NVIDIA CORPORATION. +# Copyright (c) 2020-2023, NVIDIA CORPORATION. # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -239,7 +239,7 @@ def main(): if not os.path.exists(".git"): raise Exception("This needs to always be run from the root of repo") # Check whether clang-tidy exists - if shutil.which("clang-tidy") is not None: + if shutil.which("clang-tidy") is None: print("clang-tidy not found. Exiting...") return all_files = get_all_commands(args.cdb) From c42854c54f57c9de16ba753708b135d353c76e19 Mon Sep 17 00:00:00 2001 From: Simon Adorf Date: Tue, 14 Feb 2023 05:32:48 -0800 Subject: [PATCH 05/30] Add custom GA job to run clang-tidy. --- .github/workflows/pr.yaml | 11 +++++++++++ ci/run_clang_tidy.sh | 31 +++++++++++++++++++++++++++++++ dependencies.yaml | 11 +++++++++++ 3 files changed, 53 insertions(+) create mode 100755 ci/run_clang_tidy.sh diff --git a/.github/workflows/pr.yaml b/.github/workflows/pr.yaml index 97bc4a2acc..894d88a223 100644 --- a/.github/workflows/pr.yaml +++ b/.github/workflows/pr.yaml @@ -13,6 +13,7 @@ jobs: pr-builder: needs: - checks + - clang-tidy - conda-cpp-build - conda-cpp-tests - conda-python-build @@ -29,6 +30,16 @@ jobs: uses: rapidsai/shared-action-workflows/.github/workflows/checks.yaml@branch-23.08 with: enable_check_generated_files: false + clang-tidy: + needs: checks + secrets: inherit + uses: rapidsai/shared-action-workflows/.github/workflows/custom-job.yaml@branch-23.08 + with: + build_type: pull-request + node_type: "cpu8" + arch: "amd64" + container_image: "rapidsai/ci:latest" + run_script: "ci/run_clang_tidy.sh" conda-cpp-build: needs: checks secrets: inherit diff --git a/ci/run_clang_tidy.sh b/ci/run_clang_tidy.sh new file mode 100755 index 0000000000..b47c507c7e --- /dev/null +++ b/ci/run_clang_tidy.sh @@ -0,0 +1,31 @@ +#!/bin/bash +# Copyright (c) 2020-2023, NVIDIA CORPORATION. + +set -euo pipefail + +rapids-logger "Create clang_tidy conda environment" +. /opt/conda/etc/profile.d/conda.sh + +rapids-dependency-file-generator \ + --output conda \ + --file_key clang_tidy \ + --matrix "cuda=${RAPIDS_CUDA_VERSION%.*};arch=$(arch);py=${RAPIDS_PY_VERSION}" | tee env.yaml + +rapids-mamba-retry env create --force -f env.yaml -n clang_tidy +# Temporarily allow unbound variables for conda activation. +set +u && conda activate clang_tidy && set -u + +FORMAT_FILE_URL=https://raw.githubusercontent.com/rapidsai/rapids-cmake/branch-23.02/cmake-format-rapids-cmake.json +export RAPIDS_CMAKE_FORMAT_FILE=/tmp/rapids_cmake_ci/cmake-formats-rapids-cmake.json +mkdir -p $(dirname ${RAPIDS_CMAKE_FORMAT_FILE}) +wget -O ${RAPIDS_CMAKE_FORMAT_FILE} ${FORMAT_FILE_URL} + +export LD_LIBRARY_PATH="$CONDA_PREFIX/lib${LD_LIBRARY_PATH:+:$LD_LIBRARY_PATH}" +mkdir cpp/build +cd cpp/build +cmake -DGPU_ARCHS=70 \ + -DBLAS_LIBRARIES=${CONDA_PREFIX}/lib/libopenblas.so.0 \ + .. +make treelite +cd ../.. +python cpp/scripts/run-clang-tidy.py diff --git a/dependencies.yaml b/dependencies.yaml index 6bbcb49ab2..29a0eccff9 100644 --- a/dependencies.yaml +++ b/dependencies.yaml @@ -26,6 +26,11 @@ files: includes: - checks - py_version + clang_tidy: + output: none + includes: + - common_build + - clang_tidy docs: output: none includes: @@ -81,6 +86,12 @@ dependencies: - output_types: [conda, requirements] packages: - pre-commit + clang_tidy: + common: + - output_types: [conda, requirements] + packages: + - clang-tools==8.0.1 + - make common_build: common: - output_types: [conda, requirements, pyproject] From e1eb1f4fb91929c328957d2b981dbab60372ed85 Mon Sep 17 00:00:00 2001 From: Bradley Dice Date: Tue, 14 Feb 2023 09:59:38 -0600 Subject: [PATCH 06/30] Update clang-tools. --- ci/run_clang_tidy.sh | 6 +++--- cpp/scripts/run-clang-tidy.py | 2 +- dependencies.yaml | 3 ++- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/ci/run_clang_tidy.sh b/ci/run_clang_tidy.sh index b47c507c7e..d9e4c54838 100755 --- a/ci/run_clang_tidy.sh +++ b/ci/run_clang_tidy.sh @@ -15,17 +15,17 @@ rapids-mamba-retry env create --force -f env.yaml -n clang_tidy # Temporarily allow unbound variables for conda activation. set +u && conda activate clang_tidy && set -u -FORMAT_FILE_URL=https://raw.githubusercontent.com/rapidsai/rapids-cmake/branch-23.02/cmake-format-rapids-cmake.json +FORMAT_FILE_URL=https://raw.githubusercontent.com/rapidsai/rapids-cmake/branch-23.04/cmake-format-rapids-cmake.json export RAPIDS_CMAKE_FORMAT_FILE=/tmp/rapids_cmake_ci/cmake-formats-rapids-cmake.json mkdir -p $(dirname ${RAPIDS_CMAKE_FORMAT_FILE}) wget -O ${RAPIDS_CMAKE_FORMAT_FILE} ${FORMAT_FILE_URL} export LD_LIBRARY_PATH="$CONDA_PREFIX/lib${LD_LIBRARY_PATH:+:$LD_LIBRARY_PATH}" -mkdir cpp/build +mkdir -p cpp/build cd cpp/build cmake -DGPU_ARCHS=70 \ -DBLAS_LIBRARIES=${CONDA_PREFIX}/lib/libopenblas.so.0 \ .. -make treelite +make -j treelite cd ../.. python cpp/scripts/run-clang-tidy.py diff --git a/cpp/scripts/run-clang-tidy.py b/cpp/scripts/run-clang-tidy.py index 6e01790657..02449d7560 100755 --- a/cpp/scripts/run-clang-tidy.py +++ b/cpp/scripts/run-clang-tidy.py @@ -23,7 +23,7 @@ import shutil -EXPECTED_VERSION = "8.0.1" +EXPECTED_VERSION = "15.0.7" VERSION_REGEX = re.compile(r" LLVM version ([0-9.]+)") GPU_ARCH_REGEX = re.compile(r"sm_(\d+)") SPACES = re.compile(r"\s+") diff --git a/dependencies.yaml b/dependencies.yaml index 29a0eccff9..d48e470f01 100644 --- a/dependencies.yaml +++ b/dependencies.yaml @@ -90,7 +90,8 @@ dependencies: common: - output_types: [conda, requirements] packages: - - clang-tools==8.0.1 + - clang==15.0.7 + - clang-tools==15.0.7 - make common_build: common: From 7ee6d6833996f09df25740735d030ad576f80f49 Mon Sep 17 00:00:00 2001 From: Bradley Dice Date: Wed, 15 Feb 2023 16:12:02 -0600 Subject: [PATCH 07/30] Add cudatoolkit to clang-tidy for cuda_runtime_api.h. --- dependencies.yaml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/dependencies.yaml b/dependencies.yaml index d48e470f01..b1f985eee1 100644 --- a/dependencies.yaml +++ b/dependencies.yaml @@ -29,8 +29,9 @@ files: clang_tidy: output: none includes: - - common_build - clang_tidy + - common_build + - cudatoolkit docs: output: none includes: From 4657d777a043df18d1d7303e1b5e9f7284c266c4 Mon Sep 17 00:00:00 2001 From: Bradley Dice Date: Wed, 15 Feb 2023 18:03:28 -0600 Subject: [PATCH 08/30] Try adding gcc to get omp.h. --- dependencies.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/dependencies.yaml b/dependencies.yaml index b1f985eee1..5818e44c52 100644 --- a/dependencies.yaml +++ b/dependencies.yaml @@ -93,7 +93,9 @@ dependencies: packages: - clang==15.0.7 - clang-tools==15.0.7 + - gcc_linux-64=9.* - make + - sysroot_linux-64==2.17 common_build: common: - output_types: [conda, requirements, pyproject] From 776d8abc1896e6b3336fd7853057a9742b6c9223 Mon Sep 17 00:00:00 2001 From: Simon Adorf Date: Mon, 3 Jul 2023 14:59:06 -0700 Subject: [PATCH 09/30] Apply black-formatting to run-clang-tidy.py --- cpp/scripts/run-clang-tidy.py | 77 +++++++++++++++++++++++------------ 1 file changed, 50 insertions(+), 27 deletions(-) diff --git a/cpp/scripts/run-clang-tidy.py b/cpp/scripts/run-clang-tidy.py index 02449d7560..d169698600 100755 --- a/cpp/scripts/run-clang-tidy.py +++ b/cpp/scripts/run-clang-tidy.py @@ -32,20 +32,33 @@ def parse_args(): argparser = argparse.ArgumentParser("Runs clang-tidy on a project") - argparser.add_argument("-cdb", type=str, - default="cpp/build/compile_commands.json", - help="Path to cmake-generated compilation database" - " file. It is always found inside the root of the " - "cmake build folder. So make sure that `cmake` has " - "been run once before running this script!") - argparser.add_argument("-exe", type=str, default="clang-tidy", - help="Path to clang-tidy exe") - argparser.add_argument("-ignore", type=str, default="[.]cu$|examples/kmeans/", - help="Regex used to ignore files from checking") - argparser.add_argument("-select", type=str, default=None, - help="Regex used to select files for checking") - argparser.add_argument("-j", type=int, default=-1, - help="Number of parallel jobs to launch.") + argparser.add_argument( + "-cdb", + type=str, + default="cpp/build/compile_commands.json", + help="Path to cmake-generated compilation database" + " file. It is always found inside the root of the " + "cmake build folder. So make sure that `cmake` has " + "been run once before running this script!", + ) + argparser.add_argument( + "-exe", type=str, default="clang-tidy", help="Path to clang-tidy exe" + ) + argparser.add_argument( + "-ignore", + type=str, + default="[.]cu$|examples/kmeans/", + help="Regex used to ignore files from checking", + ) + argparser.add_argument( + "-select", + type=str, + default=None, + help="Regex used to select files for checking", + ) + argparser.add_argument( + "-j", type=int, default=-1, help="Number of parallel jobs to launch." + ) args = argparser.parse_args() if args.j <= 0: args.j = mp.cpu_count() @@ -58,8 +71,9 @@ def parse_args(): raise Exception("Failed to figure out clang-tidy version!") version = version.group(1) if version != EXPECTED_VERSION: - raise Exception("clang-tidy exe must be v%s found '%s'" % \ - (EXPECTED_VERSION, version)) + raise Exception( + "clang-tidy exe must be v%s found '%s'" % (EXPECTED_VERSION, version) + ) if not os.path.exists(args.cdb): raise Exception("Compilation database '%s' missing" % args.cdb) return args @@ -144,8 +158,9 @@ def get_tidy_args(cmd, exe): def run_clang_tidy_command(tidy_cmd): cmd = " ".join(tidy_cmd) - result = subprocess.run(cmd, check=False, shell=True, - stdout=subprocess.PIPE, stderr=subprocess.STDOUT) + result = subprocess.run( + cmd, check=False, shell=True, stdout=subprocess.PIPE, stderr=subprocess.STDOUT + ) status = result.returncode == 0 if status: out = "" @@ -157,9 +172,12 @@ def run_clang_tidy_command(tidy_cmd): def run_clang_tidy(cmd, args): command, is_cuda = get_tidy_args(cmd, args.exe) - tidy_cmd = [args.exe, - "-header-filter='.*cuml/cpp/(src|include|bench|comms).*'", - cmd["file"], "--", ] + tidy_cmd = [ + args.exe, + "-header-filter='.*cuml/cpp/(src|include|bench|comms).*'", + cmd["file"], + "--", + ] tidy_cmd.extend(command) status = True out = "" @@ -187,6 +205,8 @@ def run_clang_tidy(cmd, args): # yikes! global var :( results = [] + + def collect_result(result): global results results.append(result) @@ -215,15 +235,18 @@ def run_tidy_for_all_files(args, all_files): # actual tidy checker for cmd in all_files: # skip files that we don't want to look at - if args.ignore_compiled is not None and \ - re.search(args.ignore_compiled, cmd["file"]) is not None: + if ( + args.ignore_compiled is not None + and re.search(args.ignore_compiled, cmd["file"]) is not None + ): continue - if args.select_compiled is not None and \ - re.search(args.select_compiled, cmd["file"]) is None: + if ( + args.select_compiled is not None + and re.search(args.select_compiled, cmd["file"]) is None + ): continue if pool is not None: - pool.apply_async(run_clang_tidy, args=(cmd, args), - callback=collect_result) + pool.apply_async(run_clang_tidy, args=(cmd, args), callback=collect_result) else: passed, stdout, file = run_clang_tidy(cmd, args) collect_result((passed, stdout, file)) From 0d02750511cc1b2bf702e6cdb0294386023b9cdd Mon Sep 17 00:00:00 2001 From: Simon Adorf Date: Wed, 5 Jul 2023 15:47:47 -0700 Subject: [PATCH 10/30] Update cmake-format-rapids-cmake.json to 23.08. --- ci/run_clang_tidy.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ci/run_clang_tidy.sh b/ci/run_clang_tidy.sh index d9e4c54838..84c15a1b91 100755 --- a/ci/run_clang_tidy.sh +++ b/ci/run_clang_tidy.sh @@ -15,7 +15,7 @@ rapids-mamba-retry env create --force -f env.yaml -n clang_tidy # Temporarily allow unbound variables for conda activation. set +u && conda activate clang_tidy && set -u -FORMAT_FILE_URL=https://raw.githubusercontent.com/rapidsai/rapids-cmake/branch-23.04/cmake-format-rapids-cmake.json +FORMAT_FILE_URL=https://raw.githubusercontent.com/rapidsai/rapids-cmake/branch-23.08/cmake-format-rapids-cmake.json export RAPIDS_CMAKE_FORMAT_FILE=/tmp/rapids_cmake_ci/cmake-formats-rapids-cmake.json mkdir -p $(dirname ${RAPIDS_CMAKE_FORMAT_FILE}) wget -O ${RAPIDS_CMAKE_FORMAT_FILE} ${FORMAT_FILE_URL} From 47d108a76b733d94393e7804db8fc29333c9ade2 Mon Sep 17 00:00:00 2001 From: Simon Adorf Date: Wed, 5 Jul 2023 15:52:02 -0700 Subject: [PATCH 11/30] Remove obsolete code from ci/run_clang_tidy.sh. --- ci/run_clang_tidy.sh | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/ci/run_clang_tidy.sh b/ci/run_clang_tidy.sh index 84c15a1b91..fa5a9395aa 100755 --- a/ci/run_clang_tidy.sh +++ b/ci/run_clang_tidy.sh @@ -21,11 +21,5 @@ mkdir -p $(dirname ${RAPIDS_CMAKE_FORMAT_FILE}) wget -O ${RAPIDS_CMAKE_FORMAT_FILE} ${FORMAT_FILE_URL} export LD_LIBRARY_PATH="$CONDA_PREFIX/lib${LD_LIBRARY_PATH:+:$LD_LIBRARY_PATH}" -mkdir -p cpp/build -cd cpp/build -cmake -DGPU_ARCHS=70 \ - -DBLAS_LIBRARIES=${CONDA_PREFIX}/lib/libopenblas.so.0 \ - .. -make -j treelite -cd ../.. +./build.sh treelite python cpp/scripts/run-clang-tidy.py From 2f19b14393747c55bb085d57f559812807d00664 Mon Sep 17 00:00:00 2001 From: Simon Adorf Date: Wed, 5 Jul 2023 16:06:43 -0700 Subject: [PATCH 12/30] Revert "Try adding gcc to get omp.h." This reverts commit 5ac7f9b1496bb2c59b45d07f40186d6de8dfa471. --- dependencies.yaml | 2 -- 1 file changed, 2 deletions(-) diff --git a/dependencies.yaml b/dependencies.yaml index 5818e44c52..b1f985eee1 100644 --- a/dependencies.yaml +++ b/dependencies.yaml @@ -93,9 +93,7 @@ dependencies: packages: - clang==15.0.7 - clang-tools==15.0.7 - - gcc_linux-64=9.* - make - - sysroot_linux-64==2.17 common_build: common: - output_types: [conda, requirements, pyproject] From 9be6df73cc69b57a539457173049058d96fcae45 Mon Sep 17 00:00:00 2001 From: Simon Adorf Date: Wed, 5 Jul 2023 16:18:52 -0700 Subject: [PATCH 13/30] Select build target 'libcuml'. --- ci/run_clang_tidy.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ci/run_clang_tidy.sh b/ci/run_clang_tidy.sh index fa5a9395aa..e6da69e805 100755 --- a/ci/run_clang_tidy.sh +++ b/ci/run_clang_tidy.sh @@ -21,5 +21,5 @@ mkdir -p $(dirname ${RAPIDS_CMAKE_FORMAT_FILE}) wget -O ${RAPIDS_CMAKE_FORMAT_FILE} ${FORMAT_FILE_URL} export LD_LIBRARY_PATH="$CONDA_PREFIX/lib${LD_LIBRARY_PATH:+:$LD_LIBRARY_PATH}" -./build.sh treelite +./build.sh libcuml python cpp/scripts/run-clang-tidy.py From 03580f2d94309d95ded40adb11ab734add6667c0 Mon Sep 17 00:00:00 2001 From: Simon Adorf Date: Thu, 6 Jul 2023 14:18:51 -0700 Subject: [PATCH 14/30] Remove obsolete code from run_clang_tidy.sh script. --- ci/run_clang_tidy.sh | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/ci/run_clang_tidy.sh b/ci/run_clang_tidy.sh index e6da69e805..e353a8c6a8 100755 --- a/ci/run_clang_tidy.sh +++ b/ci/run_clang_tidy.sh @@ -15,11 +15,8 @@ rapids-mamba-retry env create --force -f env.yaml -n clang_tidy # Temporarily allow unbound variables for conda activation. set +u && conda activate clang_tidy && set -u -FORMAT_FILE_URL=https://raw.githubusercontent.com/rapidsai/rapids-cmake/branch-23.08/cmake-format-rapids-cmake.json -export RAPIDS_CMAKE_FORMAT_FILE=/tmp/rapids_cmake_ci/cmake-formats-rapids-cmake.json -mkdir -p $(dirname ${RAPIDS_CMAKE_FORMAT_FILE}) -wget -O ${RAPIDS_CMAKE_FORMAT_FILE} ${FORMAT_FILE_URL} - -export LD_LIBRARY_PATH="$CONDA_PREFIX/lib${LD_LIBRARY_PATH:+:$LD_LIBRARY_PATH}" ./build.sh libcuml + +rapids-logger "Run clang-tidy" + python cpp/scripts/run-clang-tidy.py From 85c16e772dd498e0c4353bbfa44532df27217ed6 Mon Sep 17 00:00:00 2001 From: Simon Adorf Date: Fri, 7 Jul 2023 22:15:56 +0000 Subject: [PATCH 15/30] Select clang-cpp compiler with -std=c++17 option. --- cpp/scripts/run-clang-tidy.py | 52 ++++++++++++++------------- cpp/test/c_api/dbscan_api_test.c | 3 ++ cpp/test/c_api/glm_api_test.c | 3 ++ cpp/test/c_api/holtwinters_api_test.c | 8 ++++- cpp/test/c_api/knn_api_test.c | 4 ++- cpp/test/c_api/svm_api_test.c | 7 +++- 6 files changed, 50 insertions(+), 27 deletions(-) diff --git a/cpp/scripts/run-clang-tidy.py b/cpp/scripts/run-clang-tidy.py index d169698600..2f92c7e535 100755 --- a/cpp/scripts/run-clang-tidy.py +++ b/cpp/scripts/run-clang-tidy.py @@ -132,8 +132,12 @@ def get_tidy_args(cmd, exe): command, file = cmd["command"], cmd["file"] is_cuda = file.endswith(".cu") command = re.split(SPACES, command) - # compiler is always clang++! - command[0] = "clang++" + # Adjust compiler command + if "c++" in command[0]: + command[0] = "clang-cpp" + command.insert(1, "-std=c++17") + elif command[0][-2:] == "cc": + command[0] = "clang" # remove compilation and output targets from the original command remove_item_plus_one(command, "-c") remove_item_plus_one(command, "-o") @@ -161,13 +165,9 @@ def run_clang_tidy_command(tidy_cmd): result = subprocess.run( cmd, check=False, shell=True, stdout=subprocess.PIPE, stderr=subprocess.STDOUT ) - status = result.returncode == 0 - if status: - out = "" - else: - out = "CMD: " + cmd - out += result.stdout.decode("utf-8").rstrip() - return status, out + passed = result.returncode == 0 + out = "" if passed else f"CMD: {cmd}: {result.stdout.decode('utf-8').rstrip()}" + return passed, out def run_clang_tidy(cmd, args): @@ -179,28 +179,29 @@ def run_clang_tidy(cmd, args): "--", ] tidy_cmd.extend(command) - status = True - out = "" + all_passed = True + out = [] if is_cuda: tidy_cmd.append("--cuda-device-only") tidy_cmd.append(cmd["file"]) - ret, out1 = run_clang_tidy_command(tidy_cmd) - out += out1 - out += "%s" % SEPARATOR - if not ret: - status = ret + passed, out1 = run_clang_tidy_command(tidy_cmd) + out.append(out1) + out.append(SEPARATOR) + if not passed: + all_passed = False tidy_cmd[-2] = "--cuda-host-only" - ret, out1 = run_clang_tidy_command(tidy_cmd) - if not ret: - status = ret - out += out1 + passed, out1 = run_clang_tidy_command(tidy_cmd) + if not passed: + all_passed = False + out.append(out1) else: tidy_cmd.append(cmd["file"]) - ret, out1 = run_clang_tidy_command(tidy_cmd) - if not ret: - status = ret + passed, out1 = run_clang_tidy_command(tidy_cmd) + if not passed: + all_passed = False + out.append(out1) out += out1 - return status, out, cmd["file"] + return all_passed, "".join(out), cmd["file"] # yikes! global var :( @@ -245,6 +246,9 @@ def run_tidy_for_all_files(args, all_files): and re.search(args.select_compiled, cmd["file"]) is None ): continue + # from pprint import pprint; + # pprint(cmd) + # print(cmd["command"].split()[-1]) if pool is not None: pool.apply_async(run_clang_tidy, args=(cmd, args), callback=collect_result) else: diff --git a/cpp/test/c_api/dbscan_api_test.c b/cpp/test/c_api/dbscan_api_test.c index 6b565e0624..46e76d1f55 100644 --- a/cpp/test/c_api/dbscan_api_test.c +++ b/cpp/test/c_api/dbscan_api_test.c @@ -21,7 +21,10 @@ void test_dbscan() cumlHandle_t handle = 0; cumlError_t response = CUML_SUCCESS; + // Checking return type at compile time. + // NOLINTNEXTLINE(lang-analyzer-deadcode.DeadStores) response = cumlSpDbscanFit(handle, NULL, 0, 1, 1.0f, 2, NULL, NULL, 10, 1); + // NOLINTNEXTLINE(lang-analyzer-deadcode.DeadStores) response = cumlDpDbscanFit(handle, NULL, 0, 1, 1.0, 2, NULL, NULL, 10, 1); } diff --git a/cpp/test/c_api/glm_api_test.c b/cpp/test/c_api/glm_api_test.c index 47c9e40099..a460be0df1 100644 --- a/cpp/test/c_api/glm_api_test.c +++ b/cpp/test/c_api/glm_api_test.c @@ -33,7 +33,10 @@ void test_glm() .fit_intercept = true, .penalty_normalized = true}; + // Checking return type at compile time. + // NOLINTNEXTLINE(lang-analyzer-deadcode.DeadStores) response = cumlSpQnFit(handle, &pams, NULL, NULL, 0, 1, 2, NULL, NULL, NULL, true); + // NOLINTNEXTLINE(lang-analyzer-deadcode.DeadStores) response = cumlDpQnFit(handle, &pams, NULL, NULL, 0, 1, 2, NULL, NULL, NULL, true); } diff --git a/cpp/test/c_api/holtwinters_api_test.c b/cpp/test/c_api/holtwinters_api_test.c index a8a01a0e46..3353dd7893 100644 --- a/cpp/test/c_api/holtwinters_api_test.c +++ b/cpp/test/c_api/holtwinters_api_test.c @@ -21,15 +21,21 @@ void test_holtwinters() cumlHandle_t handle = 0; cumlError_t response = CUML_SUCCESS; + // Checking return type at compile time. + // NOLINTNEXTLINE(lang-analyzer-deadcode.DeadStores) response = cumlHoltWinters_buffer_size(0, 1, 2, NULL, NULL, NULL, NULL, NULL, NULL); + // NOLINTNEXTLINE(lang-analyzer-deadcode.DeadStores) response = cumlHoltWintersSp_fit(handle, 0, 1, 2, 3, ADDITIVE, 1.0f, NULL, NULL, NULL, NULL, NULL); + // NOLINTNEXTLINE(lang-analyzer-deadcode.DeadStores) response = cumlHoltWintersDp_fit(handle, 0, 1, 2, 3, ADDITIVE, 1.0f, NULL, NULL, NULL, NULL, NULL); + // NOLINTNEXTLINE(lang-analyzer-deadcode.DeadStores) response = cumlHoltWintersSp_forecast(handle, 0, 1, 2, 3, ADDITIVE, NULL, NULL, NULL, NULL); + // NOLINTNEXTLINE(lang-analyzer-deadcode.DeadStores) response = cumlHoltWintersDp_forecast(handle, 0, 1, 2, 3, ADDITIVE, NULL, NULL, NULL, NULL); -} \ No newline at end of file +} diff --git a/cpp/test/c_api/knn_api_test.c b/cpp/test/c_api/knn_api_test.c index 0740ead431..9867b60c64 100644 --- a/cpp/test/c_api/knn_api_test.c +++ b/cpp/test/c_api/knn_api_test.c @@ -21,6 +21,8 @@ void test_knn() cumlHandle_t handle = 0; cumlError_t response = CUML_SUCCESS; + // Checking return type at compile time. + // NOLINTNEXTLINE(lang-analyzer-deadcode.DeadStores) response = knn_search(handle, NULL, NULL, 1, 2, NULL, 3, NULL, NULL, 4, true, false, 0, 2.0f, false); -} \ No newline at end of file +} diff --git a/cpp/test/c_api/svm_api_test.c b/cpp/test/c_api/svm_api_test.c index 5b95bf9148..ef050686f3 100644 --- a/cpp/test/c_api/svm_api_test.c +++ b/cpp/test/c_api/svm_api_test.c @@ -21,6 +21,8 @@ void test_svm() cumlHandle_t handle = 0; cumlError_t response = CUML_SUCCESS; + // Checking return type at compile time. + // NOLINTNEXTLINE(lang-analyzer-deadcode.DeadStores) response = cumlSpSvcFit(handle, NULL, 0, @@ -44,6 +46,7 @@ void test_svm() NULL, NULL); + // NOLINTNEXTLINE(lang-analyzer-deadcode.DeadStores) response = cumlDpSvcFit(handle, NULL, 0, @@ -67,9 +70,11 @@ void test_svm() NULL, NULL); + // NOLINTNEXTLINE(lang-analyzer-deadcode.DeadStores) response = cumlSpSvcPredict( handle, NULL, 0, 1, LINEAR, 2, 3.0f, 4.0f, 5, 6.0f, NULL, NULL, 7, NULL, NULL, 8.0f, 9); + // NOLINTNEXTLINE(lang-analyzer-deadcode.DeadStores) response = cumlDpSvcPredict( handle, NULL, 0, 1, LINEAR, 2, 3.0, 4.0, 5, 6.0, NULL, NULL, 7, NULL, NULL, 8.0, 9); -} \ No newline at end of file +} From 5b4c816e08b15270eb8030073529070628afde98 Mon Sep 17 00:00:00 2001 From: Simon Adorf Date: Fri, 7 Jul 2023 22:18:36 +0000 Subject: [PATCH 16/30] Ignore deadstores issue with c_api tests. --- cpp/test/c_api/dbscan_api_test.c | 4 ++-- cpp/test/c_api/glm_api_test.c | 4 ++-- cpp/test/c_api/holtwinters_api_test.c | 10 +++++----- cpp/test/c_api/knn_api_test.c | 2 +- cpp/test/c_api/svm_api_test.c | 8 ++++---- 5 files changed, 14 insertions(+), 14 deletions(-) diff --git a/cpp/test/c_api/dbscan_api_test.c b/cpp/test/c_api/dbscan_api_test.c index 46e76d1f55..dcc31abdb9 100644 --- a/cpp/test/c_api/dbscan_api_test.c +++ b/cpp/test/c_api/dbscan_api_test.c @@ -22,9 +22,9 @@ void test_dbscan() cumlError_t response = CUML_SUCCESS; // Checking return type at compile time. - // NOLINTNEXTLINE(lang-analyzer-deadcode.DeadStores) + // NOLINTNEXTLINE(clang-analyzer-deadcode.DeadStores) response = cumlSpDbscanFit(handle, NULL, 0, 1, 1.0f, 2, NULL, NULL, 10, 1); - // NOLINTNEXTLINE(lang-analyzer-deadcode.DeadStores) + // NOLINTNEXTLINE(clang-analyzer-deadcode.DeadStores) response = cumlDpDbscanFit(handle, NULL, 0, 1, 1.0, 2, NULL, NULL, 10, 1); } diff --git a/cpp/test/c_api/glm_api_test.c b/cpp/test/c_api/glm_api_test.c index a460be0df1..375919d770 100644 --- a/cpp/test/c_api/glm_api_test.c +++ b/cpp/test/c_api/glm_api_test.c @@ -34,9 +34,9 @@ void test_glm() .penalty_normalized = true}; // Checking return type at compile time. - // NOLINTNEXTLINE(lang-analyzer-deadcode.DeadStores) + // NOLINTNEXTLINE(clang-analyzer-deadcode.DeadStores) response = cumlSpQnFit(handle, &pams, NULL, NULL, 0, 1, 2, NULL, NULL, NULL, true); - // NOLINTNEXTLINE(lang-analyzer-deadcode.DeadStores) + // NOLINTNEXTLINE(clang-analyzer-deadcode.DeadStores) response = cumlDpQnFit(handle, &pams, NULL, NULL, 0, 1, 2, NULL, NULL, NULL, true); } diff --git a/cpp/test/c_api/holtwinters_api_test.c b/cpp/test/c_api/holtwinters_api_test.c index 3353dd7893..0d2fd49a95 100644 --- a/cpp/test/c_api/holtwinters_api_test.c +++ b/cpp/test/c_api/holtwinters_api_test.c @@ -22,20 +22,20 @@ void test_holtwinters() cumlError_t response = CUML_SUCCESS; // Checking return type at compile time. - // NOLINTNEXTLINE(lang-analyzer-deadcode.DeadStores) + // NOLINTNEXTLINE(clang-analyzer-deadcode.DeadStores) response = cumlHoltWinters_buffer_size(0, 1, 2, NULL, NULL, NULL, NULL, NULL, NULL); - // NOLINTNEXTLINE(lang-analyzer-deadcode.DeadStores) + // NOLINTNEXTLINE(clang-analyzer-deadcode.DeadStores) response = cumlHoltWintersSp_fit(handle, 0, 1, 2, 3, ADDITIVE, 1.0f, NULL, NULL, NULL, NULL, NULL); - // NOLINTNEXTLINE(lang-analyzer-deadcode.DeadStores) + // NOLINTNEXTLINE(clang-analyzer-deadcode.DeadStores) response = cumlHoltWintersDp_fit(handle, 0, 1, 2, 3, ADDITIVE, 1.0f, NULL, NULL, NULL, NULL, NULL); - // NOLINTNEXTLINE(lang-analyzer-deadcode.DeadStores) + // NOLINTNEXTLINE(clang-analyzer-deadcode.DeadStores) response = cumlHoltWintersSp_forecast(handle, 0, 1, 2, 3, ADDITIVE, NULL, NULL, NULL, NULL); - // NOLINTNEXTLINE(lang-analyzer-deadcode.DeadStores) + // NOLINTNEXTLINE(clang-analyzer-deadcode.DeadStores) response = cumlHoltWintersDp_forecast(handle, 0, 1, 2, 3, ADDITIVE, NULL, NULL, NULL, NULL); } diff --git a/cpp/test/c_api/knn_api_test.c b/cpp/test/c_api/knn_api_test.c index 9867b60c64..0158d18434 100644 --- a/cpp/test/c_api/knn_api_test.c +++ b/cpp/test/c_api/knn_api_test.c @@ -22,7 +22,7 @@ void test_knn() cumlError_t response = CUML_SUCCESS; // Checking return type at compile time. - // NOLINTNEXTLINE(lang-analyzer-deadcode.DeadStores) + // NOLINTNEXTLINE(clang-analyzer-deadcode.DeadStores) response = knn_search(handle, NULL, NULL, 1, 2, NULL, 3, NULL, NULL, 4, true, false, 0, 2.0f, false); } diff --git a/cpp/test/c_api/svm_api_test.c b/cpp/test/c_api/svm_api_test.c index ef050686f3..1b02625c34 100644 --- a/cpp/test/c_api/svm_api_test.c +++ b/cpp/test/c_api/svm_api_test.c @@ -22,7 +22,7 @@ void test_svm() cumlError_t response = CUML_SUCCESS; // Checking return type at compile time. - // NOLINTNEXTLINE(lang-analyzer-deadcode.DeadStores) + // NOLINTNEXTLINE(clang-analyzer-deadcode.DeadStores) response = cumlSpSvcFit(handle, NULL, 0, @@ -46,7 +46,7 @@ void test_svm() NULL, NULL); - // NOLINTNEXTLINE(lang-analyzer-deadcode.DeadStores) + // NOLINTNEXTLINE(clang-analyzer-deadcode.DeadStores) response = cumlDpSvcFit(handle, NULL, 0, @@ -70,11 +70,11 @@ void test_svm() NULL, NULL); - // NOLINTNEXTLINE(lang-analyzer-deadcode.DeadStores) + // NOLINTNEXTLINE(clang-analyzer-deadcode.DeadStores) response = cumlSpSvcPredict( handle, NULL, 0, 1, LINEAR, 2, 3.0f, 4.0f, 5, 6.0f, NULL, NULL, 7, NULL, NULL, 8.0f, 9); - // NOLINTNEXTLINE(lang-analyzer-deadcode.DeadStores) + // NOLINTNEXTLINE(clang-analyzer-deadcode.DeadStores) response = cumlDpSvcPredict( handle, NULL, 0, 1, LINEAR, 2, 3.0, 4.0, 5, 6.0, NULL, NULL, 7, NULL, NULL, 8.0, 9); } From b3ab7afe35502ddea53f8b6f4473a3d34e3244eb Mon Sep 17 00:00:00 2001 From: Simon Adorf Date: Fri, 7 Jul 2023 22:34:50 +0000 Subject: [PATCH 17/30] Fix clang-tidy issues in experimental fil. --- .../cuml/experimental/fil/detail/raft_proto/buffer.hpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cpp/include/cuml/experimental/fil/detail/raft_proto/buffer.hpp b/cpp/include/cuml/experimental/fil/detail/raft_proto/buffer.hpp index 8538a5eae3..40de57789f 100644 --- a/cpp/include/cuml/experimental/fil/detail/raft_proto/buffer.hpp +++ b/cpp/include/cuml/experimental/fil/detail/raft_proto/buffer.hpp @@ -94,7 +94,7 @@ struct buffer { } return result; }()}, - data_{[this, input_data, mem_type]() { + data_{[input_data, mem_type]() { auto result = data_store{}; switch (mem_type) { case device_type::cpu: result = non_owning_buffer{input_data}; break; @@ -134,7 +134,7 @@ struct buffer { } return result; }()}, - data_{[this, &other, mem_type, device, stream]() { + data_{[this, &other, mem_type, stream]() { auto result = data_store{}; auto result_data = static_cast(nullptr); if (mem_type == device_type::cpu) { From 53fcbf9b76026d42df43124bf33953f957209c26 Mon Sep 17 00:00:00 2001 From: Simon Adorf Date: Fri, 7 Jul 2023 22:36:32 +0000 Subject: [PATCH 18/30] Adjust clang-tidy ignores. --- ci/run_clang_tidy.sh | 2 +- cpp/scripts/run-clang-tidy.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ci/run_clang_tidy.sh b/ci/run_clang_tidy.sh index e353a8c6a8..797a0abd1f 100755 --- a/ci/run_clang_tidy.sh +++ b/ci/run_clang_tidy.sh @@ -19,4 +19,4 @@ set +u && conda activate clang_tidy && set -u rapids-logger "Run clang-tidy" -python cpp/scripts/run-clang-tidy.py +python cpp/scripts/run-clang-tidy.py -ignore '[.]cu$|_deps|examples/kmeans/' diff --git a/cpp/scripts/run-clang-tidy.py b/cpp/scripts/run-clang-tidy.py index 2f92c7e535..092cff67a2 100755 --- a/cpp/scripts/run-clang-tidy.py +++ b/cpp/scripts/run-clang-tidy.py @@ -47,7 +47,7 @@ def parse_args(): argparser.add_argument( "-ignore", type=str, - default="[.]cu$|examples/kmeans/", + default="[.]cu$", help="Regex used to ignore files from checking", ) argparser.add_argument( From 2c2899510a44a7758c06556e947737b615e59b29 Mon Sep 17 00:00:00 2001 From: Simon Adorf Date: Mon, 10 Jul 2023 08:43:28 -0700 Subject: [PATCH 19/30] Add --nobuild option to build script. To enable skipping of actual builds and just invoking CMake. Used in the context of clang-tidy. --- build.sh | 10 ++++++---- ci/run_clang_tidy.sh | 2 +- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/build.sh b/build.sh index 82c907ad28..db78e98370 100755 --- a/build.sh +++ b/build.sh @@ -19,7 +19,7 @@ ARGS=$* REPODIR=$(cd $(dirname $0); pwd) VALIDTARGETS="clean libcuml cuml cpp-mgtests prims bench prims-bench cppdocs pydocs" -VALIDFLAGS="-v -g -n --allgpuarch --singlegpu --nolibcumltest --nvtx --show_depr_warn --codecov --ccache -h --help " +VALIDFLAGS="-v -g -n --allgpuarch --singlegpu --nolibcumltest --nvtx --show_depr_warn --codecov --ccache --nobuild -h --help " VALIDARGS="${VALIDTARGETS} ${VALIDFLAGS}" HELP="$0 [ ...] [ ...] where is: @@ -46,6 +46,7 @@ HELP="$0 [ ...] [ ...] --codecov - Enable code coverage support by compiling with Cython linetracing and profiling enabled (WARNING: Impacts performance) --ccache - Use ccache to cache previous compilations + --nobuild - Invoke CMake without actually building --nocloneraft - CMake will clone RAFT even if it is in the environment, use this flag to disable that behavior --static-treelite - Force CMake to use the Treelite static libs, cloning and building them if necessary @@ -135,6 +136,7 @@ LONG_ARGUMENT_LIST=( "ccache" "nolibcumltest" "nocloneraft" + "nobuild" ) # Short arguments @@ -262,7 +264,7 @@ if completeBuild || hasArg libcuml || hasArg prims || hasArg bench || hasArg pri fi # If `./build.sh cuml` is called, don't build C/C++ components -if completeBuild || hasArg libcuml || hasArg prims || hasArg bench || hasArg cpp-mgtests; then +if (! hasArg --nobuild) && (completeBuild || hasArg libcuml || hasArg prims || hasArg bench || hasArg cpp-mgtests); then cd ${LIBCUML_BUILD_DIR} if [ -n "${INSTALL_TARGET}" ]; then cmake --build ${LIBCUML_BUILD_DIR} -j${PARALLEL_LEVEL} ${build_args} --target ${INSTALL_TARGET} ${VERBOSE_FLAG} @@ -271,14 +273,14 @@ if completeBuild || hasArg libcuml || hasArg prims || hasArg bench || hasArg cpp fi fi -if hasArg cppdocs; then +if (! hasArg --nobuild) && hasArg cppdocs; then cd ${LIBCUML_BUILD_DIR} cmake --build ${LIBCUML_BUILD_DIR} --target docs_cuml fi # Build and (optionally) install the cuml Python package -if completeBuild || hasArg cuml || hasArg pydocs; then +if (! hasArg --nobuild) && (completeBuild || hasArg cuml || hasArg pydocs); then # Append `-DFIND_CUML_CPP=ON` to CUML_EXTRA_CMAKE_ARGS unless a user specified the option. if [[ "${CUML_EXTRA_CMAKE_ARGS}" != *"DFIND_CUML_CPP"* ]]; then CUML_EXTRA_CMAKE_ARGS="${CUML_EXTRA_CMAKE_ARGS} -DFIND_CUML_CPP=ON" diff --git a/ci/run_clang_tidy.sh b/ci/run_clang_tidy.sh index 797a0abd1f..af2ff04fd5 100755 --- a/ci/run_clang_tidy.sh +++ b/ci/run_clang_tidy.sh @@ -15,7 +15,7 @@ rapids-mamba-retry env create --force -f env.yaml -n clang_tidy # Temporarily allow unbound variables for conda activation. set +u && conda activate clang_tidy && set -u -./build.sh libcuml +./build.sh --nobuild libcuml rapids-logger "Run clang-tidy" From 61d05179b77fb36b323372718d1ec28721c292ab Mon Sep 17 00:00:00 2001 From: Simon Adorf Date: Mon, 10 Jul 2023 12:36:49 -0700 Subject: [PATCH 20/30] Handle unexpected compiler condition. --- cpp/scripts/run-clang-tidy.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/cpp/scripts/run-clang-tidy.py b/cpp/scripts/run-clang-tidy.py index 092cff67a2..d9dc423b9d 100755 --- a/cpp/scripts/run-clang-tidy.py +++ b/cpp/scripts/run-clang-tidy.py @@ -138,6 +138,8 @@ def get_tidy_args(cmd, exe): command.insert(1, "-std=c++17") elif command[0][-2:] == "cc": command[0] = "clang" + else: + raise ValueError("Unable to identify compiler.") # remove compilation and output targets from the original command remove_item_plus_one(command, "-c") remove_item_plus_one(command, "-o") From e00738cadb3dbf0547476bffd0722f33c5f873af Mon Sep 17 00:00:00 2001 From: Simon Adorf Date: Mon, 10 Jul 2023 16:33:08 -0700 Subject: [PATCH 21/30] Remove debug commands. --- cpp/scripts/run-clang-tidy.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/cpp/scripts/run-clang-tidy.py b/cpp/scripts/run-clang-tidy.py index d9dc423b9d..6741abcebd 100755 --- a/cpp/scripts/run-clang-tidy.py +++ b/cpp/scripts/run-clang-tidy.py @@ -248,9 +248,6 @@ def run_tidy_for_all_files(args, all_files): and re.search(args.select_compiled, cmd["file"]) is None ): continue - # from pprint import pprint; - # pprint(cmd) - # print(cmd["command"].split()[-1]) if pool is not None: pool.apply_async(run_clang_tidy, args=(cmd, args), callback=collect_result) else: From b468891afe13dbb05d3d6d2937b6ae1710e3e712 Mon Sep 17 00:00:00 2001 From: Simon Adorf Date: Wed, 12 Jul 2023 12:04:41 -0700 Subject: [PATCH 22/30] Fix copyright year range on run_clang_tidy.sh. --- ci/run_clang_tidy.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ci/run_clang_tidy.sh b/ci/run_clang_tidy.sh index af2ff04fd5..8527453506 100755 --- a/ci/run_clang_tidy.sh +++ b/ci/run_clang_tidy.sh @@ -1,5 +1,5 @@ #!/bin/bash -# Copyright (c) 2020-2023, NVIDIA CORPORATION. +# Copyright (c) 2023, NVIDIA CORPORATION. set -euo pipefail From 26cbc00c23a6cb35d367f4e8e7a2bf78a99764fe Mon Sep 17 00:00:00 2001 From: Simon Adorf Date: Wed, 12 Jul 2023 08:41:16 -0700 Subject: [PATCH 23/30] Use ninja instead of make for clang-tidy. --- dependencies.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dependencies.yaml b/dependencies.yaml index b1f985eee1..bac14209ab 100644 --- a/dependencies.yaml +++ b/dependencies.yaml @@ -93,7 +93,7 @@ dependencies: packages: - clang==15.0.7 - clang-tools==15.0.7 - - make + - ninja common_build: common: - output_types: [conda, requirements, pyproject] From 058bd31230c41b571ba6613d43254ce09eb5430a Mon Sep 17 00:00:00 2001 From: Simon Adorf Date: Wed, 12 Jul 2023 08:42:51 -0700 Subject: [PATCH 24/30] Rename --nobuild build option to --configure-only. --- build.sh | 12 ++++++------ ci/run_clang_tidy.sh | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/build.sh b/build.sh index db78e98370..9e58befbfc 100755 --- a/build.sh +++ b/build.sh @@ -19,7 +19,7 @@ ARGS=$* REPODIR=$(cd $(dirname $0); pwd) VALIDTARGETS="clean libcuml cuml cpp-mgtests prims bench prims-bench cppdocs pydocs" -VALIDFLAGS="-v -g -n --allgpuarch --singlegpu --nolibcumltest --nvtx --show_depr_warn --codecov --ccache --nobuild -h --help " +VALIDFLAGS="-v -g -n --allgpuarch --singlegpu --nolibcumltest --nvtx --show_depr_warn --codecov --ccache --configure-only -h --help " VALIDARGS="${VALIDTARGETS} ${VALIDFLAGS}" HELP="$0 [ ...] [ ...] where is: @@ -46,7 +46,7 @@ HELP="$0 [ ...] [ ...] --codecov - Enable code coverage support by compiling with Cython linetracing and profiling enabled (WARNING: Impacts performance) --ccache - Use ccache to cache previous compilations - --nobuild - Invoke CMake without actually building + --configure-only - Invoke CMake without actually building --nocloneraft - CMake will clone RAFT even if it is in the environment, use this flag to disable that behavior --static-treelite - Force CMake to use the Treelite static libs, cloning and building them if necessary @@ -136,7 +136,7 @@ LONG_ARGUMENT_LIST=( "ccache" "nolibcumltest" "nocloneraft" - "nobuild" + "configure-only" ) # Short arguments @@ -264,7 +264,7 @@ if completeBuild || hasArg libcuml || hasArg prims || hasArg bench || hasArg pri fi # If `./build.sh cuml` is called, don't build C/C++ components -if (! hasArg --nobuild) && (completeBuild || hasArg libcuml || hasArg prims || hasArg bench || hasArg cpp-mgtests); then +if (! hasArg --configure-only) && (completeBuild || hasArg libcuml || hasArg prims || hasArg bench || hasArg cpp-mgtests); then cd ${LIBCUML_BUILD_DIR} if [ -n "${INSTALL_TARGET}" ]; then cmake --build ${LIBCUML_BUILD_DIR} -j${PARALLEL_LEVEL} ${build_args} --target ${INSTALL_TARGET} ${VERBOSE_FLAG} @@ -273,14 +273,14 @@ if (! hasArg --nobuild) && (completeBuild || hasArg libcuml || hasArg prims || h fi fi -if (! hasArg --nobuild) && hasArg cppdocs; then +if (! hasArg --configure-only) && hasArg cppdocs; then cd ${LIBCUML_BUILD_DIR} cmake --build ${LIBCUML_BUILD_DIR} --target docs_cuml fi # Build and (optionally) install the cuml Python package -if (! hasArg --nobuild) && (completeBuild || hasArg cuml || hasArg pydocs); then +if (! hasArg --configure-only) && (completeBuild || hasArg cuml || hasArg pydocs); then # Append `-DFIND_CUML_CPP=ON` to CUML_EXTRA_CMAKE_ARGS unless a user specified the option. if [[ "${CUML_EXTRA_CMAKE_ARGS}" != *"DFIND_CUML_CPP"* ]]; then CUML_EXTRA_CMAKE_ARGS="${CUML_EXTRA_CMAKE_ARGS} -DFIND_CUML_CPP=ON" diff --git a/ci/run_clang_tidy.sh b/ci/run_clang_tidy.sh index 8527453506..99559165bb 100755 --- a/ci/run_clang_tidy.sh +++ b/ci/run_clang_tidy.sh @@ -15,7 +15,7 @@ rapids-mamba-retry env create --force -f env.yaml -n clang_tidy # Temporarily allow unbound variables for conda activation. set +u && conda activate clang_tidy && set -u -./build.sh --nobuild libcuml +./build.sh --configure-only libcuml rapids-logger "Run clang-tidy" From 6c7d28431abe4326e1c802157e2653a9d3ac9f12 Mon Sep 17 00:00:00 2001 From: Simon Adorf Date: Wed, 12 Jul 2023 08:44:12 -0700 Subject: [PATCH 25/30] Allow configuration of run-clang-tidy via pyproject.toml. To make it easier for developers to reproducibly run it. --- ci/run_clang_tidy.sh | 2 +- cpp/scripts/run-clang-tidy.py | 29 +++++++++++++++++++++++++++++ dependencies.yaml | 1 + pyproject.toml | 4 ++++ 4 files changed, 35 insertions(+), 1 deletion(-) diff --git a/ci/run_clang_tidy.sh b/ci/run_clang_tidy.sh index 99559165bb..96cc03ab89 100755 --- a/ci/run_clang_tidy.sh +++ b/ci/run_clang_tidy.sh @@ -19,4 +19,4 @@ set +u && conda activate clang_tidy && set -u rapids-logger "Run clang-tidy" -python cpp/scripts/run-clang-tidy.py -ignore '[.]cu$|_deps|examples/kmeans/' +python cpp/scripts/run-clang-tidy.py diff --git a/cpp/scripts/run-clang-tidy.py b/cpp/scripts/run-clang-tidy.py index 6741abcebd..12972f4e58 100755 --- a/cpp/scripts/run-clang-tidy.py +++ b/cpp/scripts/run-clang-tidy.py @@ -21,6 +21,9 @@ import json import multiprocessing as mp import shutil +from pathlib import Path + +import tomli EXPECTED_VERSION = "15.0.7" @@ -30,6 +33,17 @@ SEPARATOR = "-" * 16 +def _read_config_file(config_file): + try: + return tomli.loads(Path(config_file).read_text())["tool"]["run-clang-tidy"] + except KeyError: + return {} + except tomli.TOMLDecodeError as error: + raise RuntimeError( + f"Failed to read config file '{config_file}' due to syntax error: {error}" + ) + + def parse_args(): argparser = argparse.ArgumentParser("Runs clang-tidy on a project") argparser.add_argument( @@ -59,7 +73,22 @@ def parse_args(): argparser.add_argument( "-j", type=int, default=-1, help="Number of parallel jobs to launch." ) + argparser.add_argument( + "-c", "--config", type=str, help="Path to config file (default=pyproject.toml)." + ) args = argparser.parse_args() + + # Read from config file. + try: + config = _read_config_file(args.config or "./pyproject.toml") + except FileNotFoundError: + if args.config: + # only raise if config argument was explicitly used + raise RuntimeError(f"File '{args.config}' does not exist.") + else: + argparser.set_defaults(**config) + args = argparser.parse_args() + if args.j <= 0: args.j = mp.cpu_count() args.ignore_compiled = re.compile(args.ignore) if args.ignore else None diff --git a/dependencies.yaml b/dependencies.yaml index bac14209ab..99964da7d0 100644 --- a/dependencies.yaml +++ b/dependencies.yaml @@ -94,6 +94,7 @@ dependencies: - clang==15.0.7 - clang-tools==15.0.7 - ninja + - tomli common_build: common: - output_types: [conda, requirements, pyproject] diff --git a/pyproject.toml b/pyproject.toml index a6e20329b5..802575f1bb 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -14,3 +14,7 @@ quiet-level = 3 # TODO: Re-enable E501 with a reasonable line length max-line-length = 999 ignore = ['E501'] + + +[tool.run-clang-tidy] +ignore = "[.]cu$|_deps|examples/kmeans/" From c4e656531d62b497f9e758f0dcc2839019ece1f4 Mon Sep 17 00:00:00 2001 From: Simon Adorf Date: Wed, 12 Jul 2023 13:26:33 -0700 Subject: [PATCH 26/30] Add instructions on how to run clang-tidy locally. Adds dedicated conda environment. --- CONTRIBUTING.md | 40 +++++++++++++++++++ ci/run_clang_tidy.sh | 2 +- .../clang_tidy_cuda-118_arch-x86_64.yaml | 37 +++++++++++++++++ dependencies.yaml | 5 ++- 4 files changed, 82 insertions(+), 2 deletions(-) create mode 100644 conda/environments/clang_tidy_cuda-118_arch-x86_64.yaml diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 0baec5a219..0c383cb985 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -115,6 +115,46 @@ please see the `.pre-commit-config.yaml` file. of files are up-to-date and in the correct format. - `codespell`: Checks for spelling mistakes +### Clang-tidy + +In order to maintain high-quality code, cuML uses not only pre-commit hooks +featuring various formatters and linters but also the clang-tidy tool. +Clang-tidy is designed to detect potential issues within the C and C++ code. It +is typically run as part of our continuous integration (CI) process. + +While it's generally unnecessary for contributors to run clang-tidy locally, +there might be cases where you would want to do so. There are two primary +methods to run clang-tidy on your local machine: using Docker or Conda. + +* **Docker** + + 1. Navigate to the repository root directory. + 2. Run the following Docker command: + + ```bash + docker run --rm --pull always \ + --mount type=bind,source="$(pwd)",target=/opt/repo --workdir /opt/repo \ + rapidsai/ci:latest /opt/repo/ci/run_clang_tidy.sh + ``` + + +* **Conda** + + 1. Navigate to the repository root directory. + 2. Create and activate the needed conda environment: + ```bash + conda env create --force -n cuml-clang-tidy -f conda/environments/clang_tidy_cuda-118_arch-x86_64.yaml + conda activate cuml-clang-tidy + ``` + 3. Generate the compile command database with + ```bash + ./build.sh --configure-only libcuml + ``` + 3. Run clang-tidy with the following command: + ```bash + python cpp/scripts/run-clang-tidy.py --config pyproject.toml + ``` + ### Managing PR labels Each PR must be labeled according to whether it is a "breaking" or "non-breaking" change (using Github labels). This is used to highlight changes that users should know about when upgrading. diff --git a/ci/run_clang_tidy.sh b/ci/run_clang_tidy.sh index 96cc03ab89..10c3347f6b 100755 --- a/ci/run_clang_tidy.sh +++ b/ci/run_clang_tidy.sh @@ -19,4 +19,4 @@ set +u && conda activate clang_tidy && set -u rapids-logger "Run clang-tidy" -python cpp/scripts/run-clang-tidy.py +python cpp/scripts/run-clang-tidy.py --config pyproject.toml diff --git a/conda/environments/clang_tidy_cuda-118_arch-x86_64.yaml b/conda/environments/clang_tidy_cuda-118_arch-x86_64.yaml new file mode 100644 index 0000000000..b37e6bf455 --- /dev/null +++ b/conda/environments/clang_tidy_cuda-118_arch-x86_64.yaml @@ -0,0 +1,37 @@ +# This file is generated by `rapids-dependency-file-generator`. +# To make changes, edit ../../dependencies.yaml and run `rapids-dependency-file-generator`. +channels: +- rapidsai +- rapidsai-nightly +- dask/label/dev +- conda-forge +- nvidia +dependencies: +- c-compiler +- clang-tools==15.0.7 +- clang==15.0.7 +- cmake>=3.26.4 +- cudatoolkit=11.8 +- cxx-compiler +- gcc_linux-64=11.* +- gmock>=1.13.0 +- gtest>=1.13.0 +- libcublas-dev=11.11.3.6 +- libcublas=11.11.3.6 +- libcufft-dev=10.9.0.58 +- libcufft=10.9.0.58 +- libcumlprims==23.8.* +- libcurand-dev=10.3.0.86 +- libcurand=10.3.0.86 +- libcusolver-dev=11.4.1.48 +- libcusolver=11.4.1.48 +- libcusparse-dev=11.7.5.86 +- libcusparse=11.7.5.86 +- libraft-headers==23.8.* +- libraft==23.8.* +- librmm==23.8.* +- ninja +- nvcc_linux-64=11.8 +- sysroot_linux-64==2.17 +- tomli +name: clang_tidy_cuda-118_arch-x86_64 diff --git a/dependencies.yaml b/dependencies.yaml index 99964da7d0..0915e3c2ab 100644 --- a/dependencies.yaml +++ b/dependencies.yaml @@ -27,7 +27,10 @@ files: - checks - py_version clang_tidy: - output: none + output: conda + matrix: + cuda: ["11.8"] + arch: [x86_64] includes: - clang_tidy - common_build From 403ff5c728d81181dcfe8904f2b3ef8a575302e1 Mon Sep 17 00:00:00 2001 From: Simon Adorf Date: Wed, 12 Jul 2023 13:29:47 -0700 Subject: [PATCH 27/30] Return with non-zero exit code if clang-tidy is not found. --- cpp/scripts/run-clang-tidy.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/cpp/scripts/run-clang-tidy.py b/cpp/scripts/run-clang-tidy.py index 12972f4e58..1a51e220cb 100755 --- a/cpp/scripts/run-clang-tidy.py +++ b/cpp/scripts/run-clang-tidy.py @@ -16,6 +16,7 @@ from __future__ import print_function import re import os +import sys import subprocess import argparse import json @@ -295,8 +296,11 @@ def main(): raise Exception("This needs to always be run from the root of repo") # Check whether clang-tidy exists if shutil.which("clang-tidy") is None: - print("clang-tidy not found. Exiting...") - return + print( + "Unable to find the clang-tidy executable, is it installed?", + file=sys.stderr, + ) + sys.exit(1) all_files = get_all_commands(args.cdb) status = run_tidy_for_all_files(args, all_files) if not status: From c3828a14b34bb6ae2f48cea3ee2e22581b0ddf2b Mon Sep 17 00:00:00 2001 From: Simon Adorf Date: Wed, 12 Jul 2023 13:34:01 -0700 Subject: [PATCH 28/30] Adjust import order. --- cpp/scripts/run-clang-tidy.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/cpp/scripts/run-clang-tidy.py b/cpp/scripts/run-clang-tidy.py index 1a51e220cb..67189573f9 100755 --- a/cpp/scripts/run-clang-tidy.py +++ b/cpp/scripts/run-clang-tidy.py @@ -13,20 +13,18 @@ # limitations under the License. # -from __future__ import print_function -import re -import os -import sys -import subprocess import argparse import json import multiprocessing as mp +import os +import re import shutil +import subprocess +import sys from pathlib import Path import tomli - EXPECTED_VERSION = "15.0.7" VERSION_REGEX = re.compile(r" LLVM version ([0-9.]+)") GPU_ARCH_REGEX = re.compile(r"sm_(\d+)") From 3af96ce2c97f11ecdbde1a9aba9291bd37c85f21 Mon Sep 17 00:00:00 2001 From: Simon Adorf Date: Thu, 13 Jul 2023 11:43:22 -0700 Subject: [PATCH 29/30] Update environment file. --- conda/environments/clang_tidy_cuda-118_arch-x86_64.yaml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/conda/environments/clang_tidy_cuda-118_arch-x86_64.yaml b/conda/environments/clang_tidy_cuda-118_arch-x86_64.yaml index b37e6bf455..02d0fdb0d7 100644 --- a/conda/environments/clang_tidy_cuda-118_arch-x86_64.yaml +++ b/conda/environments/clang_tidy_cuda-118_arch-x86_64.yaml @@ -11,7 +11,8 @@ dependencies: - clang-tools==15.0.7 - clang==15.0.7 - cmake>=3.26.4 -- cudatoolkit=11.8 +- cuda-version=11.8 +- cudatoolkit - cxx-compiler - gcc_linux-64=11.* - gmock>=1.13.0 From 9ff16dd0047cbbdeb18db818840f6a16edac16e4 Mon Sep 17 00:00:00 2001 From: Simon Adorf Date: Thu, 13 Jul 2023 11:53:23 -0700 Subject: [PATCH 30/30] Add SCCACHE_S3_NO_CREDENTIALS=1 to docker run instructions. --- CONTRIBUTING.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 0c383cb985..b8ef5e9f7a 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -134,6 +134,7 @@ methods to run clang-tidy on your local machine: using Docker or Conda. ```bash docker run --rm --pull always \ --mount type=bind,source="$(pwd)",target=/opt/repo --workdir /opt/repo \ + -e SCCACHE_S3_NO_CREDENTIALS=1 \ rapidsai/ci:latest /opt/repo/ci/run_clang_tidy.sh ```