-
Notifications
You must be signed in to change notification settings - Fork 197
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Updating README #1836
Updating README #1836
Conversation
Updating installing instructions, bringing it into parity with the docs. Including reference to CUDA version with Conda. Pointing readers to the docs for building and installing from source.
Updated header for building from source.
Fixing typos Co-authored-by: Bradley Dice <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think overall, I'd like to not remove the C++ usage instructions nor give the impression that RAFT is solely a Python library. I think we should mention that we have benchmarking packages and docker containers and then link out to the raft_ann_benchmarks.md
file for more information about installing and using the docker containers.
RAFT is a C++ library w/ Python bindings. Even if we expose a C API or Java API, RAFT is still going to be a C++ library at its core, just like the rest of the C++ libraries in RAPIDS. I think users should expect to see a very quick overview of the ways to use RAFT on the front page and then follow a link to more comprehensive guide. We just need to make sure it's up to date and you've outlined here a couple places where they weren't updated.
``` | ||
|
||
You can also install the conda packages individually using the `mamba` command above. | ||
|
||
After installing RAFT, `find_package(raft COMPONENTS compiled distributed)` can be used in your CUDA/C++ cmake build to compile and/or link against needed dependencies in your raft target. `COMPONENTS` are optional and will depend on the packages installed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one is correct. nn and distance don't exist anymore.
``` | ||
|
||
You can also install the conda packages individually using the `mamba` command above. | ||
|
||
After installing RAFT, `find_package(raft COMPONENTS compiled distributed)` can be used in your CUDA/C++ cmake build to compile and/or link against needed dependencies in your raft target. `COMPONENTS` are optional and will depend on the packages installed. | ||
After installing RAFT, `find_package(raft COMPONENTS nn distance)` can be used in your CUDA/C++ cmake build to compile and/or link against needed dependencies in your raft target. `COMPONENTS` are optional and will depend on the packages installed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This components no longer exist. If they are still in the build instructions elsewhere then that needs to be updated.
|
||
#### Example Template Project | ||
|
||
You can find an [example RAFT](cpp/template/README.md) project template in the `cpp/template` directory, which demonstrates how to build a new application with RAFT or incorporate RAFT into an existing cmake project. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would not remove the C++ installation instructions from the front page. There's a slight difference between the use-cases of using raft through cmake
and building raft from source
. We include both in the installation and build guide
but we only include the former in the readme. This is pretty standard practive. As RAFT is a C++ library at its core, and many users use it through C++, I'd prefer not to remove the quick and dirty samples from the front page.
Perhaps we could adjust the verbiage a tad, though. Many cmake users these days are going to be familiar w/ the cmake package manager (CPM), however we could make the verbiage more friendly by maybe renaming the section title to "Using RAFT through Cmake".
- `libraft-headers` RAFT headers | ||
- `libraft` (optional) shared library of pre-compiled template instantiations and runtime APIs. | ||
- `libraft-headers` RAFT headers. | ||
- `libraft` (optional) shared library containing pre-compiled template instantiations and runtime APIs. | ||
- `pylibraft` (optional) Python wrappers around RAFT algorithms and primitives. | ||
- `raft-dask` (optional) enables deployment of multi-node multi-GPU algorithms that use RAFT `raft::comms` in Dask clusters. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We probably want to add raft-ann-bench
and raft-ann-bench-cpu
here as well, since those are conda/python packages that ship w/ raft.
Additional CMake targets can be made available by adding components in the table below to the `RAFT_COMPONENTS` list above, separated by spaces. The `raft::raft` target will always be available. RAFT headers require, at a minimum, the CUDA toolkit libraries and RMM dependencies. | ||
|
||
| Component | Target | Description | Base Dependencies | | ||
|-------------|---------------------|----------------------------------------------------------|----------------------------------------| |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the reasons outlined above, I'd prefer not to remove this from the docs. For cmake users, these are very important.
I also think should keep the Source
section below, and continue pointing the user to the build and install guide for more comprehensive instructions.
Duplicate. Closing. |
The installation instructions in the readme contains a bug. The Conda instructions should include a reference to the CUDA version. Also, the build from source instructions were out of sync with the docs, so I'm pointing the reader to the docs instead of including all of the instructions in the readme.