Skip to content
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

Branch 23.10 merge 23.08 #1672

Merged
merged 9 commits into from
Jul 25, 2023

Conversation

vyasr
Copy link
Contributor

@vyasr vyasr commented Jul 25, 2023

Replaces #1660

achirkin and others added 9 commits July 20, 2023 19:52
…apidsai#1657)

This PR changes the behavior of ANN benchmark `dataset.h` to defer reading the data until it is definitely needed. This allows to avoid copying/reading huge datasets in search benchmarks when the database (index) is already built.

Authors:
  - Artem M. Chirkin (https://github.com/achirkin)

Approvers:
  - Tamas Bela Feher (https://github.com/tfeher)

URL: rapidsai#1657
This PR always enables CUDA language support in pylibraft's CMakeLists.txt, to work around an issue I encountered while building.

I am trying to build pylibraft in rapids-compose and saw an error in the CMake support for enabling CUDA.

```
CMake Error at ~/rapids1/compose/etc/conda/cuda_12.0/envs/rapids/share/cmake-3.26/Modules/CMakeDetermineCUDACompiler.cmake:279 (message):
  CMAKE_CUDA_ARCHITECTURES:

    NATIVE

  is not one of the following:

    * a semicolon-separated list of integers, each optionally
      followed by '-real' or '-virtual'
    * a special value: all, all-major, native

Call Stack (most recent call first):
  ~/rapids1/raft/cpp/build/release/raft-config.cmake:171 (enable_language)
  CMakeLists.txt:39 (find_package)
```

This error indicates that `rapids_cuda_init_architectures(pylibraft)` isn't being called, which handles [some preprocessing for this architectures variable so that CMake recognizes the results](https://github.com/rapidsai/rapids-cmake/blob/37c650803d997a85cf522384d42c3275a240e47e/rapids-cmake/cuda/init_architectures.cmake#L95-L97).

It seems like the error shown below comes from [this part of libraft's CMakeLists.txt](https://github.com/rapidsai/raft/blob/06b3aa0919cc9b3a14e656773c1ce7cbe5b7ac73/cpp/CMakeLists.txt#L590-L598) where the CUDA language is being enabled without having first called `rapids_cuda_init_architectures`.

Currently, `rapids_cuda_init_architectures` is called and the CUDA language is [only enabled in pylibraft when `raft_FOUND` is false](https://github.com/rapidsai/raft/blob/06b3aa0919cc9b3a14e656773c1ce7cbe5b7ac73/python/pylibraft/CMakeLists.txt#L53-L61). However, I'm building in an environment where raft _can_ be found. It seems like we always need to enable CUDA support due to a requirement in libraft itself, based on the code I see in [cuML](https://github.com/rapidsai/cuml/blob/a47c73e04676f6a831f498b6ded58dac72b385c8/python/CMakeLists.txt#L21-L24) and [cuGraph](https://github.com/rapidsai/cugraph/blob/803b854fd43be7375d6354ed005001725c7710b4/python/cugraph/CMakeLists.txt#L21-L24). Therefore, I copied a similar change into pylibraft in this PR, and it appears to work.

Authors:
  - Bradley Dice (https://github.com/bdice)

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

URL: rapidsai#1659
This PR improves CAGRA ANN benchmarks:
- fixes search itopk parameter handling
- adds more search and build parameters
- improves logging

Authors:
  - Tamas Bela Feher (https://github.com/tfeher)

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

URL: rapidsai#1658
Corrects multiple test issues uncovered by running tests in parallel. Splits the matrix and neighbor tests into multiple executables to improve throughput

Previous performance on 1 GPU: 830sec
New performance on 1 GPU: 692sec
New performance on 2 GPU: 418sec

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

Approvers:
  - Vyas Ramasubramani (https://github.com/vyasr)
  - Corey J. Nolet (https://github.com/cjnolet)
  - Ray Douglass (https://github.com/raydouglass)

URL: rapidsai#1623
During testing of a new feature in cugraph I discovered that the method required either MPI comms or UCP.  I have an application that has neither.

This PR modifies the `comm_split` implementation to continue using `allgather` when performing the split instead of using `allgather` followed by UCP comms.

Authors:
  - Chuck Hastings (https://github.com/ChuckHastings)

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

URL: rapidsai#1649
This PR formally renames the namespaces to promote CAGRA from experimental. It's a lot of very small changes but the approach was the following:

1. Rename all occurrences of `raft::neighbors::experimental::cagra` to `raft::neighbors::cagra`
2. Create new namespaces in each of the public API headers for CAGRA that export `raft::neighbors::cagra` to `raft::neighbors::experimental::cagra`
3. Create an issue to eventually remove the deprecated `raft::neighbors::experimental::cagra` namespace
4. Reference issue in TODO comments in the corresponding files to remove the experimental namespaces after a couple releases.

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

Approvers:
  - Bradley Dice (https://github.com/bdice)
  - Tamas Bela Feher (https://github.com/tfeher)

URL: rapidsai#1666
Replace fused L2 Nearest Neighbors in `connect_components` with masked NN.
Closes rapidsai#743
Closes rapidsai#1569

Authors:
  - Tarang Jain (https://github.com/tarang-jain)
  - Corey J. Nolet (https://github.com/cjnolet)

Approvers:
  - Corey J. Nolet (https://github.com/cjnolet)
  - Allard Hendriksen (https://github.com/ahendriksen)

URL: rapidsai#1445
@vyasr vyasr requested review from a team as code owners July 25, 2023 23:17
@vyasr vyasr added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Jul 25, 2023
@ajschmidt8
Copy link
Member

admin merging this PR since it's time sensitive

@ajschmidt8 ajschmidt8 merged commit 80e498e into rapidsai:branch-23.10 Jul 25, 2023
@vyasr vyasr deleted the branch-23.10-merge-23.08 branch July 25, 2023 23:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci CMake cpp improvement Improvement / enhancement to an existing function non-breaking Non-breaking change python
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants