Add PyLucene integration and CPU/GPU end-to-end tests - #174
Conversation
and finish updating cuvs version
Convert the PyLucene smoke runner to a pytest-backed case matrix covering GPU CAGRA search, CAGRA-built HNSW, and forced CPU HNSW fallback across segment and force-merge topologies. Add named one-layer and three-layer CAGRA-to-HNSW codecs plus writer-path telemetry so the e2e suite can assert which CPU/GPU path was exercised. Tighten sidecar and Lucene delegate-codec validation to avoid misleading expected probe warnings while still failing on unsupported delegate codecs.
nvzm123
changed the title
| @Override | ||
| public KnnVectorsWriter fieldsWriter(SegmentWriteState state) throws IOException { | ||
| var flatWriter = FLAT_VECTORS_FORMAT.fieldsWriter(state); | ||
| System.setProperty( |
There was a problem hiding this comment.
These System.setProperty methods make sense in the context of this PR as a way to communicate between the Java and Python layers. However, they introduce unnecessary overhead for most other applications. Moreover, this approach would not work reliably in a multithreaded environment, since the same property is reused across requests. I think it would be better to separate the telemetry concerns and extract them into a dedicated method that is invoked only when telemetry data is actually needed.
There was a problem hiding this comment.
Good point! Removed the JVM-global system properties from the indexing path. Telemetry is now computed on demand from the vectors format only when the PyLucene test requests it.
cjnolet
added
improvement
| To run the PyLucene pytest smoke suite against a local PyLucene environment: | ||
|
|
||
| ```sh | ||
| ./ci/test_pylucene_smoke.sh |
There was a problem hiding this comment.
I like that we have ci scripts for this , but a user should not have to invoke ci scripts in order to run tests. Rather, we should document how to run these pytests without the need to call scripts inside the CI directory while also providing the scripts in the CI directory for GitHub actions to run automatically.
There was a problem hiding this comment.
@cjnolet I moved the user-facing logic to test_pylucene.sh and kept a thin wrapper under ci/. The current GitHub Actions workflows do not invoke it yet. Did you intend for this PR to add a PyLucene Actions job as well, or is providing the CI entry-point sufficient for now?
| ./ci/test_pylucene_smoke.sh --gpu-e2e | ||
| ``` | ||
|
|
||
| The expanded suite runs the `gpu-basic`, `gpu-segments`, `cpu-hnsw`, and |
There was a problem hiding this comment.
Great description here. How to build and run tests should really be in a separate build and install guide. I think this is okay for now, especially since we are moving cuVS-Lucene to cuVS, but it's something to consider.
| .withCagraGraphBuildAlgo(CagraGraphBuildAlgo.NN_DESCENT) | ||
| .withGraphDegree(CAGRA_GRAPH_DEGREE) | ||
| .withIntermediateGraphDegree(CAGRA_INTERMEDIATE_GRAPH_DEGREE) | ||
| .withHNSWLayer(1) |
There was a problem hiding this comment.
Wasn't this an issue that you resolved? Why layer 1?
There was a problem hiding this comment.
Yes, that issue was resolved. Layer 1 is intentional test coverage, paired with the three-layer case -- that said, I am not sure that these two particular tests will provide much utility
Move the PyLucene suite into a pytest-owned test tree and add explicit coverage for CPU HNSW, CAGRA-built HNSW, and CAGRA search. Cover segment and force-merge topologies, one- and three-layer HNSW, CAGRA search widths, deletions, vectorless documents, filtering, and single-document behavior. Verify execution paths, persisted graph configuration, and brute-force recall with deterministic vectors. Keep the shell runner focused on environment and classpath setup, and document direct pytest and full GPU execution.
Remove redundant edge cases and reuse canonical topology cases for one-layer HNSW and searchWidth=1 coverage. Add selective brute-force-validated filters across CPU HNSW, CAGRA-built HNSW, and ten-segment CAGRA search, then make cpu-hnsw-1-segment the default documented smoke case.
imotov
left a comment
imotov
left a comment
There was a problem hiding this comment.
I'm not sure how comprehensive we want these cases to be, but they all follow the same pattern - index records under some configuration, close the writer, then search. These are solid, deterministic tests, and I don't want to hold up the PR over this, but there are several important use cases where I think the next round of coverage could pay off.
Everything here is single-threaded: SerialMergeScheduler runs merges inline, IndexSearcher is built without an executor, and numMergeWorkers defaults to 1. So CuVSResources, which is managed per-thread, is only ever exercised in its simplest configuration - one thread, one resource.
In production, indexing and searching frequently overlap: new documents arriving, existing documents being updated, deletes landing, merges running (sometimes on several threads), and queries served against the index the whole time. In my experience that's where the tricky bugs live. A couple of other specific gaps:
- No updateDocument/softUpdateDocument at all, so the delete-then-add path is untested even on a single thread.
- No near-real-time search — the reader always opens after the writer closes, so a segment is never read while it's still being written, and deletes are always committed before anything searches. That should be covered by Lucene, but it would be nice to ensure that Readers still play by the book, don't leak any resources and do what Lucene expects them to do.
- No search concurrent with a merge, so a segment is never dropped from under a live searcher. Same here, Lucene should take care of it, but we should cooperate.
Create flat vector writers only when the accelerated writer consumes them. CPU fallback paths construct their own writers, so eager allocation leaked the unused flat writer for both binary and scalar quantization.
Keep the existing Lucene 102 binary-format initialization while preserving the reminder to replace version-specific partial providers in a separate initiative.
nvzm123
changed the title
There was a problem hiding this comment.
This PR Still needs a lot of work and I'm honestly not sure we should hold up the consolidation of cuVS-lucene to cuVS over it.
@nvzm123 please do make sure when you use AI to make changes that you familiarize yourself enough with our conventions that you can guide the AI to follow them. As it is, this PR is very different than how we do things and having to review changes like this becomes time consuming for everyone else. The pytest infra and related docs do not follow convention. Please make sure to follow all conventions in cuVS and other RAPIDS/cuda-x repos (like RAFT, cuml, etc...).
|
|
||
| cuvs_java_jar = Path(os.environ["CUVS_LUCENE_CUVS_JAVA_JAR"]) | ||
| cuvs_lucene_jar = next( | ||
| jar |
There was a problem hiding this comment.
These steps still look overly complex for a user that just wants to use pylucene.
Boilerplate like this should ideally be provided in a Python library or scripts somewhere that can be easily included in their code. Ideally they should just have to import the codec and call codec.forCodec call.
Have you looked at other Pylucene extensions on GitHub to see their "usage" docs?
|
|
||
| #### PyLucene end-to-end tests | ||
|
|
||
| Pytest cases are under `src/test/python`. The parametrized cases and assertions |
There was a problem hiding this comment.
If we need to expend this many words on tests then it's too complicated.
The only thing you need to say about the tests, other than a very terse description, is a 1 or 2-liner copy/paste example of how to run them. Please see the cuVS build docs for and example.
Also, the standard for test file naming should "XXX_test.py".
| `src/test/java/com/nvidia/cuvs/lucene/PyLuceneTestSupport.java` are compiled to | ||
| `target/test-classes` and are not included in the published jar. | ||
|
|
||
| `ci/run_pylucene_pytests.sh` builds the Maven artifacts when requested, resolves |
There was a problem hiding this comment.
Please don't reference CI scripts in user docs. These are specifically for CI, not for user consumption.
| Pytest cases are under `src/test/python`. The parametrized cases and assertions | ||
| are in `test_pylucene_end_to_end.py`; reusable index and search code is in | ||
| `pylucene_test_support.py`. The test-only Java adapters in | ||
| `src/test/java/com/nvidia/cuvs/lucene/PyLuceneTestSupport.java` are compiled to |
There was a problem hiding this comment.
Too much info.. users don't need this.
| `target/test-classes` and are not included in the published jar. | ||
|
|
||
| `ci/run_pylucene_pytests.sh` builds the Maven artifacts when requested, resolves | ||
| the PyLucene classpath inputs, and invokes pytest. `test_pylucene.sh` at the |
There was a problem hiding this comment.
This is not standard practice in cuVS or other cuda-x repos. Please see cuVS build docs- we use a standard consolidated "build.sh" in the root of the repository.
We are in the process of consolidating cuVs-lucene with cuvs, and the build scripts need to be consolidated as well. We do not provide separate shell scripts for simple 1-liner test run commands. As you can imagine that adds to maintenance overhead for very little benefit. The pytest command should be reference in the build docs for running the tests (as a verification the build succeeded).
| Select a focused group or set the minimum document count per scenario: | ||
|
|
||
| ```sh | ||
| ./test_pylucene.sh --cases=cagra-search-widths \ |
There was a problem hiding this comment.
Nope. Too complex. User should not require a complex script with flags just to run pytests.
| @@ -0,0 +1,232 @@ | |||
| #!/bin/bash | |||
|
|
|||
| # SPDX-FileCopyrightText: Copyright (c) 2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved. | |||
There was a problem hiding this comment.
Mentioned above. Remove. Please use pytests from cuVS Python package as a guide- the individual test cases should be parameterized.
| @@ -0,0 +1,22 @@ | |||
| <assembly xmlns="http://maven.apache.org/plugins/maven-assembly-plugin/assembly/1.1.2" | |||
| xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" | |||
| xsi:schemaLocation="http://maven.apache.org/plugins/maven-assembly-plugin/assembly/1.1.2 http://maven.apache.org/xsd/assembly-1.1.2.xsd"> | |||
There was a problem hiding this comment.
Wait- why? Aren't we already building this? cc @imotov
| this( | ||
| NAME, | ||
| LuceneProvider.getCodec("101"), | ||
| LuceneProvider.getDefaultDelegateCodec(), |
There was a problem hiding this comment.
Not sure we want to change this, but will let Igor weigh in. We need to make sure we dont break anything.
| */ | ||
| public CuVS2510GPUSearchCodec(GPUSearchParams params) throws Exception { | ||
| this(NAME, LuceneProvider.getCodec("101"), params, FilterBitsetCacheConfig.DEFAULT); | ||
| this(NAME, LuceneProvider.getDefaultDelegateCodec(), params, FilterBitsetCacheConfig.DEFAULT); |
There was a problem hiding this comment.
Please make sure this doesn't break anythingz
Signed-off-by: Zack Meeks <zmeeks@nvidia.com>
Signed-off-by: Zack Meeks <zmeeks@nvidia.com>
3 participants
Summary
This PR adds PyLucene integration support and a pytest-owned CPU/GPU end-to-end suite for cuVS-Lucene.
The test layout follows the current cuVS/cuML
test_*.pyconvention:src/test/python/test_pylucene_end_to_end.pyowns test cases, parametrization, assertions, and reporting.src/test/python/pylucene_test_support.pycontains reusable PyLucene index/search helpers.src/test/java/com/nvidia/cuvs/lucene/PyLuceneTestSupport.javaprovides test-only codecs, execution-path diagnostics, and graph verification.python3 -m pytest -q -s src/test/python/test_pylucene_end_to_end.pyonce the documented PyLucene, jar, JVM, and native-library environment is available.The standard thin
cuvs-lucenejar remains the artifact under test; Lucene andcuvs-javaremain external classpath dependencies, and the Java diagnostics stay intarget/test-classesrather than the published jar.The PR also:
Lucene101Codecdelegate instead of dynamically selecting an older outer codecApache does not publish PyLucene 10.2.0, so the full suite uses a custom PyLucene wrapper generated against the same Lucene 10.2.0 sources as this project.
PyLucene test coverage
The suite explicitly proves these paths:
GPU-required cases assert the concrete accelerated writer, reader, and query implementations and fail on unavailable cuVS or CPU fallback. CPU cases construct and report a stock Lucene HNSW path.
Coverage includes:
searchWidthvalues 1, 16, and 32Vectors and queries are deterministic, and expected neighbors are computed by brute force. Assertions verify rank-one self matches where applicable, exact hit counts, no duplicates, inactive or filter-rejected document exclusion, and a configurable recall floor.
CAGRA configurations use
graphDegree=32andintermediateGraphDegree=64. Cases construct enough vectors to avoid cuVS graph-parameter clamping, including 24,832 vectors for the three-layer case.Validation
Validated on an NVIDIA A10G with matching cuVS Java/native 26.10 and Lucene/PyLucene 10.2 environments:
mvn -Dtest=TestBackCompat,TestAcceleratedHNSWDeletedDocuments test: 17 tests, 0 failures, 0 errorsmvn clean verify -Dtests.seed=5A17C10120260817: 303 tests, 0 failures, 0 errors, 30 skippedpython3 -m pytest -q -s src/test/python/test_pylucene_end_to_end.py: 24 passed in 36.25 secondsmvn -q spotless:check: passedThe PyLucene suite emitted no cuVS graph-clamping or CPU-fallback warnings. Its only warning was the JVM notice for the incubating vector module.
Known randomized-test failure
An unseeded full Maven run selected seed
CC0EA94328BAB3E5and exposed a pre-existing, seed-dependent failure inTestCuVSVectorsFormat.testRandomWithUpdatesAndGraph:IllegalStateException: Index not found for field:fieldatCuVS2510GPUVectorsReader.java:425.It reproduces identically both before and after the final Lucene/PyLucene compatibility changes; neither the failing reader logic nor the randomized test is changed by that patch. Reproduction command:
mvn -Dtest=TestCuVSVectorsFormat#testRandomWithUpdatesAndGraph \ -Dtests.seed=CC0EA94328BAB3E5 \ -Dtests.locale=ti-Ethi-ET \ -Dtests.timezone=Greenwich testThe fixed-seed full validation listed above passes.
Follow-up multithreaded concurrency coverage is tracked in NVIDIA/cuvs#2407.