Phase 0: fix 9 bugs, add test vector infrastructure - #1
Open
jameshoweee wants to merge 1 commit into
Open
Conversation
Bug fixes: - 0.1: chisquare() rounding correction applied to wrong variable; now corrects the exp list actually passed to scipy and accounts for outliers in n_effective (crashed on scipy >= 1.7) - 0.2: sampler.c used PDT table with RCDT comparison logic, giving P(0) ~ 0.64 instead of 0.36; replaced with actual RCDT values. Also fixed doubled loop increment (i=i+1 inside for) - 0.3: doornik_hansen() had a raise before rank-deficient recovery path, making recovery dead code - 0.4: removed sampler_rep.py import and test_rejind() (descoped per unified plan; code preserved in pqcrypto2020-as-published tag) - 0.5: test_basesampler() now accepts optional data argument, fixing arity mismatch with csv_testing() - 0.6: mardia() p_B now two-sided (kurtosis can deviate either way) - 0.7: BerExp scaling factor corrected from sigma/sigma0 to sigmin/sigma (matches HPRR20 algorithm and Falcon reference) - 0.8: sigmin lowered from 1.3 to 1.2778 to accept spec-legal values (Falcon spec sigma_min = 1.277833697 for n=512) Test infrastructure: - generate_test_vectors.py: 83 seeded test vectors across 3 tiers (good/bad/mediocre) covering 10 flaw types - run_baseline.py: scores current SAGA against vectors Baseline results after fixes: - Errors: 52/78 -> 0/78 - Bad detection: 10/42 (23.8%) -> 30/42 (71.4%) - Good false alarms: 0/6 (0%) - Remaining blind spots: Markov correlation (needs Ljung-Box, Phase 4) and tail truncation (needs AD/tail exceedance, Phase 3)
Owner
Author
|
consolidating into a single PR |
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
1 participant
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.
Fixes a bunch of bugs I found while going through the code:
sampler.py: sigmin was 1.3 instead of 1.2778 (NIST spec value), BerExp scaling used wrong ratiosampler.c: RCDT table values were wrong (PDT instead of RCDT), loop had doubled increment (i=i+1inside a for loop)saga.py: chisquare rounding bug (n_effective vs len, missing diff correction), doornik_hansen rank-deficient path had shape mismatch, mardia p_B was one-sided instead of two-sided, removed deadsampler_repimport and unusedtest_rejindAlso adds
generate_test_vectors.py(86 seeded vectors across 10+ flaw types) andrun_baseline.pyfor scoring — needed these for regression testing the subsequent changes.