refactor(src): dissolve basic.py into calibration + statistics#199
Draft
cailmdaley wants to merge 3 commits into
Draft
refactor(src): dissolve basic.py into calibration + statistics#199cailmdaley wants to merge 3 commits into
cailmdaley wants to merge 3 commits into
Conversation
The module named "basic" was a grab-bag: the 546-line `metacal` response class (the heart of shear calibration) plus galaxy-selection masks and a handful of cosmology-independent statistics helpers — none of which "basic" described. Its symbols now live where they belong, and basic.py is deleted. - `metacal` class + `mask_gal_size`/`mask_gal_SNR` (galaxy selection) → calibration.py, joining the m/c routines that already consumed a `gal_metacal` instance. One subsystem, one module. - `jackknif_weighted_average2`, `corr_from_cov`, `chi2_and_pte`, `cov_from_one_covariance` → new statistics.py (a clean leaf: numpy/scipy only; calibration imports the jackknife from it). - Every importer repointed (papers, scripts, the two scratch/guerrini import lines — path-only, his logic untouched); dead `from sp_validation import basic` lines removed from calibration.py and cat.py; `__all__` and the architecture docs updated. - Tests split: metacal + mask pins → test_calibration.py, jackknife pin → test_statistics.py; test_basic.py removed. All moved code is byte-identical to the original (md5-verified); value-drift pins (metacal R-matrix rtol 1e-12) and the full suite pass in-container, except the pre-existing galaxy/cs_util.size old-sandbox gap. No circular imports. Verified by an adversarial multi-agent pass (byte-identity, no-stale-refs, value-pins, no-cycles). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
scratch/guerrini/ and the namaster_utils→source / Gaussian-sims work he reserved for his next PR in the #197 review. So future workers don't touch it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
cailmdaley
force-pushed
the
refactor/src-module-layout
branch
from
902377f to
1ff913c
Compare
…ation Follow-up polish on the basic.py dissolution (#199). Tests — characterization (value-drift) coverage for the three statistics.py helpers that had none, with literals generated by running the real functions in-container and teeth on each: - corr_from_cov: unit diagonal + reconstruction from cov/outer(std,std) - chi2_and_pte: diagonal reduces to sum((d/sigma)^2) with matching scipy PTE, plus a non-diagonal case exercising the full d^T C^-1 d path - cov_from_one_covariance: gaussian(col 10) vs non-gaussian(col 9) selection and a row-major-layout check (a transpose would be caught) Calibration — strictly behavior-preserving dead-code removal: - 3 unused module imports (util, io, get_footprint — verified unreferenced) - an unused local (col_noshear) in metacal._read_data - the uncallable metacal._return method (defined without self, references self.* in its body — would NameError if ever invoked; referenced nowhere) Value pins (metacal R-matrix, m/c bias) stay green; conservatively skipped any change that would reorder float ops or restructure an estimator. Verified by an adversarial behavior-preservation review. 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 #197 (based on it so the diff is refactor-only; will retarget to
developonce #197 merges). Clean break — no compatibility shim.Why
basic.pywas a grab-bag misnamed for genericity: the 546-linemetacalresponse class (the heart of shear calibration) sat there alongside galaxy-selection masks and a few cosmology-independent statistics helpers — beside a separatecalibration.pywhose functions all consume ametacalinstance. One subsystem split across two files, under a name that told you nothing.What moved
metacalclass +mask_gal_size/mask_gal_SNR→calibration.py(joins the m/c routines that already took agal_metacal).jackknif_weighted_average2,corr_from_cov,chi2_and_pte,cov_from_one_covariance→ newstatistics.py(a clean leaf: numpy/scipy only;calibrationimports the jackknife from it).basic.pydeleted. Every importer repointed (papers, scripts,__all__, docs). Deadfrom sp_validation import basiclines removed fromcalibration.pyandcat.py(both imported it but never used it).test_calibration.py; jackknife pin →test_statistics.py;test_basic.pyremoved.Verification
Done as an adversarial multi-agent pass:
metacalR-matrixrtol 1e-12and the full suite pass in-container (the only failure is the pre-existinggalaxy/cs_util.sizeold-sandbox gap, unrelated).@martinkilbinger — this changes the public import surface (
from sp_validation.basic import …→calibration/statistics); flagging since it touches what scripts import.— Claude on behalf of Cail