Pin statistics across implementations in a parity test suite - #137
Merged
Conversation
Collaborator
|
did you base off of #132 [that is, merge target is nsp-multiallelic-trunk]? I think that fixed the singleton and da and fst issues, at least |
Member
Author
|
shit, I did not. not me pull in those changes to this branch |
Collaborator
|
i'd just rebase on nsp-multiallelic-trunk, that's where i'm doing all the multiallelic stuff so I don't break main in the meantime |
andrewkern
marked this pull request as draft
July 13, 2026 20:37
Add tests/test_implementation_parity.py, which asserts that the same population-genetic statistic agrees across the code paths that compute it: scalar functions, FrequencySpectrum, and the windowed scatter, fused-CUDA, and python-loop engines. Each statistic is described once in a declarative _STATS registry; the parametrized test drives every supported path across four data conditions (clean, missing/include, missing/exclude, multiallelic) and asserts it matches the scalar reference. GPU-only, skipped without CUDA. 16 statistics, ~180 cases. Divergences between paths are recorded as strict xfails in _XFAILS with a mechanism-level reason; a (stat, path) may carry several condition-scoped rules. The remaining divergences are: - fst_wc (fused, all conditions): the fused kernel was not converted to the per-allele one-vs-rest Weir-Cockerham ANOVA the scalar/python-loop path uses - theta_w, tajimas_d (fused, missing/include): the fused kernel's per-site sample-size handling under missing data still diverges from the scalar - tajimas_d, normalized_fay_wu_h, zeng_e (fs and scatter, missing/include): the Achaz neutrality-test variance uses a modal / full-sample size rather than per-site valid counts - pi, theta_w, tajimas_d, segregating_sites, fay_wu_h, normalized_fay_wu_h, zeng_e (fs, multiallelic): FrequencySpectrum builds a per-allele SFS whose segregating-class count and pi differ from the scalar per-allele counts - da (scatter, missing/exclude): the within-population pi terms use a different site set than the scalar (dxy agrees) Refs #135
andrewkern
force-pushed
the
test/implementation-parity
branch
from
July 13, 2026 20:45
39b4996 to
2fe2fca
Compare
andrewkern
marked this pull request as ready for review
July 13, 2026 21:52
Member
Author
|
okay this should be ready to go now @nspope |
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.
Summary
Addresses #135. Adds
tests/test_implementation_parity.py, a suite that asserts the same population-genetic statistic agrees across the code paths that compute it: scalar functions,FrequencySpectrum, and the windowed scatter, fused CUDA, and python-loop engines.Targets
nsp-multiallelic-trunk(this suite validates the multiallelic work), notmain.Each statistic is described once in a declarative
_STATSregistry; the parametrized test drives every supported path across four data conditions — clean, missing/include, missing/exclude, and multiallelic — and asserts it matches the scalar reference. 16 statistics, ~180 cases: 149 passed, 12 skipped, 19 xfailed. GPU-only (skipped without CUDA).This is pure test infrastructure — no library code changes. Divergences between paths are recorded as
strict=Truexfails in_XFAILSwith a mechanism-level reason; a(stat, path)may carry several condition-scoped rules. Fixing any divergence removes its rule and flips the cell to a pass.Divergences this suite currently pins
fst_wc(fused; all conditions) — the fused kernel was not converted to the per-allele one-vs-rest Weir-Cockerham ANOVA the scalar/python-loop path now uses.theta_w,tajimas_d(fused; missing/include) — the fused kernel's per-site sample-size handling under missing data still diverges from the scalar.tajimas_d,normalized_fay_wu_h,zeng_e(fs and scatter; missing/include) — the Achaz neutrality-test variance uses a modal / full-sample size rather than per-site valid counts.pi,theta_w,tajimas_d,segregating_sites,fay_wu_h,normalized_fay_wu_h,zeng_e(fs; multiallelic) —FrequencySpectrumbuilds a per-allele SFS whose segregating-class count and pi differ from the scalar's per-allele counts.da(scatter; missing/exclude) — the within-population pi terms use a different site set than the scalar (dxyagrees).Notes
missing_data='include', so it is skipped (not failed) under the exclude condition.fst_weir_cockerhamrequires for its diploid pairing.