fix(memory): resolve dim=1 float32/float64 blob ambiguity
When hrr_dim=1 the prefixed float32 blob (4+4=8 bytes) collides in size with a raw float64 blob (1×8=8 bytes), making the format discriminator in bytes_to_phases ambiguous — a legacy blob starting with HRR1 would be misread as a prefixed float32 vector. - phases_to_bytes now accepts an optional dim and falls back to writing raw float64 when the two blob sizes are equal. - bytes_to_phases prefers the legacy float64 interpretation when sizes collide and dim is provided, since phases_to_bytes never writes a prefixed float32 blob at dim=1. - Three regression tests cover dim=1 write, round-trip, and the legacy-prefix collision case. Addresses hermes-sweeper review on PR #30499.
This commit is contained in:
@@ -167,7 +167,7 @@ def encode_fact(content: str, entities: list[str], dim: int = 1024) -> "np.ndarr
|
||||
return bundle(*components)
|
||||
|
||||
|
||||
def phases_to_bytes(phases: "np.ndarray") -> bytes:
|
||||
def phases_to_bytes(phases: "np.ndarray", dim: int | None = None) -> bytes:
|
||||
"""Serialize phase vectors as float32 blobs.
|
||||
|
||||
float32 halves SQLite BLOB storage versus the legacy float64 format
|
||||
@@ -175,8 +175,20 @@ def phases_to_bytes(phases: "np.ndarray") -> bytes:
|
||||
preserving enough precision for phase-similarity retrieval.
|
||||
``bytes_to_phases`` keeps reading legacy float64 blobs for backward
|
||||
compatibility.
|
||||
|
||||
When ``dim`` is 1 the prefixed float32 blob (8 bytes) collides in size
|
||||
with a raw float64 blob (8 bytes), making the format ambiguous. In
|
||||
that case we fall back to writing raw float64 so that ``bytes_to_phases``
|
||||
can never misinterpret the blob.
|
||||
"""
|
||||
numpy = _np()
|
||||
if dim is None:
|
||||
dim = int(phases.shape[0])
|
||||
float32_blob_bytes = len(_FLOAT32_BLOB_PREFIX) + dim * numpy.dtype(numpy.float32).itemsize
|
||||
float64_bytes = dim * numpy.dtype(numpy.float64).itemsize
|
||||
if float32_blob_bytes == float64_bytes:
|
||||
# dim=1: sizes collide, write legacy float64 to stay unambiguous
|
||||
return numpy.asarray(phases, dtype=numpy.float64).tobytes()
|
||||
payload = numpy.asarray(phases, dtype=numpy.float32).tobytes()
|
||||
return _FLOAT32_BLOB_PREFIX + payload
|
||||
|
||||
@@ -189,6 +201,13 @@ def bytes_to_phases(data: bytes, dim: int | None = None) -> "np.ndarray":
|
||||
readable for backward compatibility. The returned array is copied and
|
||||
promoted to float64 so downstream HRR math keeps the existing numerical
|
||||
behavior.
|
||||
|
||||
When ``dim`` is 1 the prefixed float32 blob and the raw float64 blob are
|
||||
both 8 bytes, so size alone cannot disambiguate. ``phases_to_bytes``
|
||||
avoids writing prefixed blobs in that case; here we guard the remaining
|
||||
collision window (a legacy float64 blob that happens to start with the
|
||||
``HRR1`` prefix) by preferring the legacy interpretation when sizes
|
||||
match and the caller supplied ``dim``.
|
||||
"""
|
||||
numpy = _np()
|
||||
|
||||
@@ -197,6 +216,23 @@ def bytes_to_phases(data: bytes, dim: int | None = None) -> "np.ndarray":
|
||||
float32_blob_bytes = len(_FLOAT32_BLOB_PREFIX) + float32_payload_bytes
|
||||
float64_bytes = dim * numpy.dtype(numpy.float64).itemsize
|
||||
|
||||
# When sizes collide (dim=1), prefer legacy float64 for a blob that
|
||||
# starts with the prefix, because phases_to_bytes never writes a
|
||||
# prefixed float32 blob at dim=1 — any such blob must be legacy.
|
||||
if float32_blob_bytes == float64_bytes:
|
||||
if len(data) == float64_bytes:
|
||||
return numpy.frombuffer(data, dtype=numpy.float64).copy()
|
||||
if data.startswith(_FLOAT32_BLOB_PREFIX):
|
||||
payload_len = len(data) - len(_FLOAT32_BLOB_PREFIX)
|
||||
raise ValueError(
|
||||
f"HRR vector blob has {len(data)} bytes ({payload_len} payload bytes after "
|
||||
f"the float32 prefix); expected {float64_bytes} (legacy float64) for dim={dim}"
|
||||
)
|
||||
raise ValueError(
|
||||
f"HRR legacy vector blob has {len(data)} bytes; expected "
|
||||
f"{float64_bytes} (float64) for dim={dim}"
|
||||
)
|
||||
|
||||
if data.startswith(_FLOAT32_BLOB_PREFIX) and len(data) == float32_blob_bytes:
|
||||
payload = data[len(_FLOAT32_BLOB_PREFIX):]
|
||||
return numpy.frombuffer(payload, dtype=numpy.float32).astype(numpy.float64)
|
||||
|
||||
@@ -94,6 +94,45 @@ def test_bytes_to_phases_prefers_dim_matched_legacy_float64_on_prefix_collision(
|
||||
)
|
||||
|
||||
|
||||
def test_dim1_phases_to_bytes_writes_legacy_float64() -> None:
|
||||
"""At dim=1 the float32 prefixed blob (8 B) collides with raw float64
|
||||
(8 B), so phases_to_bytes must fall back to raw float64."""
|
||||
dim = 1
|
||||
phases = hrr.encode_atom("dim-one-ambiguity", dim=dim)
|
||||
|
||||
blob = hrr.phases_to_bytes(phases, dim=dim)
|
||||
|
||||
assert len(blob) == dim * np.dtype(np.float64).itemsize # 8 bytes, no prefix
|
||||
assert not blob.startswith(hrr._FLOAT32_BLOB_PREFIX)
|
||||
|
||||
|
||||
def test_dim1_round_trip_with_dim() -> None:
|
||||
"""Round-trip at dim=1 must work via the legacy float64 path."""
|
||||
dim = 1
|
||||
phases = hrr.encode_atom("dim-one-round-trip", dim=dim)
|
||||
|
||||
restored = hrr.bytes_to_phases(hrr.phases_to_bytes(phases, dim=dim), dim=dim)
|
||||
|
||||
assert restored.shape == (dim,)
|
||||
np.testing.assert_allclose(restored, phases, rtol=0, atol=0)
|
||||
|
||||
|
||||
def test_dim1_legacy_blob_starting_with_prefix_decodes_as_float64() -> None:
|
||||
"""A legacy float64 blob at dim=1 that happens to start with HRR1 must
|
||||
decode as float64, not be misread as a prefixed float32 blob."""
|
||||
dim = 1
|
||||
phases = hrr.encode_atom("prefix-collision-dim-one", dim=dim)
|
||||
legacy_blob = phases.astype(np.float64).tobytes()
|
||||
# Force the blob to start with HRR1 prefix bytes
|
||||
collision_blob = hrr._FLOAT32_BLOB_PREFIX + legacy_blob[len(hrr._FLOAT32_BLOB_PREFIX):]
|
||||
assert len(collision_blob) == dim * np.dtype(np.float64).itemsize
|
||||
|
||||
restored = hrr.bytes_to_phases(collision_blob, dim=dim)
|
||||
|
||||
assert restored.shape == (dim,)
|
||||
np.testing.assert_allclose(restored, np.frombuffer(collision_blob, dtype=np.float64).copy(), rtol=0, atol=0)
|
||||
|
||||
|
||||
def test_memory_store_reads_legacy_float64_vectors(tmp_path) -> None:
|
||||
dim = 64
|
||||
db_path = tmp_path / "legacy_memory_store.db"
|
||||
|
||||
Reference in New Issue
Block a user