Skip to content

DiskSeismic: add python tests & de-dup serialization - #33

Merged
chishui merged 1 commit into
opensearch-project:mainfrom
zirui-song-18:disk-seismic-dedup-pytest
Aug 27, 2026
Merged

chishui merged 1 commit into
opensearch-project:mainfrom
zirui-song-18:disk-seismic-dedup-pytest

Conversation

@zirui-song-18

Copy link
Copy Markdown
Collaborator

Description

Addresses the two follow-ups from #32:

  1. De-duplicate the serialization. DiskSeismicIndex wrote the per-cluster doc-id membership twice — in the posting-list section and as per-block doc ids in the inline forward index — yet the posting-list copy is never read on the mmap search path (doc ids come from the forward index; only the cluster summaries + counts are used). write_index now serializes the posting lists summaries-only, emitting docs_/offsets_ as count-0 arrays. Since the align helpers round-trip empty arrays byte-for-byte, the reader is unchanged and plain SeismicIndex output stays byte-identical.
  2. Python integration tests. Adds python_tests/test_disk_seismic_index.py (mmap-only contract, top-k′ block budget monotone/saturating, bit-exact fresh-build vs mmap-reload, id-map, filtering, empty/unbuilt, padding). No binding changes needed — disk_seismic is already reachable through SWIG.

Issues Resolved

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: Zirui Song <zrsong@amazon.com>
@chishui

chishui commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator

does the index size results in previous PR change much?

@zirui-song-18

zirui-song-18 commented Aug 27, 2026 •

Copy link
Copy Markdown
Collaborator Author

does the index size results in previous PR change much?

Thanks for your question. Following is a benchmark:

  • lambda 4000: -0.3854% compared with 439b992
  • lambda 6000: -0.4052% compared with 439b992

@chishui
chishui merged commit 99bc33f into opensearch-project:main Aug 27, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants