Skip to content

Commit 44fcfdc

Browse files
committed
counting: vectorize the subsampling paths, bit-identically
`subsample="prob"`/`"dirty"` were the last two configurations still on the Python loop, on the stated grounds that they were already the fastest ones and vectorizing them "would optimize the cheap case". That was measured and it was wrong in the usual way: they were fastest per surviving pair and the *slowest per call*, which is the number a user waits on. At window=5 on 720k tokens they were the two slowest rows in the benchmark. The real obstacle was the one the equivalence gate named: the two draw streams interleave. Within a sentence `iterate_tokens` draws one random() per subsample-eligible token, and only then, knowing which survived, one randint(1, window) per survivor -- so neither stream can be drawn in a single pass over the chunk. `_subsample_draws` walks that interleaving sentence by sentence in Python and leaves only the emission to numpy, which is the right split: the draws were the cheap half all along. subsample="prob" 1.19s -> 0.39s (3.0x) subsample="dirty" 1.39s -> 0.42s (3.3x) both knobs random 1.04s -> 0.50s (2.1x) Because the streams are reproduced rather than replaced, `subsample="prob"` is now bit-identical to the frozen f68cc74 reference. The equivalence grid grows from 48 exact cells to 96 -- all four windows x all four dynamic_window modes x three subsample modes x both seeds -- and the statistical comparison for "prob" is removed: bit-identity implies equality of every moment, so keeping it would cost ten runs per cell to prove less. "dirty" keeps its statistical check (the frozen reference predates the mode) and gains an exact one against our own loop. Fixes a latent bug found by extending that fallback test to the randomized modes: a chunk above MAX_VECTORIZED_EVENTS had already drawn from the RNG before discovering it was too large, so the loop resumed from an advanced generator and a corpus counted differently depending only on how it was chunked. count_texts now snapshots and restores the RNG state around the attempt. Also drops `is_vectorizable`, which now has no false case, and corrects the benchmark's note that the "prob" rows' checksums are expected to move -- nothing vectorized the RNG in the end, so every row is bit-reproducible.
1 parent 3bc7f2c commit 44fcfdc

5 files changed

Lines changed: 403 additions & 249 deletions

File tree

CHANGELOG.md

Lines changed: 41 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -44,10 +44,47 @@
4444
holds this configuration to `assert_array_equal` across every window and
4545
seed, and the docstring records why the old claim was wrong.
4646

47-
`subsample="prob"`/`"dirty"` deliberately stay on the Python loop: they
48-
discard so many tokens that they are already the *fastest* configurations
49-
(0.12-0.13s against 0.31s for the vectorized default), so vectorizing them
50-
would optimize the cheap case and buy a second code path to keep in sync.
47+
- **`subsample="prob"`/`"dirty"` are vectorized too, and are also
48+
bit-identical.** ~3x faster (1.19s to 0.39s for clean, 1.39s to 0.42s for
49+
dirty, on 720k tokens at `window=5`); with a randomized window on top of the
50+
subsampling, 2.1x. Every configuration now takes the vectorized path.
51+
52+
This entry supersedes the paragraph above, which said these two "deliberately
53+
stay on the Python loop" because they were already the fastest configurations
54+
and vectorizing them "would optimize the cheap case". That was measured, and
55+
it was wrong in the way benchmarks usually are: they were fastest *per
56+
surviving pair* and the slowest *per call*, which is the number a user waits
57+
on. At `window=5` they were the two slowest rows in the table.
58+
59+
The real obstacle was the one the equivalence gate named: the two draw
60+
streams **interleave**. Within a sentence, `iterate_tokens` draws one
61+
`random()` per subsample-eligible token, and only then, knowing which tokens
62+
survived, one `randint(1, window)` per survivor — so neither stream can be
63+
drawn in a single pass over the chunk. `_subsample_draws` walks that
64+
interleaving sentence by sentence, in Python, and leaves only the *emission*
65+
to numpy. That is the right split: the draws were the cheap half all along.
66+
67+
Because the streams are reproduced rather than replaced, `subsample="prob"`
68+
is now held to `assert_array_equal` against the frozen f68cc74 reference —
69+
across all 4 windows, all 4 `dynamic_window` modes and both seeds, both knobs
70+
randomized at once included. The equivalence grid grew from 48 exact cells to
71+
96, and the statistical comparison for `"prob"` was **removed**: bit-identity
72+
implies equality of every moment, so keeping it would have cost 10 runs per
73+
cell to prove less. `"dirty"` keeps its statistical check (the frozen
74+
reference predates the mode and cannot produce it) and gains an exact one
75+
against this package's own loop.
76+
77+
### Fixed
78+
79+
- **An oversized chunk no longer changes randomized results.** A chunk above
80+
`MAX_VECTORIZED_EVENTS` falls back to the Python loop — but the vectorized
81+
path had already drawn from the RNG before discovering it was too large, so
82+
the loop resumed from an advanced generator. A corpus would have counted
83+
differently depending only on how it happened to be chunked, which is exactly
84+
what that memory cap must never do. `count_texts` now snapshots and restores
85+
the RNG state around the attempt. Present since `dynamic_window="prob"` was
86+
vectorized (unreleased), and found by extending the fallback test to the
87+
randomized modes.
5188

5289
- **`bench/bench_svd.py` — which SVD backend to use, measured.** The package
5390
offered three (`scipy` exact, `gensim` and `scikit` randomized) and never said

README.md

Lines changed: 27 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -185,11 +185,23 @@ tests and respect the correctness gate around the counting code.
185185

186186
## Future Work / TODO
187187

188-
- Vectorize pair counting. The counting in `hyperhyper/pair_counts.py` is still
189-
pure Python (`iterate_tokens`), and rewriting it in numpy is the obvious next
190-
optimisation. Measured on a 1.5M-token synthetic corpus (`window=2`,
191-
`dynamic_window="deter"`, 10-core M1 Pro, Python 3.12), a full
192-
corpus→count→PMI→SVD→evaluate run splits as:
188+
- ~~Vectorize pair counting.~~ **Done** — every configuration now goes through
189+
`CountPairsClosure.count_texts_vectorized`, and the Python loop
190+
(`iterate_tokens`) survives only as the streaming fallback for chunks above
191+
`MAX_VECTORIZED_EVENTS` and as the readable definition of what is being
192+
computed. Measured speedups at `window=5` on 720k tokens: 2.6x for
193+
`dynamic_window="prob"`, ~3x for `subsample="prob"`/`"dirty"`, 2.1x with both
194+
randomized at once.
195+
196+
Every one of those is **bit-identical** to the pre-vectorization output,
197+
randomized modes included: the fast path reproduces `random.Random`'s draw
198+
stream instead of replacing it with a numpy generator, so results recorded
199+
before the rewrite still reproduce exactly. `bench/bench_pair_counts.py`
200+
measures it, `tests/test_pair_counts_equivalence.py` enforces it.
201+
202+
The measurement that motivated the work, kept for scale. On a 1.5M-token
203+
synthetic corpus (`window=2`, `dynamic_window="deter"`, 10-core M1 Pro,
204+
Python 3.12), a full corpus→count→PMI→SVD→evaluate run split as:
193205

194206
| stage | time | share |
195207
|---|---:|---:|
@@ -199,10 +211,9 @@ tests and respect the correctness gate around the counting code.
199211
| PMI | 0.37s | 3.0% |
200212
| evaluation | 0.03s | 0.2% |
201213

202-
Inside counting, `cProfile` puts ~55% in `iterate_tokens` and ~32% in the
203-
dictionary accumulation around it — i.e. roughly 85% is the pure-Python part
204-
a vectorized rewrite would replace. So the ceiling here is real: several
205-
percent of end-to-end per doubling of the loop's speed.
214+
Inside counting, `cProfile` put ~55% in `iterate_tokens` and ~32% in the
215+
dictionary accumulation around it — i.e. roughly 85% was the pure-Python part
216+
the rewrite replaced. The ceiling was real, and so was the payoff.
206217

207218
Note that the pool usually does *not* run at this size — `count_pairs`
208219
estimates the serial cost and skips the pool when its ~3s spawn startup would
@@ -213,10 +224,13 @@ tests and respect the correctness gate around the counting code.
213224
with the loop "roughly a quarter" of that. The measurement above does not
214225
support it and supersedes it.)
215226

216-
A correctness gate is in place for whoever does it:
217-
`tests/test_pair_counts_equivalence.py` compares the live counter against a
218-
frozen reference (`bench/reference.py`) and requires bit-identical output on
219-
the deterministic configurations. See [CONTRIBUTING.md](./CONTRIBUTING.md).
227+
The correctness gate stays: `tests/test_pair_counts_equivalence.py` compares
228+
the live counter against a frozen reference (`bench/reference.py`) and
229+
requires bit-identical output on every configuration that reference can
230+
produce. See [CONTRIBUTING.md](./CONTRIBUTING.md).
231+
232+
- Tokenization is now the largest single stage. It is the next thing worth
233+
attacking, and unlike counting it has no correctness gate yet.
220234

221235
## `hyperhyper`?
222236

bench/bench_pair_counts.py

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -57,9 +57,12 @@
5757
cheap smoke test; the real gate is
5858
`pytest tests/test_pair_counts_equivalence.py -m slow`, which compares
5959
whole matrices against the frozen snapshot in `bench/reference.py`.
60-
2. The two `prob` rows will differ in `checksum` even when the rewrite is
61-
correct -- vectorizing the RNG changes the draw order. Only their timings
62-
are comparable.
60+
2. So must the `prob` rows'. This note used to say the opposite -- that they
61+
"will differ even when the rewrite is correct, because vectorizing the RNG
62+
changes the draw order". Nothing vectorized the RNG in the end: the
63+
randomized paths reproduce their draw streams from `random.Random` and only
64+
vectorize the emission, so every row in this table is bit-reproducible and
65+
a moved checksum is a bug on any of them.
6366
3. `best` (the fastest of `repeats` runs) and `tok/s` are the numbers to
6467
quote. `mean` is there to show how noisy the machine is; if `mean - best`
6568
is a large fraction of `best`, close things and run it again.
@@ -106,6 +109,11 @@
106109
("w10 dyn=deter", {"window": 10, "dynamic_window": "deter", "subsample": None}),
107110
("w5 dyn=prob", {"window": 5, "dynamic_window": "prob", "subsample": None}),
108111
("w5 sub=prob", {"window": 5, "dynamic_window": None, "subsample": "prob"}),
112+
("w5 sub=dirty", {"window": 5, "dynamic_window": None, "subsample": "dirty"}),
113+
# both knobs randomized: the case whose two draw streams interleave, and the
114+
# one where the vectorized counter has the least room to win, because it has
115+
# to walk that interleaving in Python
116+
("w5 dyn+sub=prob", {"window": 5, "dynamic_window": "prob", "subsample": "prob"}),
109117
]
110118

111119
# large enough that subsampling actually drops tokens instead of erasing the
@@ -261,8 +269,8 @@ def main(argv):
261269
f"best {min(e2e):.3f}s over {len(corpus.texts)} chunks"
262270
)
263271
print()
264-
print("checksum/sum/nnz must not move on the deterministic rows; the two")
265-
print("'prob' rows are expected to move once the RNG is vectorized.")
272+
print("checksum/sum/nnz must not move on ANY row: the randomized modes")
273+
print("reproduce their draw streams rather than replacing them.")
266274
print()
267275

268276

0 commit comments

Comments
 (0)