Skip to content

fix(anomaly): RSHash slot assignment depends on PYTHONHASHSEED - #449

Draft
nikolas-sapa wants to merge 1 commit into
adaptive-machine-learning:mainfrom
nikolas-sapa:fix/rshash-stable-hash
Draft

nikolas-sapa wants to merge 1 commit into
adaptive-machine-learning:mainfrom
nikolas-sapa:fix/rshash-stable-hash

Conversation

@nikolas-sapa

Copy link
Copy Markdown
Contributor

Problem

RSHash is reproducible in one process but not across processes. Two runs of the same seeded detector on the same stream produce different scores.

Cause: RSHashCountMinSketch._indices maps a discretised vector to a slot with hash((key, payload)), where payload is bytes. CPython salts hash() for bytes and tuple inputs from PYTHONHASHSEED, which is randomized per process by default.

Same code, same seed, only the env var differs (m=300, s=64, w=4, p=10000, 300 instances of TinyBlobs, score trace digested with sha256):

PYTHONHASHSEED=0      772cb45f80b84deeacaf4c914aea1d6133fbf120ff79af36a2c3a63a0d42ffcb
PYTHONHASHSEED=1      3a2997505529e323...
PYTHONHASHSEED=42     7ef1cb838ea391c5...
PYTHONHASHSEED=777    57ed2ab829aaad49...
PYTHONHASHSEED=12345  c4d2bc82e6564f43d7b21fed58b3c4a5be66a3f1e277c965fdcd8b123a7913d2

The existing test suite cannot catch this. Within one process the salt is constant, so the trace is self-consistent either way. The only observable effect is that two vectors sometimes land in the same slot under one salt and not another, which changes a sparse set of scores. The 0.78 AUC pin in tests/test_anomaly_detectors.py is unaffected.

Fix

One line, using zlib.crc32 as its initial register:

# before
return [hash((key, payload)) % self.p for key in self.hash_keys]

# after
return [
    zlib.crc32(payload, (key ^ (key >> 32)) & 0xFFFFFFFF) % self.p
    for key in self.hash_keys
]

hash() took the key as part of the tuple it hashed. crc32 takes the key as its initial register instead, so the same property holds: each of the w keys steers its own table, and the detector seed still reaches slot assignment through hash_keys. key ^ (key >> 32) folds the 63-bit key down to the 32 bits crc32 accepts.

This changes scores for existing users

Slot assignment changes for every vector, so individual scores move. On TinyBlobs (1100 instances, m=300, s=256, w=4, p=10000, seed=42), 6 of 1100 scores change.

The AUC pin does not move:

RSHash AUC, TinyBlobs, seed=42:  0.781305 before -> 0.781305 after  (delta 0.000000000)

So tests/test_anomaly_detectors.py stays at 0.78. The default-constructed detector (s=1000, window longer than the stream) also stays at 0.504595 on both sides.

Anyone relying on exact score values, or on a stored ranking, will see different numbers. Anyone relying on AUC or on reproducibility gets strictly better behaviour.

Cost

_indices dominates this detector: over 1100 instances at m=300, s=1000, w=4, p=10000 it is called 390,000 times, computing 1,560,000 individual slot hashes.

Slot assignment only, 80k single-key index computations, min of 7:

hash()        4.39 ms
zlib.crc32   10.08 ms   (2.3x)

End to end, RSHash(m=300, s=1000, w=4, p=10000) over 3000 instances:

hash()        6.536 s
zlib.crc32    7.202 s   (1.10x)

Regression test

tests/test_rshash_reproducibility.py, four tests:

  • test_rshash_is_reproducible_across_processes[1,12345] runs a seeded detector in a subprocess under an explicit PYTHONHASHSEED and compares score-trace digests across processes. Verified in both directions: fails on the current code (2 failed), passes after the fix.
  • test_table_keys_keep_the_tables_independent asserts the w keys still steer distinct slots and that distinct payloads do not collapse. This one is load bearing: a seed-blind zlib.crc32(payload, 0) passes the other three tests and only fails here.
  • test_rshash_still_consumes_its_seed asserts seed 42 and seed 1 still differ, so cross-process stability is not bought by dropping the seed.

Suite results:

tests/test_anomaly_detectors.py          9 passed, 1 failed (Autoencoder, pre-existing: torch not installed)
tests/test_rshash_reproducibility.py     4 passed
tests/test_determinism.py               20 passed
doctests, src/capymoa/anomaly/           9 passed (Autoencoder excluded, needs torch)
invoke fmt                               clean

Relation to #432

This was offered on #432 as a follow-up while working through the seeding fixes there, and #432's own open question to the maintainer is still pending. This PR does not fix, depend on, or supersede #432: it only touches _indices and is independent of the random_seed wiring. The 0.78 AUC pin is unchanged, so it does not conflict with that work.

Draft on purpose

This is a behaviour change that alters results for existing users, and it costs about 1.1x end to end. It should not land without maintainer sign-off, so it is opened as a draft. zlib.crc32 was the recommended option in the original offer; a keyed blake2b was the alternative at roughly 3x the crc32 slot cost.

Assisted-by: opencode:space-bunny-free

RSHashCountMinSketch._indices mapped a discretised vector to a slot with
hash((key, payload)). CPython salts hash() for bytes and tuple inputs from
PYTHONHASHSEED, which is randomized per process, so a seeded RSHash produced
different scores in different processes.

Replace the salted hash with zlib.crc32, seeded from the table key. The key
still steers its own table, so the detector seed still reaches slot
assignment, but the result no longer depends on the interpreter's hash salt.

Measured on TinyBlobs, RSHash AUC with seed=42 is 0.781305 before and after,
so the 0.78 pin in tests/test_anomaly_detectors.py does not move. Individual
scores do change (6 of 1100 on TinyBlobs), so this is a behaviour change for
existing users.

Add tests/test_rshash_reproducibility.py, which asserts reproducibility
across processes with different PYTHONHASHSEED values, that the sketch keys
keep the tables independent, and that the detector still consumes its seed.

Assisted-by: opencode:space-bunny-free

This branch has not been deployed

No deployments
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.

1 participant