-
Notifications
You must be signed in to change notification settings - Fork 71
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
[WIP] Expose extend()
in C API
#276
Open
ajit283
wants to merge
94
commits into
rapidsai:branch-25.02
Choose a base branch
from
ajit283:extend-c-api
base: branch-25.02
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 16 commits
Commits
Show all changes
94 commits
Select commit
Hold shift + click to select a range
7961798
wip
ajit283 764dc1e
cleanup
ajit283 b0fd0b1
Added support for different data types and device allocation in NewMa…
ajit283 b47cf5b
bruteforce works
ajit283 1ec0c8c
refactor
ajit283 d5c0dc3
improve resources
ajit283 cc6319e
add ivf_flat, modules not working yet
ajit283 01af7c9
sync
ajit283 ed9bf47
cleanup
ajit283 4e79d8e
Merge branch 'rapidsai:branch-24.08' into go
ajit283 d278abc
rename
ajit283 b91721d
sync
ajit283 6b90861
.
ajit283 773fd94
Merge branch 'rapidsai:branch-24.08' into go
ajit283 7cbc1f9
sync
ajit283 46ec2f7
ivf_pq + cagra
ajit283 9dcffbd
Merge branch 'branch-24.08' into go
ajit283 f82216c
CI/CD attempt
ajit283 486d40a
add distance
ajit283 6f5c5a6
replace string arguments
ajit283 bfdf3be
make library easier to use
ajit283 de3cea0
graph degrees
ajit283 57e8dc0
fix top-level tests
ajit283 ab173dc
Merge branch 'branch-24.08' into go
cjnolet 2d5fb95
renaming
ajit283 505d8dc
Merge branch 'go' of https://github.com/ajit283/cuvs into go
ajit283 c3360ee
change package name
ajit283 e261c8a
package name change (rapidsai)
ajit283 daaebdf
first try
ajit283 a4890ed
dlpack add expand
ajit283 e5a3dcc
fix comments
ajit283 f2bac2d
cagra: expose some types
ajit283 a684b01
Merge branch 'branch-24.08' into go
ajit283 34837b6
Merge branch 'branch-24.10' into extend-c-api
cjnolet ccb5d86
fix test
ajit283 c78e86f
add return tensor
ajit283 7d45e24
extend
ajit283 a7084c2
add extend with return (wip, test missing)
ajit283 3af1b73
Merge branch 'branch-24.10' into go
cjnolet 44d9e58
fix brute_force, add pool
ajit283 862ecc5
Merge branch 'branch-24.10' into extend-c-api
ajit283 49379a0
Merge branch 'branch-24.10' into extend-c-api
cjnolet 3db15ff
Merge branch 'branch-24.12' into extend-c-api
ajit283 9447f63
update, add search_width
ajit283 04b6532
add memory resource test
ajit283 a082f67
thread_local fix
ajit283 a84e764
change pool memory
ajit283 b0d4686
Merge branch 'branch-24.12' into extend-c-api
cjnolet 853d538
Merge branch 'branch-24.10' into go
cjnolet 4021229
Merge branch 'branch-24.12' into go
cjnolet 6dd2044
simplify memory_resource
ajit283 3e33692
Merge branch 'go' of https://github.com/ajit283/cuvs into go
ajit283 59bb8ca
.
ajit283 2b10a23
fix documentation
ajit283 8be2d05
proper test
ajit283 f37c58e
fix test
ajit283 4fd78ca
cleanup
ajit283 11428ea
Merge branch 'branch-24.12' into extend-c-api
ajit283 b1a0476
Merge branch 'branch-24.12' into go
cjnolet e36d556
Merge branch 'branch-24.12' into c-expose-filtering
ajit283 13dba54
fixes, add issue
ajit283 fd270b0
Merge branch 'branch-24.12' into extend-c-api
ajit283 a2c5033
add TODO comment
ajit283 dddb165
Merge branch 'branch-24.12' into go
ajit283 e05782a
cleanup top-level packages
ajit283 a315632
cleanup neighbors
ajit283 3c97864
.
ajit283 bd2dd76
build
ajit283 bd76cf8
ci
ajit283 1e5a756
ci
ajit283 927f2be
fix pointer pinning issues
ajit283 fad44c2
address comments
ajit283 0d0184a
.
ajit283 e3d33a0
Update cpp/src/neighbors/cagra_c.cpp
ajit283 d0dc961
Merge branch 'branch-24.12' into c-expose-filtering
benfred 102aee5
Merge branch 'branch-24.12' into extend-c-api
ajit283 3b17895
fix filter repr.
ajit283 f7fac35
add docstrings
ajit283 d814e37
Merge branch 'branch-24.12' into go
ajit283 4e584cc
Merge branch 'branch-24.12' into c-expose-filtering
lowener 7999cd6
Merge branch 'branch-24.12' into extend-c-api
cjnolet 21924ad
Merge branch 'branch-24.12' into extend-c-api
ajit283 1d61a90
Merge branch 'branch-25.02' into extend-c-api
ajit283 f30b57c
adjust
ajit283 b1fdcb6
adjust 2
ajit283 4a340bc
adjust 3
ajit283 a245fd1
adjust 4
ajit283 2708bc0
Merge branch 'branch-25.02' into go
ajit283 4e2202e
Merge branch 'branch-25.02' into c-expose-filtering
ajit283 05bf3c8
Merge pull request #7 from ajit283/c-expose-filtering
ajit283 efeaf39
Merge pull request #6 from ajit283/go
ajit283 6edcf93
Merge branch 'integration-branch' into extend-c-api
ajit283 311dd82
Revert "Merge branch 'integration-branch' into extend-c-api"
ajit283 feaef9b
Merge branch 'branch-25.02' into extend-c-api
cjnolet File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 doesn't need to be changed in this PR (especially since the rest of cagra/ivf-* doesn't follow this right now) - but I think we should start re-using the C-api parameter structs in the C++ api , like we are doing in the distances here
cuvs/cpp/include/cuvs/distance/distance.hpp
Line 27 in 72154b0
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.
+1 to this. It increases the maintenance burden when we have parallel objects like this.
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.
@ajit283 as Ben mentioned, it doesn't have to be changed in this PR, but can you create a Github issue for this and reference the issue number in a TODO here in the code? This way we won't lose sight of it.
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.
One problem that arises when we do this is that we have to include the C header file in the C++ header file. While it works for distances enum, here the issue is that we also have dlpack headers which we don't link to the C++ target.
So we'll have to pull out all the structs into another file to make it work.
@cjnolet @benfred