Skip to content

Additional Distances for CAGRA C and Python API#546

Merged
rapids-bot[bot] merged 2 commits into
NVIDIA:branch-25.02from
tarang-jain:cagra-c-python-metric
Dec 19, 2024
Merged

Additional Distances for CAGRA C and Python API#546
rapids-bot[bot] merged 2 commits into
NVIDIA:branch-25.02from
tarang-jain:cagra-c-python-metric

Conversation

@tarang-jain

Copy link
Copy Markdown
Contributor

Add InnerProduct metric to CAGRA C and Python API + updates to CAGRA pytests.
Closes #545

@tarang-jain tarang-jain requested review from a team as code owners December 19, 2024 01:14
@tarang-jain tarang-jain self-assigned this Dec 19, 2024
@tarang-jain tarang-jain added C Python non-breaking Introduces a non-breaking change improvement Improves an existing functionality and removed cpp Python labels Dec 19, 2024
# self.params.metric = _get_metric(metric)
# self.params.metric_arg = 0
self._metric = metric
self.params.metric = <cuvsDistanceType>DISTANCE_TYPES[metric]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can you check if the dictionary has an entry for "sqeuclidean" and not "euclidean"? I'm not sure how the tests were working until now.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The tests until now were using the L2Expanded metric for CAGRA irrespective of what was being passed as the metric. "sqeuclidean" translates to L2Expanded and "euclidean" translates to L2SqrtExpanded. The min recall bound in the pytests so far was quite low (=0.7) which is why even if the groundtruth was created using inner product, the tests were passing. I've checked precisely what cuvs distance type is being dispatched to the C++ level.

Comment thread python/cuvs/cuvs/neighbors/cagra/cagra.pyx
@github-actions github-actions Bot added the cpp label Dec 19, 2024
@cjnolet

cjnolet commented Dec 19, 2024

Copy link
Copy Markdown
Contributor

/merge

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C cpp improvement Improves an existing functionality non-breaking Introduces a non-breaking change Python

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[FEA] Inner Product distance for CAGRA in python package

4 participants