Skip to content

Commit eea33cd

Browse files
committed
counting: vectorize dynamic_window="prob", bit-identically
2.6x faster (0.910s -> 0.355s on 200k tokens at window=5), and -- the part that matters more -- bit-identical to the frozen reference, so every number ever recorded with this configuration still reproduces. The equivalence gate asserted the opposite. Its docstring said bit-identity was "not achievable" for randomized configurations because "any sensible vectorization draws a whole array up front from a numpy generator". The premise was the mistake, not the conclusion drawn from it: iterate_tokens draws exactly one randint(1, window) per token in token order, and a comprehension over the flattened chunk draws the same numbers in the same order. Measured, the draws are not the expensive part anyway -- 0.08s per 200k tokens against the ~0.9s of emission they gate. So the gate now holds this configuration to assert_array_equal across all four windows and both seeds, and its docstring records why the old claim was wrong rather than quietly dropping it. subsample="prob"/"dirty" stay on the loop, and the measurement is the reason: they discard so many tokens that they are already the FASTEST configurations there are -- 0.12-0.13s against 0.31s for the vectorized deterministic default, because so few pairs survive. Vectorizing them would optimize the cheap case and buy a second implementation to keep in sync. Their draw streams also interleave in a way the prob window does not (subsampling draws precede window draws, and under the clean variant a dropped token consumes one but not the other), so it would be the delicate work as well as the pointless kind.
1 parent bfba90d commit eea33cd

3 files changed

Lines changed: 143 additions & 20 deletions

File tree

CHANGELOG.md

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,24 @@
44

55
### Added
66

7+
- **`dynamic_window="prob"` is vectorized too, and is now bit-identical.**
8+
2.6x faster (0.910s to 0.355s on 200k tokens at `window=5`) — and, unlike a
9+
numpy-RNG rewrite would have been, it reproduces the *exact* matrices this
10+
configuration produced before, so results recorded with it still hold.
11+
12+
The equivalence gate previously stated that bit-identity was "not achievable"
13+
for any randomized configuration. That assumed the vectorization would draw
14+
from a numpy generator up front; it does not have to. `iterate_tokens` draws
15+
one `randint(1, window)` per token in token order, and a comprehension over
16+
the flattened chunk draws the same numbers in the same order. The gate now
17+
holds this configuration to `assert_array_equal` across every window and
18+
seed, and the docstring records why the old claim was wrong.
19+
20+
`subsample="prob"`/`"dirty"` deliberately stay on the Python loop: they
21+
discard so many tokens that they are already the *fastest* configurations
22+
(0.12-0.13s against 0.31s for the vectorized default), so vectorizing them
23+
would optimize the cheap case and buy a second code path to keep in sync.
24+
725
- **`bench/bench_svd.py` — which SVD backend to use, measured.** The package
826
offered three (`scipy` exact, `gensim` and `scikit` randomized) and never said
927
which to pick. It reports speed *and* fidelity in the terms the package

hyperhyper/pair_counts.py

Lines changed: 50 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -345,6 +345,31 @@ def uses_rng(self):
345345
"""
346346
return self.subsampler_prob is not None or self.dynamic_window_prob
347347

348+
def is_vectorizable(self):
349+
"""
350+
Whether `count_texts_vectorized` can handle this configuration.
351+
352+
Two families qualify, and what they have in common is that the *draw
353+
stream* can be reproduced exactly -- either because there is none, or
354+
because it is one draw per token in token order, which a list
355+
comprehension over the flattened chunk gives verbatim:
356+
357+
* everything deterministic (no draws at all);
358+
* `dynamic_window="prob"` **without** subsampling: exactly one
359+
`randint(1, window)` per token, in order.
360+
361+
`subsample="prob"`/`"dirty"` stay on the loop. Not because their stream
362+
could not be reproduced -- it could, with more care, since the
363+
subsampling draws for a sentence all precede its radius draws -- but
364+
because they are already the *fastest* configurations by a wide margin:
365+
they discard so many tokens that few pairs survive, so vectorizing them
366+
would optimize the cheap case and buy a second code path to keep in
367+
sync.
368+
"""
369+
return not self.uses_rng() or (
370+
self.dynamic_window_prob and self.subsampler_prob is None
371+
)
372+
348373
def count_texts(self, texts, rng):
349374
"""
350375
The counting itself, split out from `__call__` so that a caller can time
@@ -354,8 +379,8 @@ def count_texts(self, texts, rng):
354379
bit-identical matrix several times faster; everything else keeps the
355380
Python loop. See `count_texts_vectorized`.
356381
"""
357-
if not self.uses_rng():
358-
vectorized = self.count_texts_vectorized(texts)
382+
if self.is_vectorizable():
383+
vectorized = self.count_texts_vectorized(texts, rng)
359384
if vectorized is not None:
360385
return vectorized
361386

@@ -376,19 +401,24 @@ def count_texts(self, texts, rng):
376401
counter[pair[0], pair[1]] += pair[2]
377402
return to_count_matrix(counter, self.vocab_size)
378403

379-
def count_texts_vectorized(self, texts):
404+
def count_texts_vectorized(self, texts, rng=None):
380405
"""
381406
Count a whole chunk with numpy instead of the per-token Python loop.
382407
383408
Returns the same CSR matrix as the loop, **bit for bit**, or `None` when
384409
the chunk is too large to hold its event arrays (the caller then falls
385410
back to the loop, which is slower but streams).
386411
387-
Only for configurations that draw no random numbers. The randomized
388-
modes are left on the loop deliberately: their draw order per token is a
389-
contract (see `iterate_tokens`), they emit far fewer pairs so they are
390-
already the cheap case, and duplicating that logic would be the kind of
391-
second implementation that drifts.
412+
Handles the configurations `is_vectorizable` accepts: everything
413+
deterministic, plus `dynamic_window="prob"` without subsampling.
414+
415+
The `"prob"` window keeps its exact draw stream rather than switching to
416+
a numpy generator. `iterate_tokens` draws one `randint(1, window)` per
417+
token in token order, and a comprehension over the flattened chunk draws
418+
the same numbers in the same order -- so this stays bit-identical to the
419+
loop *and* to every result recorded before it existed, which a numpy RNG
420+
could never be. The draws themselves are not the expensive part
421+
(~0.08s per 200k tokens against ~0.9s for the emission they gate).
392422
393423
How bit-identity is kept
394424
------------------------
@@ -428,8 +458,18 @@ def count_texts_vectorized(self, texts):
428458
sentence_end = sentence_start + np.repeat(lengths, lengths)
429459
centre = np.arange(len(flat), dtype=np.int64)
430460

431-
lo = np.maximum(sentence_start, centre - self.window)
432-
hi = np.minimum(sentence_end, centre + self.window + 1)
461+
if self.dynamic_window_prob:
462+
# one draw per token, in token order -- exactly what the loop does
463+
radius = np.fromiter(
464+
(rng.randint(1, self.window) for _ in range(len(flat))),
465+
dtype=np.int64,
466+
count=len(flat),
467+
)
468+
else:
469+
radius = self.window
470+
471+
lo = np.maximum(sentence_start, centre - radius)
472+
hi = np.minimum(sentence_end, centre + radius + 1)
433473
span = hi - lo # still includes the centre itself
434474
n_events = int(span.sum())
435475
if n_events > MAX_VECTORIZED_EVENTS:

tests/test_pair_counts_equivalence.py

Lines changed: 75 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -32,16 +32,31 @@
3232
If a vectorized rewrite cannot hit bit-identity here, that is a finding to
3333
discuss, not a tolerance to widen.
3434
35-
Randomized configurations -- STATISTICAL EQUIVALENCE ONLY
36-
----------------------------------------------------------
37-
dynamic_window == "prob" or subsample == "prob"
38-
39-
Bit-identity is *not achievable* here and demanding it would be a bug in the
40-
test, not in the code. The current implementation draws one number per token
41-
from `random.Random`, interleaved with the counting loop; any sensible
42-
vectorization draws a whole array up front from a `numpy` generator. Different
43-
RNG, different draw order, different numbers -- at an identical seed. The
44-
matrices will differ per seed no matter how correct the rewrite is.
35+
`dynamic_window="prob"` without subsampling -- ALSO BIT-IDENTICAL
36+
------------------------------------------------------------------
37+
This section used to say bit-identity was "not achievable" for every randomized
38+
configuration. That was wrong, and the reasoning behind it is worth keeping
39+
because it is a plausible trap: it assumed a vectorization would draw its
40+
numbers from a *numpy* generator up front, which would indeed give different
41+
numbers at the same seed no matter how correct the rewrite was.
42+
43+
It does not have to. `iterate_tokens` draws exactly one `randint(1, window)` per
44+
token in token order, and a comprehension over the flattened chunk draws the
45+
same numbers in the same order -- so the vectorized counter reproduces the
46+
stream verbatim and stays bit-identical to the frozen reference, and therefore
47+
to every number recorded before it existed. Held to `assert_array_equal` below,
48+
across all four windows and both seeds.
49+
50+
Randomized subsampling -- STATISTICAL EQUIVALENCE ONLY
51+
-------------------------------------------------------
52+
subsample in ("prob", "dirty")
53+
54+
These stay on the Python loop, so nothing about their output has changed. The
55+
statistical machinery below remains the standard they are held to, because a
56+
future vectorization of *them* would face the interleaving the paragraph above
57+
sidesteps: the subsampling draws for a sentence precede its window draws, and
58+
under the clean variant a dropped token consumes a subsample draw but no window
59+
draw.
4560
4661
What must survive is the *distribution*. So these are checked by:
4762
@@ -892,3 +907,53 @@ def test_vectorized_counter_handles_a_single_token_sentence(grid_corpus):
892907
counter[pair[0], pair[1]] = counter.get((pair[0], pair[1]), 0) + pair[2]
893908
expected = pair_counts.to_count_matrix(counter, grid_corpus.vocab.size)
894909
np.testing.assert_array_equal(fast.toarray(), expected.toarray())
910+
911+
912+
@pytest.mark.parametrize("seed", SEEDS)
913+
@pytest.mark.parametrize("window", WINDOWS)
914+
def test_prob_window_without_subsampling_is_bit_identical(grid_corpus, window, seed):
915+
"""
916+
`dynamic_window="prob"` is randomized and *still* bit-identical.
917+
918+
The vectorized counter draws its radii from the same `random.Random`, one
919+
per token in token order, rather than from a numpy generator -- so the
920+
stream is the one the Python loop produced, and every number recorded before
921+
the rewrite still reproduces. See the module docstring for why this was
922+
previously believed impossible.
923+
"""
924+
live, ref = _both(
925+
grid_corpus,
926+
window=window,
927+
dynamic_window="prob",
928+
subsample=None,
929+
subsample_factor=SUBSAMPLE_FACTOR,
930+
seed=seed,
931+
)
932+
np.testing.assert_array_equal(
933+
live,
934+
ref,
935+
err_msg=(
936+
f"live count_pairs diverged from the frozen reference for "
937+
f"window={window} dynamic_window='prob' seed={seed}. The radius "
938+
f"draws must come from random.Random in token order, so this is "
939+
f"reproducible despite being randomized."
940+
),
941+
)
942+
assert ref.sum() > 0
943+
944+
945+
def test_prob_window_is_vectorized_but_subsampling_is_not(grid_corpus):
946+
keep = pair_counts.subsample_keep_probabilities(grid_corpus.counts, 1.0)
947+
assert _closure(grid_corpus, dynamic_window="prob").is_vectorizable() is True
948+
# ... and adding subsampling takes it back off the fast path, because the
949+
# two draw streams interleave
950+
assert (
951+
_closure(
952+
grid_corpus, dynamic_window="prob", subsample="prob", subsampler_prob=keep
953+
).is_vectorizable()
954+
is False
955+
)
956+
assert (
957+
_closure(grid_corpus, subsample="dirty", subsampler_prob=keep).is_vectorizable()
958+
is False
959+
)

0 commit comments

Comments
 (0)