-
Notifications
You must be signed in to change notification settings - Fork 87
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
Go API - [WIP] #212
Go API - [WIP] #212
Changes from 71 commits
7961798
764dc1e
b0fd0b1
b47cf5b
1ec0c8c
d5c0dc3
cc6319e
01af7c9
ed9bf47
4e79d8e
d278abc
b91721d
6b90861
773fd94
7cbc1f9
46ec2f7
9dcffbd
f82216c
486d40a
6f5c5a6
bfdf3be
de3cea0
57e8dc0
ab173dc
2d5fb95
505d8dc
c3360ee
e261c8a
a4890ed
f2bac2d
a684b01
7d45e24
a7084c2
3af1b73
44d9e58
9447f63
04b6532
a082f67
a84e764
853d538
4021229
6dd2044
3e33692
b1a0476
dddb165
e05782a
a315632
3c97864
bd2dd76
bd76cf8
1e5a756
927f2be
f7fac35
d814e37
2708bc0
e634821
8c9105a
4a80cf7
f5b8e72
7b93510
f738da4
51b5747
ff36031
3a45833
78e9b1a
3287f7c
e2b6104
a656870
b70d6e2
ef2187c
23e67f7
aa38afc
35bc1f1
dba8cfc
5d97c8e
bf50e3f
a93d286
20f41b9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -47,6 +47,19 @@ jobs: | |
node_type: "gpu-v100-latest-1" | ||
run_script: "ci/build_rust.sh" | ||
sha: ${{ inputs.sha }} | ||
go-build: | ||
needs: cpp-build | ||
secrets: inherit | ||
uses: rapidsai/shared-workflows/.github/workflows/[email protected] | ||
with: | ||
build_type: ${{ inputs.build_type || 'branch' }} | ||
branch: ${{ inputs.branch }} | ||
arch: "amd64" | ||
date: ${{ inputs.date }} | ||
container_image: "rapidsai/ci-conda:latest" | ||
node_type: "gpu-v100-latest-1" | ||
run_script: "ci/build_go.sh" | ||
sha: ${{ inputs.sha }} | ||
python-build: | ||
needs: [cpp-build] | ||
secrets: inherit | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,6 +22,7 @@ jobs: | |
- conda-python-tests | ||
- docs-build | ||
- rust-build | ||
- go-build | ||
- wheel-build-libcuvs | ||
- wheel-build-cuvs | ||
- wheel-tests-cuvs | ||
|
@@ -58,13 +59,15 @@ jobs: | |
- '!notebooks/**' | ||
- '!python/**' | ||
- '!rust/**' | ||
- '!go/**' | ||
- '!thirdparty/LICENSES/**' | ||
test_notebooks: | ||
- '**' | ||
- '!.devcontainer/**' | ||
- '!.pre-commit-config.yaml' | ||
- '!README.md' | ||
- '!rust/**' | ||
- '!go/**' | ||
- '!thirdparty/LICENSES/**' | ||
test_python: | ||
- '**' | ||
|
@@ -75,6 +78,7 @@ jobs: | |
- '!img/**' | ||
- '!notebooks/**' | ||
- '!rust/**' | ||
- '!go/**' | ||
- '!thirdparty/LICENSES/**' | ||
checks: | ||
secrets: inherit | ||
|
@@ -136,6 +140,16 @@ jobs: | |
arch: "amd64" | ||
container_image: "rapidsai/ci-conda:latest" | ||
run_script: "ci/build_rust.sh" | ||
go-build: | ||
needs: conda-cpp-build | ||
secrets: inherit | ||
uses: rapidsai/shared-workflows/.github/workflows/[email protected] | ||
with: | ||
build_type: pull-request | ||
node_type: "gpu-v100-latest-1" | ||
arch: "amd64" | ||
container_image: "rapidsai/ci-conda:latest" | ||
run_script: "ci/build_go.sh" | ||
wheel-build-libcuvs: | ||
needs: checks | ||
secrets: inherit | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @benfred do you know how to install/find dlpack? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Seems like we got past the dlpack error - but now I'm seeing
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it seems like it installs the previous release from the conda channels, where some features aren't available yet which it relies on. Is there any way to do this through conda or would cmake be better? @benfred There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hmm - it should be grabbing the built cpp artifacts from S3 from the conda cpp build, but it looks like its installing 24.12 libcuvs instead:
(from the build-go GHA log ). fwiw I just checked the rust build, which is set up very similar - and seems to be pulling the right libcuvs package.
I'm not quite sure why this is going wrong here - @bdice or @vyasr do you have any ideas? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. looking at the difference in build scripts between build_go.sh and build_rust.sh - and I think it might just be that the rust build is specifying the rapids version. $ diff ci/build_rust.sh ci/build_go.sh
13c13
< --file-key rust \
---
> --file-key go \
16c16
< rapids-mamba-retry env create --yes -f env.yaml -n rust
---
> rapids-mamba-retry env create --yes -f env.yaml -n go
21c21
< conda activate rust
---
> conda activate go
26,29c26,28
< # we need to set up LIBCLANG_PATH to allow rust bindgen to work,
< # grab it from the conda env
< export LIBCLANG_PATH=$(dirname $(find /opt/conda -name libclang.so | head -n 1))
< echo "LIBCLANG_PATH=$LIBCLANG_PATH"
---
> export CGO_CFLAGS="-I${CONDA_PREFIX}/include"
> export CGO_LDFLAGS="-L${CONDA_PREFIX}/lib -lcudart -lcuvs -lcuvs_c"
> export CC=clang
37,38c36,38
< "libcuvs=${RAPIDS_VERSION}" \
< "libraft=${RAPIDS_VERSION}"
---
> libcuvs \
> libraft \
> cuvs
40c40
< bash ./build.sh rust
---
> bash ./build.sh go going to try updating the go build to match |
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
@@ -0,0 +1,40 @@ | ||||
#!/bin/bash | ||||
# Copyright (c) 2024, NVIDIA CORPORATION. | ||||
|
||||
set -euo pipefail | ||||
|
||||
rapids-logger "Create test conda environment" | ||||
. /opt/conda/etc/profile.d/conda.sh | ||||
|
||||
RAPIDS_VERSION="$(rapids-version)" | ||||
|
||||
rapids-dependency-file-generator \ | ||||
--output conda \ | ||||
--file-key go \ | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. do you know how I could add this file key to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add a key under Line 70 in 710e9f5
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. More explanation in the README here: https://github.com/rapidsai/dependency-file-generator |
||||
--matrix "cuda=${RAPIDS_CUDA_VERSION%.*};arch=$(arch);py=${RAPIDS_PY_VERSION}" | tee env.yaml | ||||
|
||||
rapids-mamba-retry env create --yes -f env.yaml -n go | ||||
|
||||
# seeing failures on activating the environment here on unbound locals | ||||
# apply workaround from https://github.com/conda/conda/issues/8186#issuecomment-532874667 | ||||
set +eu | ||||
conda activate go | ||||
set -eu | ||||
|
||||
rapids-print-env | ||||
|
||||
export CGO_CFLAGS="-I${CONDA_PREFIX}/include" | ||||
export CGO_LDFLAGS="-L${CONDA_PREFIX}/lib -lcudart -lcuvs -lcuvs_c" | ||||
export CC=clang | ||||
|
||||
rapids-logger "Downloading artifacts from previous jobs" | ||||
CPP_CHANNEL=$(rapids-download-conda-from-s3 cpp) | ||||
|
||||
# installing libcuvs/libraft will speed up the rust build substantially | ||||
rapids-mamba-retry install \ | ||||
--channel "${CPP_CHANNEL}" \ | ||||
libcuvs \ | ||||
libraft \ | ||||
cuvs | ||||
|
||||
bash ./build.sh go |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,83 @@ | ||
package brute_force | ||
|
||
// #include <cuvs/neighbors/brute_force.h> | ||
import "C" | ||
|
||
import ( | ||
"errors" | ||
"unsafe" | ||
|
||
cuvs "github.com/rapidsai/cuvs/go" | ||
) | ||
|
||
// Brute Force KNN Index | ||
type BruteForceIndex struct { | ||
index C.cuvsBruteForceIndex_t | ||
trained bool | ||
} | ||
|
||
// Creates a new empty Brute Force KNN Index | ||
func CreateIndex() (*BruteForceIndex, error) { | ||
var index C.cuvsBruteForceIndex_t | ||
|
||
err := cuvs.CheckCuvs(cuvs.CuvsError(C.cuvsBruteForceIndexCreate(&index))) | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
return &BruteForceIndex{index: index, trained: false}, nil | ||
} | ||
|
||
// Destroys the Brute Force KNN Index | ||
func (index *BruteForceIndex) Close() error { | ||
err := cuvs.CheckCuvs(cuvs.CuvsError(C.cuvsBruteForceIndexDestroy(index.index))) | ||
if err != nil { | ||
return err | ||
} | ||
return nil | ||
} | ||
|
||
// Builds a new Brute Force KNN Index from the dataset for efficient search. | ||
// | ||
// # Arguments | ||
// | ||
// * `Resources` - Resources to use | ||
// * `Dataset` - A row-major matrix on either the host or device to index | ||
// * `metric` - Distance type to use for building the index | ||
// * `metric_arg` - Value of `p` for Minkowski distances - set to 2.0 if not applicable | ||
func BuildIndex[T any](Resources cuvs.Resource, Dataset *cuvs.Tensor[T], metric cuvs.Distance, metric_arg float32, index *BruteForceIndex) error { | ||
CMetric, exists := cuvs.CDistances[metric] | ||
|
||
if !exists { | ||
return errors.New("cuvs: invalid distance metric") | ||
} | ||
|
||
err := cuvs.CheckCuvs(cuvs.CuvsError(C.cuvsBruteForceBuild(C.cuvsResources_t(Resources.Resource), (*C.DLManagedTensor)(unsafe.Pointer(Dataset.C_tensor)), C.cuvsDistanceType(CMetric), C.float(metric_arg), index.index))) | ||
if err != nil { | ||
return err | ||
} | ||
index.trained = true | ||
|
||
return nil | ||
} | ||
|
||
// Perform a Nearest Neighbors search on the Index | ||
// | ||
// # Arguments | ||
// | ||
// * `Resources` - Resources to use | ||
// * `queries` - Tensor in device memory to query for | ||
// * `neighbors` - Tensor in device memory that receives the indices of the nearest neighbors | ||
// * `distances` - Tensor in device memory that receives the distances of the nearest neighbors | ||
func SearchIndex[T any](resources cuvs.Resource, index BruteForceIndex, queries *cuvs.Tensor[T], neighbors *cuvs.Tensor[int64], distances *cuvs.Tensor[T]) error { | ||
if !index.trained { | ||
return errors.New("index needs to be built before calling search") | ||
} | ||
|
||
prefilter := C.cuvsFilter{ | ||
addr: 0, | ||
_type: C.NO_FILTER, | ||
} | ||
|
||
return cuvs.CheckCuvs(cuvs.CuvsError(C.cuvsBruteForceSearch(C.ulong(resources.Resource), index.index, (*C.DLManagedTensor)(unsafe.Pointer(queries.C_tensor)), (*C.DLManagedTensor)(unsafe.Pointer(neighbors.C_tensor)), (*C.DLManagedTensor)(unsafe.Pointer(distances.C_tensor)), prefilter))) | ||
} |
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 am not able to give a full CI/packaging review at the moment, but wanted to mention this.
Build jobs must be CPU only unless the build is less than 3-4 minutes. Otherwise please create binary artifacts from the CPU build, upload them, and use them for GPU testing in a separate job. We are too lax about this already and the GPU CI load is going up with all the Rust/Java/Go language bindings being added to cuVS.
It appears that the Go jobs pass in ~6 minutes but if that changes in the future we may need to work on this.