Skip to content

Commit

Permalink
Moving a few more things around (#822)
Browse files Browse the repository at this point in the history
1. Moving `lap` -> `raft::solver`, `mst` -> `raft::sparse::solver`, `single_linkage` -> `raft::cluster`. 

2. This also establishes a convention for headers which include definitions for various API vocabulary types, such as knn indices, distance enum types, etc... 


3. Common utilities are moved into the raft/util directory. 

4. raft/sparse/selection is renamed to raft/sparse/spatial 

5. Excluding removed and deprecated items from docs where appropriate.

Proper deprecation is done so these changes shouldn't break any users downstream.

Authors:
  - Corey J. Nolet (https://github.com/cjnolet)

Approvers:
  - Dante Gama Dessavre (https://github.com/dantegd)

URL: #822
  • Loading branch information
cjnolet authored Sep 14, 2022
1 parent 0d71ceb commit cc95aed
Show file tree
Hide file tree
Showing 350 changed files with 7,549 additions and 5,374 deletions.
12 changes: 11 additions & 1 deletion DEVELOPER_GUIDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

Devloping features and fixing bugs for the RAFT library itself is straightforward and only requires building and installing the relevant RAFT artifacts.

The process for working on a CUDA/C++ feature which spans RAFT and one or more consumers can vary slightly depending on whether the consuming project relies on a source build (as outlined in the [BUILD](BUILD.md#install_header_only_cpp) docs). In such a case, the option `CPM_raft_SOURCE=/path/to/raft/source` can be passed to the cmake of the consuming project in order to build the local RAFT from source. The PR with relevant changes to the consuming project can also pin the RAFT version temporarily by explicitly changing the `FORK` and `PINNED_TAG` arguments to the RAFT branch containing their changes when invoking `find_and_configure_raft`. The pin should be reverted after the changed is merged to the RAFT project and before it is merged to the dependent project(s) downstream.
The process for working on a CUDA/C++ feature which might span RAFT and one or more consuming libraries can vary slightly depending on whether the consuming project relies on a source build (as outlined in the [BUILD](BUILD.md#install_header_only_cpp) docs). In such a case, the option `CPM_raft_SOURCE=/path/to/raft/source` can be passed to the cmake of the consuming project in order to build the local RAFT from source. The PR with relevant changes to the consuming project can also pin the RAFT version temporarily by explicitly changing the `FORK` and `PINNED_TAG` arguments to the RAFT branch containing their changes when invoking `find_and_configure_raft`. The pin should be reverted after the changed is merged to the RAFT project and before it is merged to the dependent project(s) downstream.

If building a feature which spans projects and not using the source build in cmake, the RAFT changes (both C++ and Python) will need to be installed into the environment of the consuming project before they can be used. The ideal integration of RAFT into consuming projects will enable both the source build in the consuming project only for this case but also rely on a more stable packaging (such as conda packaging) otherwise.

Expand All @@ -14,6 +14,16 @@ Since RAFT is a core library with multiple consumers, it's important that the pu

The public APIs should be lightweight wrappers around calls to private APIs inside the `detail` namespace.

## Common Design Considerations

1. Use the `hpp` extension for files which can be compiled with `gcc` against the CUDA-runtime. Use the `cuh` extension for files which require `nvcc` to be compiled. `hpp` can also be used for functions marked `__host__ __device__` only if proper checks are in place to remove the `__device__` designation when not compiling with `nvcc`.

2. When additional classes, structs, or general POCO types are needed to be used for representing data in the public API, place them in a new file called `<primitive_name>_types.hpp`. This tells users they are safe to expose these types on their own public APIs without bringing in device code. At a minimum, the definitions for these types, at least, should not require `nvcc`. In general, these classes should only store very simple state and should not perform their own computations. Instead, new functions should be exposed on the public API which accept these objects, reading or updating their state as necessary.

3. Documentation for public APIs should be well documented, easy to use, and it is highly preferred that they include usage instructions.

4. Before creating a new primitive, check to see if one exists already. If one exists but the API isn't flexible enough to include your use-case, consider first refactoring the existing primitive. If that is not possible without an extreme number of changes, consider how the public API could be made more flexible. If the new primitive is different enough from all existing primitives, consider whether an existing public API could invoke the new primitive as an option or argument. If the new primitive is different enough from what exists already, add a header for the new public API function to the appropriate subdirectory and namespace.

## Testing

It's important for RAFT to maintain a high test coverage in order to minimize the potential for downstream projects to encounter unexpected build or runtime behavior as a result of changes. A well-defined public API can help maintain compile-time stability but means more focus should be placed on testing the functional requirements and verifying execution on the various edge cases within RAFT itself. Ideally, bug fixes and new features should be able to be made to RAFT independently of the consuming projects.
Expand Down
22 changes: 21 additions & 1 deletion cpp/doxygen/Doxyfile.in
Original file line number Diff line number Diff line change
Expand Up @@ -880,7 +880,27 @@ RECURSIVE = YES
# run.

EXCLUDE = @CMAKE_CURRENT_SOURCE_DIR@/include/raft/sparse/linalg/symmetrize.hpp \
\
@CMAKE_CURRENT_SOURCE_DIR@/include/raft/cache \
@CMAKE_CURRENT_SOURCE_DIR@/include/raft/common \
@CMAKE_CURRENT_SOURCE_DIR@/include/raft/lap \
@CMAKE_CURRENT_SOURCE_DIR@/include/raft/sparse/selection \
@CMAKE_CURRENT_SOURCE_DIR@/include/raft/sparse/csr.hpp \
@CMAKE_CURRENT_SOURCE_DIR@/include/raft/linalg/lanczos.cuh \
@CMAKE_CURRENT_SOURCE_DIR@/include/raft/linalg/lanczos.hpp \
@CMAKE_CURRENT_SOURCE_DIR@/include/raft/cuda_utils.cuh \
@CMAKE_CURRENT_SOURCE_DIR@/include/raft/cudart_utils.h \
@CMAKE_CURRENT_SOURCE_DIR@/include/raft/util/device_atomics.cuh \
@CMAKE_CURRENT_SOURCE_DIR@/include/raft/device_utils.cuh \
@CMAKE_CURRENT_SOURCE_DIR@/include/raft/error.hpp \
@CMAKE_CURRENT_SOURCE_DIR@/include/raft/handle.hpp \
@CMAKE_CURRENT_SOURCE_DIR@/include/raft/integer_utils.h \
@CMAKE_CURRENT_SOURCE_DIR@/include/raft/interruptible.hpp \
@CMAKE_CURRENT_SOURCE_DIR@/include/raft/mdarray.hpp \
@CMAKE_CURRENT_SOURCE_DIR@/include/raft/pow2_utils.cuh \
@CMAKE_CURRENT_SOURCE_DIR@/include/raft/span.hpp \
@CMAKE_CURRENT_SOURCE_DIR@/include/raft/vectorized.cuh \
@CMAKE_CURRENT_SOURCE_DIR@/include/raft/raft.hpp \
@CMAKE_CURRENT_SOURCE_DIR@/include/raft/core/cudart_utils.hpp

# The EXCLUDE_SYMLINKS tag can be used to select whether or not files or
# directories that are symbolic links (a Unix file system feature) are excluded
Expand Down
2 changes: 1 addition & 1 deletion cpp/include/raft.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
/**
* This file is deprecated and will be removed in release 22.06.
*/
#include "raft/handle.hpp"
#include "raft/core/handle.hpp"
#include "raft/mdarray.hpp"
#include "raft/span.hpp"

Expand Down
Loading

0 comments on commit cc95aed

Please sign in to comment.