bench: multi-seed bench_dsa_select — kill single-input pivot-luck spike (#357)#378
Open
woolcoxm wants to merge 1 commit into
Open
bench: multi-seed bench_dsa_select — kill single-input pivot-luck spike (#357)#378woolcoxm wants to merge 1 commit into
woolcoxm wants to merge 1 commit into
Conversation
…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.
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Fixes a measurement artifact in
bench_dsa_selectthat @KingIcyCreamProjects caught on the 9950X3D (see #357 thread): thenk=8192cell reported ~75×, far above the real ~13–40× algorithm. Two compounding bench bugs, both verified against the code: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 onenkrow (stable across re-runs because deterministic, not because real).brngwas never reset between cells. Each(shape, nk)cell's input depended on every prior cell'sbrand()draws — reorderingnks[]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.N_SEEDS=11independent inputs, each itself a median overN_REPEAT=2000timing 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)
Band across all cells: 6–26×, no anomalous spikes. (
plateauignores the RNG so it's unchanged, as expected.) On the 9950X3D the 75× row should now land in the same band.Scope
partial_select_descandattention_rowsare unchanged.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-
brngconfound they didn't mention.