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.

Sign up for GitHub

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

Jump to bottom

ANN refinement Python wrapper #1052

Merged
merged 9 commits into from
Dec 1, 2022
Merged

ANN refinement Python wrapper #1052

merged 9 commits into from
Dec 1, 2022

Conversation

tfeher
Copy link
Contributor

@tfeher tfeher commented Nov 29, 2022 •
edited
Loading

This PR adds Python wrapper for the ANN refinement method. Refinement can work both with device dataset and host dataset.

@tfeher tfeher requested a review from achirkin November 29, 2022 14:13
@tfeher tfeher requested review from a team as code owners November 29, 2022 14:13
@tfeher tfeher added enhancement New feature or request feature request New feature or request non-breaking Non-breaking change and removed enhancement New feature or request feature request New feature or request labels Nov 29, 2022
@tfeher
Copy link
Contributor Author

tfeher commented Nov 29, 2022 •
edited
Loading

Todo

  • support refinement using host dataset

@tfeher tfeher changed the title ANN Python wrapper update ANN refinement Python wrapper Nov 30, 2022
cjnolet
cjnolet reviewed Nov 30, 2022
View reviewed changes
python/pylibraft/pylibraft/common/mdspan.pxd
from cython.operator cimport dereference as deref


cdef extern from "raft/thirdparty/mdspan/include/experimental/__p0009_bits/layout_stride.hpp" namespace "std::experimental": # noqa: E501
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cc @benfred for awareness. This looks very similar to #1016 but keeps the LayoutPolicy, which will prove to be useful generally.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The two pxd files are very similar otherwise, so I don't think there's much concern for conflicts across the two.

@tfeher tfeher mentioned this pull request Nov 30, 2022
Closed
cjnolet
cjnolet requested changes Nov 30, 2022
View reviewed changes
Copy link
Member

@cjnolet cjnolet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this looks good. Just a couple very minor things, really. If you think we should push them off to a follow-on, I don't mind approving.

tfeher
tfeher commented Dec 1, 2022
View reviewed changes
Copy link
Contributor Author

@tfeher tfeher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @cjnolet and @achirkin for the reviews, I have updated the PR.

@cjnolet
Copy link
Member

cjnolet commented Dec 1, 2022

@gpucibot merge

@rapids-bot rapids-bot bot merged commit 20457ce into rapidsai:branch-22.12 Dec 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@achirkin achirkin achirkin left review comments

@cjnolet cjnolet cjnolet approved these changes

Assignees
No one assigned
Labels
CMake cpp feature request New feature or request non-breaking Non-breaking change python
Projects
VS/ML/DM Primitives Release Board
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@tfeher @cjnolet @achirkin