Skip to content

[Java] Test indexing and serialization with integral (byte) dataset#1366

Merged
rapids-bot[bot] merged 5 commits intorapidsai:mainfrom
ldematte:java/int-types-tests
Oct 26, 2025
Merged

[Java] Test indexing and serialization with integral (byte) dataset#1366
rapids-bot[bot] merged 5 commits intorapidsai:mainfrom
ldematte:java/int-types-tests

Conversation

@ldematte
Copy link
Copy Markdown
Contributor

@ldematte ldematte commented Sep 24, 2025

Some additional tests to increase our test coverage.

Good to review, but we might want to expand this after #1283 (or vice-versa, whichever comes first).

@ldematte ldematte requested a review from a team as a code owner September 24, 2025 10:28
@copy-pr-bot
Copy link
Copy Markdown

copy-pr-bot Bot commented Sep 24, 2025

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@benfred benfred added improvement Improves an existing functionality non-breaking Introduces a non-breaking change labels Sep 24, 2025
@cjnolet
Copy link
Copy Markdown
Member

cjnolet commented Sep 26, 2025

/ok to test 4a9f56c

@mythrocks
Copy link
Copy Markdown
Contributor

mythrocks commented Oct 1, 2025

Looks like this is going to need a rebase.

@cjnolet will keep me honest: Given the code-freeze, maybe we should retarget this to 25.12.

@ldematte
Copy link
Copy Markdown
Contributor Author

ldematte commented Oct 2, 2025

@mythrocks @cjnolet I can do that, but at the moment it seems that 25.12 is behind 25.10, so you probably want to incorporate all changes from 25.10 before I rebase, right?

@ldematte
Test indexing and serialization with integral (byte) dataset
8c775c7 
# Conflicts:
#	java/cuvs-java/src/test/java/com/nvidia/cuvs/CagraBuildAndSearchIT.java
#	java/cuvs-java/src/test/java/com/nvidia/cuvs/CuVSMatrixIT.java
@ldematte ldematte force-pushed the java/int-types-tests branch from 7775b8f to 8c775c7 Compare October 21, 2025 12:59
@ldematte ldematte requested review from a team as code owners October 21, 2025 12:59
@ldematte ldematte changed the base branch from branch-25.10 to branch-25.12 October 21, 2025 12:59
@ldematte ldematte changed the base branch from branch-25.12 to main October 21, 2025 14:37
@mythrocks
Copy link
Copy Markdown
Contributor

mythrocks commented Oct 21, 2025

/ok to test 9a9c811

@mythrocks mythrocks removed the request for review from a team October 21, 2025 18:10
@mythrocks mythrocks removed request for a team October 21, 2025 18:10
@mythrocks mythrocks removed request for a team and KyleFromNVIDIA October 21, 2025 18:10
mythrocks
mythrocks reviewed Oct 21, 2025
View reviewed changes
Comment thread java/cuvs-java/src/test/java/com/nvidia/cuvs/CagraBuildAndSearchIT.java
Comment on lines +137 to +147
try (var index = indexOnce(CuVSMatrix.ofArray(dataset), resources)) {
var indexPath = serializeOnce(index);
try (var loadedIndex = deserializeOnce(indexPath, resources)) {
queryAndCompare(
index,
loadedIndex,
SearchResults.IDENTITY_MAPPING,
queries,
expectedResults,
resources);
Files.deleteIfExists(indexPath);
Copy link
Copy Markdown
Contributor

@mythrocks mythrocks Oct 21, 2025

Choose a reason for hiding this comment

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

👍

mythrocks
mythrocks approved these changes Oct 21, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@mythrocks mythrocks left a comment

Choose a reason for hiding this comment

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

This looks good to me.

I've removed a bunch of aliases from the reviewer-list. They might have been added as an artifact of rebasing this PR.

I'll merge this once the CI passes.

@mythrocks
Copy link
Copy Markdown
Contributor

mythrocks commented Oct 21, 2025

/ok to test cd2ae5f

@ldematte
Copy link
Copy Markdown
Contributor Author

ldematte commented Oct 22, 2025

Hey @mythrocks , test failure says:

Caused by: java.lang.AssertionError: Exception while executing runnable: java.lang.RuntimeException: cuvsCagraBuild returned 0[RAFT failure at file=/tmp/conda-bld-output/bld/rattler-build_libcuvs/work/cpp/src/neighbors/detail/cagra/graph_core.cuh line=1417: Could not generate an intermediate CAGRA graph because the initial kNN graph contains too many invalid or duplicated neighbor nodes. This error can occur, for example, if too many overflows occur during the norm computation between the dataset vectors.

This is due to the fact that we generate data randomly. I wonder if there is a way to avoid this? The error mentions too many overflows; maybe limit the range of the generated floats? Keep them between a given min and max?

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Oct 23, 2025

Important

Review skipped

Auto reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Comment @coderabbitai help to get the list of available commands and usage tips.

@mythrocks
Copy link
Copy Markdown
Contributor

mythrocks commented Oct 23, 2025

The error mentions too many overflows; maybe limit the range of the generated floats? Keep them between a given min and max?

Yes, I think so. This test seems flaky.

I'm re-kicking this PR after updating.

@mythrocks
Copy link
Copy Markdown
Contributor

mythrocks commented Oct 23, 2025

/ok to test 4062549

@ldematte
Copy link
Copy Markdown
Contributor Author

ldematte commented Oct 24, 2025

Yes, I think so. This test seems flaky.

Any suggestion on a sensible range to use as input? E.g. keep them between 0 and 1.0 or ...?

@mythrocks
Copy link
Copy Markdown
Contributor

mythrocks commented Oct 24, 2025

/ok to test bbddabf

@mythrocks
Copy link
Copy Markdown
Contributor

mythrocks commented Oct 26, 2025

Let's pick up the flaky test in another issue.

@mythrocks mythrocks changed the title [Review][Java] Test indexing and serialization with integral (byte) dataset [Java] Test indexing and serialization with integral (byte) dataset Oct 26, 2025
@mythrocks
Copy link
Copy Markdown
Contributor

mythrocks commented Oct 26, 2025

/merge

@rapids-bot rapids-bot Bot merged commit 4442a3f into rapidsai:main Oct 26, 2025
84 checks passed
This was referenced Oct 28, 2025
Closed
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mythrocks mythrocks mythrocks approved these changes

Assignees

@ldematte ldematte

Labels

improvement Improves an existing functionality non-breaking Introduces a non-breaking change testing

Projects

Unstructured Data Processing
Status: Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@ldematte @cjnolet @mythrocks @benfred