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

[RELEASE] raft v23.08 #1692

Merged
merged 76 commits into from
Aug 9, 2023
Merged

[RELEASE] raft v23.08 #1692

merged 76 commits into from
Aug 9, 2023

Conversation

raydouglass
Copy link
Member

❄️ Code freeze for branch-23.08 and v23.08 release

What does this mean?

Only critical/hotfix level issues should be merged into branch-23.08 until release (merging of this PR).

What is the purpose of this PR?

  • Update documentation
  • Allow testing for the new release
  • Enable a means to merge branch-23.08 into main for the release

raydouglass and others added 30 commits May 19, 2023 09:51
Forward-merge branch-23.06 to branch-23.08
Forward-merge branch-23.06 to branch-23.08
This PR fixes some outdated pinnings in `branch-23.08` and applies some fixes to `update-version.sh`.

See also:
rapidsai/cuml#5440
#1554 (comment)

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

Approvers:
  - Ray Douglass (https://github.com/raydouglass)
  - Divye Gala (https://github.com/divyegala)
  - AJ Schmidt (https://github.com/ajschmidt8)

URL: #1556
We recently created new scripts for building documentation with GitHub Actions.

This PR removes the old scripts that were used by Jenkins and are no longer in use.

Authors:
  - AJ Schmidt (https://github.com/ajschmidt8)

Approvers:
  - Ray Douglass (https://github.com/raydouglass)

URL: #1570
This PR changes CAGRA `knn_graph_sort` function to use `raft::util::bitnic_sort` instead of a custom sorting function.

Rel: #1503 (comment)

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

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

URL: #1550
This PR adds support to __half and nb_bfloat16 to most math functions:

- cos
- sin
- exp
- log
- max
- min
- sqrt

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

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

URL: #1554
…anup (#1541)

The PR does the following:
* Introduces `ivf_flat::search_with_filtering()` call in the same way the filtering was introduced to ivf_pq in #1513 
* Moves `sample_filter.cuh` from `raft/neighbor/detail` to `raft/neighbor`
* Moves `NoneSampleFilter` from `raft::neighbor::ivf_pq::detail` namespace to `raft::neighbor::filtering` namespace
* Renames `NoneSampleFilter` to `NoneIvfSampleFilter` and template argument `SampleFilterT` to `IvfSampleFilterT`
* Adds a missing `resource::get_workspace_resource(handle)` in `ivf_flat-inl.cuh` in a `search_with_filtering()` call (which was copied from `search()` call with the same problem)
* Adds more comments in `ivf_pq-inl.h`
* Some code cleanup in `ivf_pq-inl.h`

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

Approvers:
  - Artem M. Chirkin (https://github.com/achirkin)
  - Corey J. Nolet (https://github.com/cjnolet)

URL: #1541
Forward-merge branch-23.06 to branch-23.08
… test (#1574)

This PR unpins `dask` and `distributed` to `>=2023.5.1` for `23.08` development.

xref: rapidsai/cudf#13508

The offending test was using an rmm::device_scalar for some memory that should have been a vector. Not sure how this didn't fail in the past but these changes fix it.

Authors:
  - GALI PREM SAGAR (https://github.com/galipremsagar)
  - Corey J. Nolet (https://github.com/cjnolet)

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

URL: #1574
Forward-merge branch-23.06 to branch-23.08
This PR adds the identity matrix function `eye` (will be used in LOBPCG) and fixes a check on the layout for norm function.

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

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

URL: #1548
This PR updates the `build_docs.sh` script to use the new consolidatory `rapids-upload-script` [shared script](rapidsai/gha-tools#56). 

The shared script enables docs uploads to applicable S3 buckets for branch. nightly and PR builds.

Authors:
  - Jake Awe (https://github.com/AyodeAwe)
  - Corey J. Nolet (https://github.com/cjnolet)
  - AJ Schmidt (https://github.com/ajschmidt8)

Approvers:
  - AJ Schmidt (https://github.com/ajschmidt8)

URL: #1578
The `#if _RAFT_HAS_CUDA` references introduced in [this commit](6bc237f) seems to have broken the `clang-tidy-check` that runs in `cugraph-ops`, as shown in the cugraph GHA logs

This PR updates all the `#if _RAFT_HAS_CUDA` references to `#if defined(_RAFT_HAS_CUDA)` as shown in the [official docs](https://en.cppreference.com/w/cpp/preprocessor/conditional).

Authors:
  - Jake Awe (https://github.com/AyodeAwe)

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

URL: #1582
This PR adds padding to the dataset (if necessary) to make reading any of its rows compatible with 128bit vectorized loads. This change also enables handling arbitrary number of input features (before this PR each row had to be at least 64bit aligned, which constrained the acceptable number of input features).

Fixes #1458.

With this change, it is sufficient to keep a single "load type" specialization for the search kernels, which shall cut the binary size by half (#1459).

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

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

URL: #1505
Remove scikit-build upper bound pinning

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

Approvers:
  - AJ Schmidt (https://github.com/ajschmidt8)
  - Ben Frederickson (https://github.com/benfred)

URL: #1547
The template argument `Size` of `raft::util::bitnic_sort` must be power-of-two but is set to three in the small usage example in the source code. This PR is only for documentation improvement and does not change any behavior.

Authors:
  - tsuki (https://github.com/enp1s0)

Approvers:
  - Artem M. Chirkin (https://github.com/achirkin)
  - Corey J. Nolet (https://github.com/cjnolet)

URL: #1580
The reasoning behind this PR is as follows:
for now, anyone wanting to use `RAFT_CUDA_TRY_NO_THROW` still needs to include `cudart_utils.hpp` which can be costly (compilation) due to the include of `memory_pool.hpp`.
By moving the macros to the essentials, we should not break anything for anyone, but allow anyone to improve compilation times by including the essentials only. At the same time, it should add minimal overhead to the compilation time of the essentials file since the pre-processor is (usually) fast compared to the rest of the compilation pipeline.

Authors:
  - Matt Joux (https://github.com/MatthiasKohl)

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

URL: #1584
Forward-merge branch-23.06 to branch-23.08
Rather than exit the process immediately, use the `RAFT_FAIL` macro to throw an exception inside the cagra code when certain conditions aren't met

Authors:
  - Ben Frederickson (https://github.com/benfred)

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

URL: #1594
…stance kernels (#1597)

-- This fixes the runtime failure seen in the pytest on arm-A100 config in CI.

Authors:
  - Mahesh Doijade (https://github.com/mdoijade)

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

URL: #1597
This fixes the launch config for epilogue kernels in the GramMatrix computation for large number of columns in the resulting kernel matrix.
This code-path is triggered when predicting scores with a support vector space larger than 262140 which would result in a y-grid-dimension larger than 65535. 

Although this is not a regression this code path might get hit more often now that we allow sparse input data, so we might want to fix it in 23.06 as well.

CC @tfeher .

Authors:
  - Malte Förster (https://github.com/mfoerste4)

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

URL: #1586
## Add scheduler_file argument to support MNMG setup

### Overview:
The primary goal is to provide more flexibility and adaptability in how the Dask cluster for testing is configured.

### Changes:



1. **Allow connecting to an existing cluster** 
- The creation of the `LocalCUDACluster` instances is now contingent on the presence of a `SCHEDULER_FILE` environment variable. If this variable exists, the path to the Dask scheduler file is returned instead of creating a new cluster. This change allows the use of pre-existing clusters specified via the `SCHEDULER_FILE` environment variable.
   
   
 2.  **Remove UCX related flags as they are no longer needed**
- Removed specific flags (`enable_tcp_over_ucx`, `enable_nvlink`, `enable_infiniband`) previously used to initialize the `LocalCUDACluster`.  This is because since `Dask-CUDA 22.02` and `UCX >= 1.11.1` we dont need those. 
  See docs:  https://docs.rapids.ai/api/dask-cuda/nightly/examples/ucx/#localcudacluster-with-automatic-configuration




This could help in situations where test scenarios need to be conducted on a specific pre-existing cluster (especially for MNMG setups) . 


### Testing:

I tested using the following setup: 
Start Cluster:
```
dask scheduler --scheduler-file /raid/vjawa/scheduler.json &
dask-cuda-worker --scheduler-file /raid/vjawa/scheduler.json
```

Run Tests:
```
export SCHEDULER_FILE=/raid/vjawa/scheduler.json 
cd /home/nfs/vjawa/raft/python/raft-dask/raft_dask/test
pytest .
```

Authors:
  - Vibhu Jawa (https://github.com/VibhuJawa)

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

URL: #1593
This PR renames the `cagra::prune` function to `cagra::optimize` since it adds reverse edges other than pruning unimportant edges.

Authors:
  - tsuki (https://github.com/enp1s0)

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

URL: #1588
This PR adds the RAFT ANN benchmark for CAGRA

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

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

URL: #1552
Forward-merge branch-23.06 to branch-23.08
cjnolet and others added 11 commits July 24, 2023 19:32
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: #1666
Replace fused L2 Nearest Neighbors in `connect_components` with masked NN.
Closes #743
Closes #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: #1445
Wrap the `rmm::mr::device_memory_resource` returned by `get_workspace_resource` into a `rmm::mr::limiting_resource_adaptor`. This allows to control how much memory is dedicated for the temporary allocations within raft and query this limit in the code (`mr->get_allocation_limit()` - `mr->get_allocated_bytes()`).

**Breaking change**: workspace resource passed in arguments everywhere via a shared_ptr instead of a raw pointer. This affects `set_workspace_resource` function and the constructors of `device_resources` and `handle_t`. Luckily, it's optional everywhere and is likely rarely used, so the impact should be low.

Resolves #1310

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

Approvers:
  - Tamas Bela Feher (https://github.com/tfeher)
  - Corey J. Nolet (https://github.com/cjnolet)

URL: #1356
This PR decouples CAGRA `IdxT` from the mdspan `IndexType` used in CAGRA API, by making the following change
`mdspan<Type, IdxT> -> mdspan<Type, int64_t>`.

In `cagra::index<T, IdxT>` the index type `IdxT` is the type that represents the neighbor indices returned during the ANN search, and the same type is also used as to store the neighborhood graph.

The build and search method take `mdspan` objects as input/output argument. These `mdspan`s have an `IndexType` that is used during offset calculation when indexing the matrix. Prior to this PR, we used `IdxT` as the mdspan index type.

There is a strong motivation to keep `IdxT` as `uint32_t` in order to minimize the memory footprint of the KNN graph. At the same time, the size and offset calculations of mdspans that represent dataset and knn graph require 64-bit indexing for large datasets. This PR changes the mdspan extent `IndexType` to `int64_t`.

This PR does not affect the performance of CAGRA: the search kernels already used correct index type. Only a few operations are affected by this change, and these are outside perf sensitive regions.

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

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

URL: #1664
PR #1356 increased the maximum internal batch size of IVF-PQ and, by doing so, exposed a bug of uint32_t integer overflow that resulted in incorrectly allocated intermediate buffers.
This PR fixes the original bug, but also:
  - Proofs the places where `max_samples` multiplied by something could cause integer overflow
  - Make more careful estimation of the workspace size to avoid `out_of_memory` errors from the limiting resource adaptor
  - Removes an unused argument from `compute_similarity_kernel` which has been slipping between code updates for a really long time

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

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

URL: #1685
The name `search_width` is more expressive, therefore this PR renames `num_parents` to `search_width`.

The original CAGRA implementation used both names internally, but externally it used `search_width`.

Authors:
  - Tamas Bela Feher (https://github.com/tfeher)
  - Corey J. Nolet (https://github.com/cjnolet)

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

URL: #1676
To achieve high QPS with small batch size, it is recommended to use the `multi_cta` algorithm variant. CAGRA has an input parameter to specify which algorithm to use, but until now the benchmarks only used the default algorithm.

This PR adds configuration option for changing the algorithm parameter for the CAGRA ANN benchmark wrappers. This will be useful while benchmarking various configurations with CAGRA.

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

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

URL: #1687
The parameter `rmm::mr::device_memory_resource* mr` is missing while passing down.

Authors:
  - JiayuSun (https://github.com/ucassjy)

Approvers:
  - Ben Frederickson (https://github.com/benfred)

URL: #1682
raydouglass and others added 3 commits July 28, 2023 20:22
* Remove push condition on docs-build

* Update docs-build to match cudf
First verstion of a CAGRA API in pylibraft. 

Todos:

- [x] C++ raft_runtime instantiations and void overloads
- [x] Cython API
- [x] Solve issue of `cagra_types.hpp` including `#include <raft/util/pow2_utils.cuh>` that makes it need nvcc, blocking a clean C++ only cython build
- [x] Check in pytests
- [x] Add examples to docstrings 
- [x] Accommodate for parameter rename of #1676
- [x] Accomodate changes of #1664 
- [x] Move out of experimental namespace

Authors:
   - Dante Gama Dessavre (https://github.com/dantegd)
   - Corey J. Nolet (https://github.com/cjnolet)

Approvers:
   - Corey J. Nolet (https://github.com/cjnolet)
Add a new folder `notebooks` and a notebook with an example of tweaking ivf-pq using a dataset from ann-benchmarks.com

Authors:
   - Artem M. Chirkin (https://github.com/achirkin)
   - Corey J. Nolet (https://github.com/cjnolet)

Approvers:
   - Corey J. Nolet (https://github.com/cjnolet)
@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

lowener and others added 4 commits July 31, 2023 21:36
Support reading interleaved lists from a packed non-interleaved dense IVF-list matrix and vice versa.

Authors:
   - Tarang Jain (https://github.com/tarang-jain)

Approvers:
   - Corey J. Nolet (https://github.com/cjnolet)
This PR pins `dask` & `distributed` to `2023.7.1` version for `23.08` release.

xref: rapidsai/cudf#13802

Authors:
   - GALI PREM SAGAR (https://github.com/galipremsagar)

Approvers:
   - Ray Douglass (https://github.com/raydouglass)
   - Peter Andreas Entschev (https://github.com/pentschev)
@codecov-commenter
Copy link

codecov-commenter commented Aug 2, 2023

Codecov Report

Patch coverage has no change and project coverage change: +0.09% 🎉

Comparison is base (42167ad) 87.98% compared to head (7a6fe79) 88.08%.
Report is 36 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1692      +/-   ##
==========================================
+ Coverage   87.98%   88.08%   +0.09%     
==========================================
  Files          21       22       +1     
  Lines         491      495       +4     
==========================================
+ Hits          432      436       +4     
  Misses         59       59              
Files Changed Coverage Δ
python/pylibraft/pylibraft/__init__.py 100.00% <ø> (ø)
python/pylibraft/pylibraft/common/ai_wrapper.py 89.28% <ø> (+0.39%) ⬆️
python/pylibraft/pylibraft/common/cai_wrapper.py 100.00% <ø> (ø)
python/pylibraft/pylibraft/neighbors/__init__.py 100.00% <ø> (ø)
...on/pylibraft/pylibraft/neighbors/cagra/__init__.py 100.00% <ø> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@raydouglass raydouglass merged commit 5797ef5 into main Aug 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.