SAGA v2 follow-up: integer guard, repo hygiene, Falcon submodule - #4
Merged
Conversation
UnivariateSamples histograms by integer key, so continuous input turned every sample into an outlier -> n_effective=0 -> chi-square 0/0 = nan (a RuntimeWarning), silently. Because every multivariate test vector was continuous, the per-coordinate Gaussianity channel (fisher_bh) was never actually exercised, and mv_bad_variance_one_coord was "detected" only via that nan degeneracy. - UnivariateSamples: reject non-integer input and all-outlier data with a clear ValueError. - MultivariateSamples: detect integer-ness; run the per-coordinate discrete channel only for integer data (nb_gaussian_coord=None otherwise), instead of feeding it nan p-values. - Test vectors: add an integer discrete-Gaussian good vector (lights up the channel dim/dim) and make the wrong-variance vector integer so it is genuinely caught. Sampling the discrete Gaussian directly, not rounding a continuous draw (rounding inflates variance by 1/12 -> squared-norm false alarm). - Battery verdict: fold in diagcov and BH-localization so a single flawed coordinate (diluted in the global Fisher p) and a correlated pair (which diagcov catches) are not silently passed. - Minor: use math.pi instead of a hardcoded 3.14159265. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Remove sampler_rep.py: orphaned since the rejection-channel test was descoped (its import and test_rejind were already gone). - Remove extensive_calibration.json: 4700 lines of committed calibration *output*, read by no code; regenerate with run_extensive_calibration.py (now gitignored). - Move the four standalone scripts (report_card, run_baseline, run_extensive_calibration, visualize_mockups) under code/scripts/ so the code/ root shows only the core suite; fix their sys.path accordingly. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The vendored code/falcon/ had diverged into a SAGA-specific fork: re-packaged to `from falcon.X` imports with an added __init__.py, plus a local newsampler.py wired into ffsampling and reject=False / T_fft hooks. Meanwhile upstream restructured (samplerz.py, rng.py). Replace it with a git submodule pinned to the reference implementation. - code/falcon is now a submodule of github.com/tprest/falcon.py. - test_sig rewritten against the reference API: Falcon(n).keygen(), then __sample_preimage__ to read the raw (s0, s1) Gaussian vector directly. This replaces the old reject=False path (the norm rejection sits ~6.7 sigma out, mass ~1e-11, statistically indistinguishable). The T_fft perturbation demo is dropped -- it is covered by the synthetic mv_bad_fft_zeroed vector. - The import is lazy (inside test_sig), so the core suite has no dependency on falcon/ or its extra deps (pycryptodome, beartype), exposed as the optional [falcon] extra. Fetch with: git submodule update --init. Verified: test_sig(n=64) on real Falcon signatures gives dim=128, sigma=157.5, DH p=0.89, 128/128 Gaussian coordinates -- exercising the integer per-coordinate channel on real data. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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.
Follow-up to #3 (SAGA v2), from a fresh re-review of the merged suite. Three self-contained commits.
1. Enforce discrete Gaussians; fix a silent
nanUnivariateSampleshistograms by integer key, so continuous input turned every sample into an outlier →n_effective = 0→ chi-square0/0 = nan, silently (aRuntimeWarningin the suite). Consequences, found by running the merged suite:rng.normal), so the per-coordinate Gaussianity channel (fisher_bh) was never actually exercised — a regression there would pass CI.mv_bad_variance_one_coordwas "detected" only via thatnandegeneracy (spuriouscoord_fail_rate = 1.0).Fixes: reject non-integer / all-outlier input with a clear error;
MultivariateSamplesruns the discrete per-coordinate channel only for integer data (nb_gaussian_coord=Noneotherwise); add an integer discrete-Gaussian good vector (samples the discrete Gaussian directly — rounding a continuous draw inflates variance by 1/12 and false-alarms the norm test) and make the wrong-variance vector integer; folddiagcov+ BH-localization into the battery verdict so a single flawed coordinate or a correlated pair isn't silently passed;math.pifor a hardcoded constant.2. Repo hygiene
Remove
sampler_rep.py(orphaned dead code) andextensive_calibration.json(4700 lines of committed output); move the four standalone scripts undercode/scripts/socode/shows only the core suite.3. Reference Falcon as a submodule
The vendored
code/falcon/had diverged into a SAGA-specific fork (re-packaged imports, added__init__.py+newsampler.py,reject=False/T_ffthooks) while upstream restructured. Replace it with a git submodule pinned to tprest/falcon.py, and rewritetest_sigagainst the reference API (Falcon(n).keygen(),__sample_preimage__for the raw(s0,s1)vector). The import is lazy, so the core suite has no dependency onfalcon/or its extra deps (pycryptodome,beartype→ optional[falcon]extra).Verification
RuntimeWarning(checked under-W error::RuntimeWarning).test_sig(n=64)on real Falcon signatures: dim=128, σ=157.5, DH p=0.89, 128/128 Gaussian coordinates — exercises the integer per-coordinate channel on real data.Note for reviewers/CI:
test_signeedsgit submodule update --init; the pytest suite does not (it never runstest_sig), so CI stays green without fetching the submodule.🤖 Generated with Claude Code