Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 8 additions & 9 deletions .claude/skills/benchmark-seismic/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -157,9 +157,7 @@ OMP_NUM_THREADS=1 python demos/seismic_mmap.py \
--reuse-index --keep
```

Needs `numpy>=2.1` installed *over* the nsparse package: `pyproject.toml` pins `numpy<2.0`, which on
Python 3.13+ resolves to a numpy older than the interpreter and silently corrupts arrays (`a - b`
overwrites `a`) while every check still passes. Pin Python 3.12 or force the newer numpy.
Install the bindings with `pip install build/nsparse/python`; any numpy 1.26+ or 2.x works.

### Peak build memory — `disk_build_mem_bench`

Expand All @@ -182,12 +180,13 @@ recall = np.mean([len(set(map(int, results[i])) & set(map(int, correct[i])))

## Gotchas that have invalidated real runs

- **The serialized `.dat` header carries no format version** — `write_header` writes only fourcc and
dimension. A new binary reading an old file throws `std::length_error` (loud), but an **old binary
reading a new file silently loads a garbage index** with the correct `num_vectors` and wrong
results. Any change to on-disk layout (e.g. the alignment padding in `nsparse/io/align.h`)
invalidates every cached `.dat`. Regenerate them whenever the tree changes, and distrust a
suspiciously large speedup — one apparent 100× win was a binary misparsing a stale file.
- **A cached `.dat` is only as safe as its format version.** The header carries a per-index-type
version (`kFormatVersion`, checked by `read_index`), so a file from a layout this binary does not
know fails loudly — but only if the layout change also bumped the version. A layout change shipped
without a bump (e.g. to the alignment padding in `nsparse/io/align.h`) still **silently loads a
garbage index** with the correct `num_vectors` and wrong results. Regenerate cached `.dat` files
whenever the on-disk layout changes, and distrust a suspiciously large speedup — one apparent 100×
win was a binary misparsing a stale file.
- **mmap requires the unquantized write path.** `seismic_sq` is not mmap-able, so enabling
`kUseMmap` means writing plain `seismic` and losing 8-bit quantization — the index file roughly
doubles. Do not attribute a latency change to residency when quantization moved with it.
Expand Down
24 changes: 14 additions & 10 deletions .github/workflows/CI.yml
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,10 @@ jobs:
needs: check-files
if: needs.check-files.outputs.RUN_BUILD_AND_TEST == 'true'
runs-on: ubuntu-latest
strategy:
fail-fast: false
matrix:
python-version: ['3.12', '3.13']
steps:
- name: Checkout
uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4
Expand All @@ -173,18 +177,14 @@ jobs:
sudo apt-get update
sudo apt-get install -y swig

# Pinned to 3.12 because nsparse/python/pyproject.toml requires numpy<2.0,
# and numpy 1.26 is the last release supporting that constraint. On 3.13+
# the pin resolves to a numpy older than the interpreter, which corrupts
# arrays at runtime rather than failing to install.
- name: Set up Python
id: python
uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5.6.0
with:
python-version: '3.12'
python-version: ${{ matrix.python-version }}

- name: Install build dependencies
run: python -m pip install --upgrade pip "numpy<2.0" packaging setuptools wheel pytest
run: python -m pip install --upgrade pip -r nsparse/python/requirements.txt packaging setuptools wheel pytest

- name: Configure
run: |
Expand All @@ -197,13 +197,10 @@ jobs:
- name: Build
run: cmake --build build -j$(nproc)

# --no-deps keeps the numpy installed above. Letting pip re-resolve here can
# swap numpy after the extension was compiled against its headers, and a
# mismatched pair corrupts search results instead of erroring.
- name: Install the built package

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

FYI, with --no-deps everywhere, numpy>=1.26 is never actually resolved by pip in CI.

run: |
cd build/nsparse/python
python -m pip install --no-deps .
python -m pip install .

- name: Import check
run: python -c "import nsparse; print(nsparse.index_factory)"
Expand All @@ -215,6 +212,13 @@ jobs:
- name: Run Python tests
run: python -m pytest python_tests -v -rX

# The floor pyproject.toml declares, exactly. 3.13 has no numpy 1.x wheel.
- name: Run Python tests on the numpy floor
if: matrix.python-version == '3.12'
run: |
python -m pip install "numpy==1.26.0"
python -m pytest python_tests -v -rX

Build-nsparse-MacOS:
name: Build and Test nsparse on MacOS
needs: check-files
Expand Down
7 changes: 5 additions & 2 deletions DEVELOPER_GUIDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -186,13 +186,16 @@ cover the internals SWIG deliberately hides (`MmapCursor`, borrowing
They need the bindings built and installed:

```bash
pip install -r nsparse/python/requirements.txt pytest # numpy 2.x headers for the build
cmake -S . -B build -DNSPARSE_ENABLE_PYTHON=ON
cmake --build build -j
pip install "numpy<2.0" pytest
pip install --no-deps build/nsparse/python
pip install build/nsparse/python
pytest python_tests -v
```

The bindings are compiled against the numpy present at configure time, which
must be 2.x; a module built that way runs on numpy 1.26+ and 2.x.

One file per index type, named after the use case being exercised
(`test_happy_case`, `test_with_id_map`, `test_exact_match`, ...). Accuracy is
checked against an independent numpy brute-force oracle in
Expand Down
5 changes: 3 additions & 2 deletions benchmarks/DISK_SEISMIC_BENCH.md
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,8 @@ larger on disk but only its summaries stay resident) to expose the disk benefit.

- **`base_small` is a smoke test, not a benchmark** — it fits in cache, so the
memory-bound behavior that is the whole point disappears. Only report `base_full`.
- Regenerate the `.dat` files whenever the source tree changes; the serialized format
carries no version and an old binary silently misreads a new file.
- Regenerate the `.dat` files whenever the on-disk layout changes. The header carries a
per-type format version, so a mismatch is caught only if the change bumped
`kFormatVersion`; one that did not is misread silently.
- Query threads are pinned to 1 (`OMP_NUM_THREADS=1`) — per-query cost is the metric
and the workload is bandwidth-bound.
7 changes: 0 additions & 7 deletions demos/seismic_mmap.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,6 @@
Usage:
python demos/seismic_mmap.py <data.csr> <queries.csr> [options]

Needs a numpy that supports the running interpreter. Installing the nsparse
package pulls numpy<2.0 (see pyproject.toml), and on Python 3.13+ that resolves
to a numpy released before the interpreter existed, which silently corrupts live
arrays: `a - b` overwrites `a`, so scores turn to zeros midway through a run
while every check still passes. Install numpy>=2.1 over it (pip complains about
the pin; the complaint is the bug, not the fix).

Exits non-zero if any check fails.
"""

Expand Down
8 changes: 8 additions & 0 deletions nsparse/python/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,14 @@ find_package(Python REQUIRED
COMPONENTS Interpreter Development.Module NumPy
)

# A module built against numpy 2.x headers loads on numpy 1.x and 2.x; one built
# against 1.x cannot import under 2.x, which pyproject.toml permits.
if(Python_NumPy_VERSION VERSION_LESS 2.0)
message(FATAL_ERROR "numpy>=2.0 headers are required to build the bindings; "
"found ${Python_NumPy_VERSION} for ${Python_EXECUTABLE} "
"(pip install -r nsparse/python/requirements.txt).")
endif()

find_package(SWIG REQUIRED COMPONENTS python)
include(${SWIG_USE_FILE})

Expand Down
27 changes: 12 additions & 15 deletions nsparse/python/loader.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,27 +24,24 @@ def supported_instruction_sets():
"""

def is_sve_supported():
if platform.machine() != "aarch64":
# AT_HWCAP (16) from the kernel; HWCAP_SVE is bit 22 on aarch64.
if platform.machine() != "aarch64" or platform.system() != "Linux":
return False
if platform.system() != "Linux":
return False
import numpy
import ctypes

if Version(numpy.__version__) >= Version("2.0"):
return False
try:
import numpy.distutils.cpuinfo

return (
"sve" in numpy.distutils.cpuinfo.cpu.info[0].get("Features", "").split()
)
except ImportError:
return bool(__import__("ctypes").CDLL(None).getauxval(16) & (1 << 22))
getauxval = ctypes.CDLL(None).getauxval
getauxval.restype = ctypes.c_ulong
getauxval.argtypes = [ctypes.c_ulong]
return bool(getauxval(16) & (1 << 22))

import numpy

if Version(numpy.__version__) >= Version("1.19"):
from numpy._core._multiarray_umath import __cpu_features__
try:
# numpy 2.x, and the shim numpy 1.26.1+ ships
from numpy._core._multiarray_umath import __cpu_features__
except ImportError: # numpy 1.x up to 1.26.0
from numpy.core._multiarray_umath import __cpu_features__

supported = {k for k, v in __cpu_features__.items() if v}
if is_sve_supported():
Expand Down
4 changes: 2 additions & 2 deletions nsparse/python/pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ build-backend = "setuptools.build_meta"
name = "nsparse"
version = "0.1.0"
description = "Python bindings for NSPARSE sparse vector search library"
requires-python = ">=3.8"
dependencies = ["numpy>=1.20.0,<2.0", "packaging"]
requires-python = ">=3.9"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

why? not in the description's change list. And CI only runs 3.12/3.13, so the declared floor is untested.

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.

No numpy >= 1.26 has a release for Python 3.8, so increase the floor version to 3.9.

Do we need CI test for 3.9, since we it was untested for 3.8 before?

dependencies = ["numpy>=1.26", "packaging"]
license = "MIT"

[tool.setuptools]
Expand Down
1 change: 1 addition & 0 deletions nsparse/python/requirements.txt
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
# Build-time headers only; the runtime bound is in pyproject.toml.
numpy>=2.0
3 changes: 3 additions & 0 deletions nsparse/python/swignsparse.swig
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@

%{
#define SWIG_FILE_WITH_INIT
// Pin the C-API floor (the 1.25/1.26 level) rather than inherit whatever the
// numpy 2.x providing the headers defaults to. The package floor is pyproject's.
#define NPY_TARGET_VERSION NPY_1_25_API_VERSION
#define NPY_NO_DEPRECATED_API NPY_1_7_API_VERSION
#include <new>
#include <stdexcept>
Expand Down
17 changes: 4 additions & 13 deletions python_tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,8 @@
"""Fixtures for the black-box index tests.

Everything here goes through the installed extension module and the SWIG
surface only. Two harness guards run before any test, because both failure
modes are silent: a shadowed import gives an incomplete module, and a
mismatched numpy corrupts arrays rather than erroring.
surface only. One harness guard runs before any test, because its failure mode
is silent: a shadowed import gives an incomplete module rather than an error.
"""

import os
Expand Down Expand Up @@ -39,7 +38,7 @@


def pytest_configure(config):
"""Fail loudly on the two silent harness failure modes."""
"""Fail loudly on the silent harness failure mode."""
# Importing from the repo root resolves the source tree `nsparse/` as a
# namespace package (__file__ is None) when the extension is not installed.
# The result is a module without index_factory, which reads as a test bug.
Expand All @@ -48,17 +47,9 @@ def pytest_configure(config):
):
raise pytest.UsageError(
f"nsparse resolved to an incomplete module ({nsparse.__file__!r}). "
"Install the built extension: pip install --no-deps "
"Install the built extension: pip install "
"build/nsparse/python"
)
# nsparse/python/pyproject.toml pins numpy<2.0. A newer numpy than the one
# the extension was compiled against corrupts arrays at runtime instead of
# failing to import, which would look like a search regression.
if int(np.__version__.split(".")[0]) >= 2:
raise pytest.UsageError(
f"numpy {np.__version__} is incompatible with these bindings "
"(pyproject pins numpy<2.0); results would be silently corrupt."
)


@pytest.fixture(scope="session")
Expand Down
Loading