Skip to content

Commit

Permalink
Clean Up #include Dependencies (#3402)
Browse files Browse the repository at this point in the history
Closes #3376

This PR moves a few header files around to fix bad dependencies from `include` -> `src`. Moving forward, the only allowed `#include` direction will be:

- `src` -> `include`
- `src` -> `src_prims`
- `src_prims` -> `include`

To facilitate this, a few changes needed to be made:
1. Functions for the C-API have been separated into their own `*_api.cpp` file (some were combined with C++ files)
2. `host_buffer`, `device_buffer`, `hostAllocator` and `deviceAllocator` were moved from `src_prims` to `include`
3. `#include <common/cumlHandle.hpp>` has been removed from all C++ files.

This PR includes some additional improvements:
- Updated the `include_checker.py` script:
   - Added functionality to check for badly placed `#pragma once`
   - Added functionality to fix some of the existing warnings
   - General refactor to support more checks
- Fixed all incorrect uses of `#pragma once`
- Fixed incorrect uses of `using namespace ...` in header files outside of a namespace
- Removed some unnecessary `#include`

While this touches a lot of files, the actual number of changes is relatively small. Below is a before/after comparison of the include graphs:

**`src/include`:**
Before:
![image](https://user-images.githubusercontent.com/42954918/105561661-ab32fe00-5cd4-11eb-8850-bfeaffaba60f.png)
After:
![image](https://user-images.githubusercontent.com/42954918/105561673-b4bc6600-5cd4-11eb-8bb7-04be91f902b6.png)

**`src/src_prims`:**
Before:
![image](https://user-images.githubusercontent.com/42954918/105561616-79ba3280-5cd4-11eb-8a49-1d0c030df7c1.png)
After:
![image](https://user-images.githubusercontent.com/42954918/105561633-89397b80-5cd4-11eb-9702-27b2a809834b.png)

Authors:
  - Michael Demoret (@mdemoret-nv)

Approvers:
  - Dante Gama Dessavre (@dantegd)
  - John Zedlewski (@JohnZed)
  - Thejaswi. N. S (@teju85)

URL: #3402
  • Loading branch information
mdemoret-nv authored Feb 5, 2021
1 parent dec134a commit d49dd1a
Show file tree
Hide file tree
Showing 191 changed files with 996 additions and 703 deletions.
37 changes: 29 additions & 8 deletions cpp/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,9 @@ set(CMAKE_MODULE_PATH ${CMAKE_CURRENT_BINARY_DIR}/cmake)
# https://github.com/rapidsai/cuml/issues/2194
find_package(CUDA 10.0 REQUIRED)

# Set the -isystem flag to not have an '=' sign to allow parsing of compile_commands.json
set(CMAKE_INCLUDE_SYSTEM_FLAG_CUDA "-isystem ")

if (NOT DISABLE_OPENMP OR NOT ${DISABLE_OPENMP})
find_package(OpenMP)
if(OPENMP_FOUND)
Expand Down Expand Up @@ -324,16 +327,18 @@ include(cmake/Dependencies.cmake)

set(CUML_INCLUDE_DIRECTORIES
${CUML_INCLUDE_DIR}
${CMAKE_CURRENT_SOURCE_DIR}/src
${CMAKE_CURRENT_SOURCE_DIR}/src_prims
${CMAKE_CURRENT_SOURCE_DIR}/test/prims
${CMAKE_CUDA_TOOLKIT_INCLUDE_DIRECTORIES}
${CUTLASS_DIR}/src/cutlass
${SPDLOG_DIR}/src/spdlog/include
${FAISS_INCLUDE_DIRS}
${RAFT_DIR}/cpp/include
${RMM_INCLUDE_DIRS})

# These directories will be used by dependent targets
set(CUML_SRC_DIR ${CMAKE_CURRENT_SOURCE_DIR}/src)
set(CUML_SRC_PRIMS_DIR ${CMAKE_CURRENT_SOURCE_DIR}/src_prims)
set(CUML_TEST_PRIMS_DIR ${CMAKE_CURRENT_SOURCE_DIR}/test/prims)

if(NOT CUB_IS_PART_OF_CTK)
list(APPEND CUML_INCLUDE_DIRECTORIES ${CUB_DIR}/src/cub)
endif(NOT CUB_IS_PART_OF_CTK)
Expand Down Expand Up @@ -386,8 +391,6 @@ if(BUILD_CUML_CPP_LIBRARY)
add_library(${CUML_CPP_TARGET} SHARED
src/arima/batched_arima.cu
src/arima/batched_kalman.cu
src/common/cumlHandle.cpp
src/common/cuml_api.cpp
src/common/logger.cpp
src/common/nvtx.cu
src/datasets/make_arima.cu
Expand Down Expand Up @@ -462,15 +465,24 @@ if(BUILD_CUML_CPP_LIBRARY)
link_directories(${CUDA_TOOLKIT_ROOT_DIR}/lib64)
endif(NVTX)

target_compile_definitions(${CUML_CPP_TARGET}
PRIVATE
CUML_CPP_API)

target_include_directories(${CUML_CPP_TARGET}
PRIVATE ${CUML_INCLUDE_DIRECTORIES})
PUBLIC
${CUML_INCLUDE_DIRECTORIES}
PRIVATE
${CUML_SRC_DIR}
${CUML_SRC_PRIMS_DIR})

target_link_libraries(${CUML_CPP_TARGET}
PUBLIC
${CUML_PUBLIC_LINK_LIBRARIES}
PRIVATE
${CUML_PRIVATE_LINK_LIBRARIES}
)

# If we export the libdmlc symbols, they can lead to weird crashes with other
# libraries that use libdmlc. This just hides the symbols internally.
target_link_options(${CUML_CPP_TARGET} PRIVATE "-Wl,--exclude-libs,libdmlc.a")
Expand All @@ -486,14 +498,23 @@ if(BUILD_CUML_C_LIBRARY)
set(CUML_C_TARGET "cuml")

add_library(${CUML_C_TARGET} SHARED
src/common/cumlHandle.cpp
src/common/cuml_api.cpp
src/dbscan/dbscan_api.cpp
src/glm/glm_api.cpp
src/holtwinters/holtwinters_api.cpp
src/svm/svm_api.cpp)
src/knn/knn_api.cpp
src/svm/svm_api.cpp
)

target_compile_definitions(${CUML_C_TARGET}
PRIVATE
CUML_C_API)

target_include_directories(${CUML_C_TARGET}
PRIVATE ${CUML_INCLUDE_DIRECTORIES})
PRIVATE
${CUML_SRC_DIR}
)

target_link_libraries(${CUML_C_TARGET} PUBLIC ${CUML_CPP_TARGET})
endif(BUILD_CUML_C_LIBRARY)
Expand Down
24 changes: 16 additions & 8 deletions cpp/bench/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
#=============================================================================
# Copyright (c) 2019-2020, NVIDIA CORPORATION.
# Copyright (c) 2019-2021, NVIDIA CORPORATION.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -41,8 +41,10 @@ if(BUILD_CUML_BENCH)
benchmarklib)

target_include_directories(sg_benchmark
PRIVATE ${GBENCH_DIR}/include
${CUML_INCLUDE_DIRECTORIES})
PRIVATE
${GBENCH_DIR}/include
${CUML_SRC_PRIMS_DIR}
)
endif(BUILD_CUML_BENCH)

##############################################################################
Expand All @@ -64,17 +66,23 @@ if(BUILD_CUML_PRIMS_BENCH)
prims/matrix_vector_op.cu
prims/permute.cu
prims/reduce.cu
prims/rng.cu
../src/common/logger.cpp) # because prims is header only!
prims/rng.cu)

if(NOT CUB_IS_PART_OF_CTK)
add_dependencies(prims_benchmark cub)
endif(NOT CUB_IS_PART_OF_CTK)
add_dependencies(prims_benchmark spdlog)

target_link_libraries(prims_benchmark ${CUDA_cublas_LIBRARY} benchmarklib)
target_link_libraries(prims_benchmark
PUBLIC
${CUDA_cublas_LIBRARY}
benchmarklib
${CUML_CPP_TARGET}
)

target_include_directories(prims_benchmark
PRIVATE ${GBENCH_DIR}/include
${CUML_INCLUDE_DIRECTORIES})
PRIVATE
${GBENCH_DIR}/include
${CUML_SRC_PRIMS_DIR}
)
endif(BUILD_CUML_PRIMS_BENCH)
3 changes: 1 addition & 2 deletions cpp/bench/sg/dataset.cuh
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2019-2020, NVIDIA CORPORATION.
* Copyright (c) 2019-2021, NVIDIA CORPORATION.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -18,7 +18,6 @@

#include <raft/cudart_utils.h>
#include <raft/linalg/transpose.h>
#include <common/cumlHandle.hpp>
#include <cuml/cuml.hpp>
#include <cuml/datasets/make_blobs.hpp>
#include <fstream>
Expand Down
3 changes: 1 addition & 2 deletions cpp/bench/sg/dataset_ts.cuh
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2020, NVIDIA CORPORATION.
* Copyright (c) 2020-2021, NVIDIA CORPORATION.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -16,7 +16,6 @@

#pragma once

#include <common/cumlHandle.hpp>
#include <cuml/cuml.hpp>
#include <raft/cuda_utils.cuh>

Expand Down
2 changes: 1 addition & 1 deletion cpp/bench/sg/dbscan.cu
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
*/

#include <cuml/cluster/dbscan.hpp>
#include <cuml/cuml.hpp>

#include <utility>
#include "benchmark.cuh"

Expand Down
2 changes: 1 addition & 1 deletion cpp/bench/sg/fil.cu
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@
*/

#include <cuml/fil/fil.h>

#include <cuml/tree/algo_helper.h>
#include <decisiontree/decisiontree_impl.h>
#include <treelite/c_api.h>
#include <treelite/tree.h>
#include <cuml/common/logger.hpp>
Expand Down
2 changes: 1 addition & 1 deletion cpp/cmake/Dependencies.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ else(DEFINED ENV{RAFT_PATH})

ExternalProject_Add(raft
GIT_REPOSITORY https://github.com/rapidsai/raft.git
GIT_TAG a61cbed999ec3bb9143bbd2c6c455f946d9e1330
GIT_TAG 4a79adcb0c0e87964dcdc9b9122f242b5235b702
PREFIX ${RAFT_DIR}
CONFIGURE_COMMAND ""
BUILD_COMMAND ""
Expand Down
File renamed without changes.
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2019, NVIDIA CORPORATION.
* Copyright (c) 2019-2021, NVIDIA CORPORATION.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -16,7 +16,6 @@

#pragma once

#include <cuml/common/cuml_allocator.hpp>
#include <raft/mr/device/buffer.hpp>

namespace MLCommon {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2019, NVIDIA CORPORATION.
* Copyright (c) 2019-2021, NVIDIA CORPORATION.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down
15 changes: 13 additions & 2 deletions cpp/include/cuml/cuml_api.h
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2019, NVIDIA CORPORATION.
* Copyright (c) 2019-2021, NVIDIA CORPORATION.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -20,13 +20,24 @@

#include <cuda_runtime_api.h>

// Block inclusion of this header when compiling libcuml++.so. If this error is
// shown during compilation, there is an issue with how the `#include` have
// been set up. To debug the issue, run `./build.sh cppdocs` and open the page
// 'cpp/build/html/cuml__api_8h.html' in a browser. This will show which files
// directly and indirectly include this file. Only files ending in '*_api' or
// 'cumlHandle' should include this header.
#ifdef CUML_CPP_API
#error \
"This header is only for the C-API and should not be included from the C++ API."
#endif

#ifdef __cplusplus
extern "C" {
#endif

typedef int cumlHandle_t;

typedef enum {
typedef enum cumlError_t {
CUML_SUCCESS,
CUML_ERROR_UNKNOWN,
CUML_INVALID_HANDLE
Expand Down
5 changes: 1 addition & 4 deletions cpp/include/cuml/decomposition/pca_mg.hpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2020, NVIDIA CORPORATION.
* Copyright (c) 2020-2021, NVIDIA CORPORATION.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -19,9 +19,6 @@
#include <opg/matrix/data.hpp>
#include <opg/matrix/part_descriptor.hpp>
#include "pca.hpp"

#include <common/cumlHandle.hpp>

namespace ML {

enum class mg_solver { COV_EIG_DQ, COV_EIG_JACOBI, QR };
Expand Down
4 changes: 2 additions & 2 deletions cpp/include/cuml/decomposition/sign_flip_mg.hpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2020, NVIDIA CORPORATION.
* Copyright (c) 2020-2021, NVIDIA CORPORATION.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -16,9 +16,9 @@

#pragma once

#include <common/cumlHandle.hpp>
#include <opg/matrix/data.hpp>
#include <opg/matrix/part_descriptor.hpp>
#include <raft/handle.hpp>

namespace ML {
namespace PCA {
Expand Down
4 changes: 1 addition & 3 deletions cpp/include/cuml/decomposition/tsvd_mg.hpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2020, NVIDIA CORPORATION.
* Copyright (c) 2020-2021, NVIDIA CORPORATION.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -20,8 +20,6 @@
#include <opg/matrix/part_descriptor.hpp>
#include "tsvd.hpp"

#include <common/cumlHandle.hpp>

namespace ML {
namespace TSVD {
namespace opg {
Expand Down
4 changes: 2 additions & 2 deletions cpp/include/cuml/linear_model/glm.hpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2018-2020, NVIDIA CORPORATION.
* Copyright (c) 2018-2021, NVIDIA CORPORATION.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -15,7 +15,7 @@
*/
#pragma once

#include <common/cumlHandle.hpp>
#include <raft/handle.hpp>

namespace ML {
namespace GLM {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2019, NVIDIA CORPORATION.
* Copyright (c) 2019-2021, NVIDIA CORPORATION.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -16,6 +16,7 @@
#pragma once

#include <cuml/cuml_api.h>
#include <stdbool.h>

#ifdef __cplusplus
extern "C" {
Expand Down
4 changes: 1 addition & 3 deletions cpp/include/cuml/linear_model/ols_mg.hpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2020 NVIDIA CORPORATION.
* Copyright (c) 2020-2021, NVIDIA CORPORATION.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -20,8 +20,6 @@
#include <opg/matrix/data.hpp>
#include <opg/matrix/part_descriptor.hpp>

#include <common/cumlHandle.hpp>

namespace ML {
namespace OLS {
namespace opg {
Expand Down
4 changes: 2 additions & 2 deletions cpp/include/cuml/linear_model/preprocess_mg.hpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2020 NVIDIA CORPORATION.
* Copyright (c) 2020-2021, NVIDIA CORPORATION.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -16,11 +16,11 @@

#pragma once

#include <common/cumlHandle.hpp>
#include <cuml/common/cuml_allocator.hpp>
#include <opg/matrix/data.hpp>
#include <opg/matrix/part_descriptor.hpp>
#include <raft/comms/comms.hpp>
#include <raft/handle.hpp>

namespace ML {
namespace GLM {
Expand Down
4 changes: 1 addition & 3 deletions cpp/include/cuml/linear_model/ridge_mg.hpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2020 NVIDIA CORPORATION.
* Copyright (c) 2020-2021, NVIDIA CORPORATION.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -20,8 +20,6 @@
#include <opg/matrix/part_descriptor.hpp>
#include "glm.hpp"

#include <common/cumlHandle.hpp>

namespace ML {
namespace Ridge {
namespace opg {
Expand Down
1 change: 0 additions & 1 deletion cpp/include/cuml/neighbors/knn.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@

#include <faiss/gpu/GpuIndex.h>
#include <faiss/gpu/StandardGpuResources.h>
#include <common/cumlHandle.hpp>
#include <cuml/common/logger.hpp>
#include <cuml/cuml.hpp>

Expand Down
Loading

0 comments on commit d49dd1a

Please sign in to comment.