Skip to content

bench: multi-seed bench_dsa_select — kill single-input pivot-luck spike (#357)#378

Open
woolcoxm wants to merge 1 commit into
JustVugg:devfrom
woolcoxm:fix/bench-dsa-select-seeds
Open

bench: multi-seed bench_dsa_select — kill single-input pivot-luck spike (#357)#378
woolcoxm wants to merge 1 commit into
JustVugg:devfrom
woolcoxm:fix/bench-dsa-select-seeds

Conversation

@woolcoxm

Copy link
Copy Markdown
Contributor

What

Fixes a measurement artifact in bench_dsa_select that @KingIcyCreamProjects caught on the 9950X3D (see #357 thread): the nk=8192 cell reported ~75×, far above the real ~13–40× algorithm. Two compounding bench bugs, both verified against the code:

  1. One frozen input per cell. partial_select_desc's median-of-three pivot is fully deterministic (no RNG), and the bench froze the input then ran 2000 reps on that identical array — so all reps hit the exact same pivot sequence. A single lucky input spiked one nk row (stable across re-runs because deterministic, not because real).
  2. brng was never reset between cells. Each (shape, nk) cell's input depended on every prior cell's brand() draws — reordering nks[] or adding a shape silently shifted all later inputs.

Fix

  • brng_seed() reseeds per (shape, nk, seed) → every cell reproducible and independent of cell ordering.
  • Report the median of N_SEEDS=11 independent inputs, each itself a median over N_REPEAT=2000 timing reps. A lucky pivot now moves one of 11 samples, not the reported number.

Measured (this fix, Intel Core Ultra 9 185H, median of 11 seeds)

shape nk old new speedup
realistic 8192 631,600 57,600 10.97×
uniform 8192 833,700 88,800 9.39×
plateau 8192 161,100 10,000 16.11×

Band across all cells: 6–26×, no anomalous spikes. (plateau ignores the RNG so it's unchanged, as expected.) On the 9950X3D the 75× row should now land in the same band.

Scope

  • Bench fidelity fix only. No engine code touched. partial_select_desc and attention_rows are unchanged.
  • Correctness unaffected — test_dsa_select (129 cases) still passes.

Credit

The artifact and its diagnosis are @KingIcyCreamProjects's — caught in the #357 thread, with the correct root-cause read (deterministic pivot + single frozen input). This PR implements their suggested fix (average seeds per cell) and additionally fixes the never-reset-brng confound they didn't mention.

…ke (JustVugg#357)

@KingIcyCreamProjects caught a real artifact on the 9950X3D (JustVugg#357 thread): the
nk=8192 cell reported ~75x, far above the real ~13-40x algorithm. Root cause was
two compounding bench bugs, both verified against the code:

  1. ONE frozen input per cell. partial_select_desc's median-of-three pivot is
     fully deterministic (no RNG), and bench_dsa_select froze the input then ran
     2000 reps on that identical array -- so all 2000 reps hit the exact same
     pivot sequence. A single lucky input spiked one nk row (stable across re-runs
     because it's deterministic, not because it's real).
  2. brng was a static global, initialized once and NEVER reset between cells.
     So each (shape,nk) cell's input depended on every prior cell's brand() draws
     -- reordering nks[] or adding a shape silently shifted all later inputs.

Fix:
  - brng_seed() reseeds per (shape, nk, seed) so every cell is reproducible and
    independent of cell ordering.
  - Report the MEDIAN of N_SEEDS=11 independent inputs, each itself a median over
    N_REPEAT=2000 timing reps. A lucky pivot now moves one of 11 samples, not the
    reported number.

Measured on Intel Core Ultra 9 185H (this fix, median of 11 seeds):
  realistic@8192  10.97x   (was 13.57x single-seed on this box)
  uniform@8192     9.39x   (was 7.48x)
  plateau@8192    16.11x   (plateau ignores the RNG, unchanged as expected)
  band: 6-26x across all cells, no anomalous spikes.

Correctness is unaffected (test_dsa_select, 129 cases, unchanged). This is a bench
fidelity fix only -- no engine code touched.
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