refactor(cosmo_val): thin the orchestrator — extract survey-stat + pseudo-Cℓ primitives, pin the estimator#209
Merged
cailmdaley merged 5 commits intoJun 21, 2026
Conversation
…n the mixin Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…_cl.py, thin the mixin Move the stateless pseudo-Cl computation out of PseudoClMixin into named free functions in a new top-level sp_validation/pseudo_cl.py (mirrors b_modes.py / rho_tau.py): make_namaster_bin, get_n_gal_map (weights kwarg, subsumes the glass_mock unweighted twin), apply_random_rotation (rng injectable; default preserves the no-arg np.random.seed() noise-debiasing behavior), and the map-/catalog-based estimators get_pseudo_cls_map / get_pseudo_cls_catalog. pseudo_cl_geometry centralizes the (lmin, lmax, b_lmax) triple. The mixin methods are now thin wrappers (load via self -> call primitive), public names/signatures unchanged so call sites do not move. glass_mock's get_n_gal_map delegates to the primitive via a lazy import, preserving its CAMB-only import-time independence from the harmonic stack. Pins (test_pseudo_cl.py) green; value-drift gate (test_cosmo_val + test_b_modes) 18 passed / 1 skipped; glass_mock tests green; actual cross-module import verified in the container. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…opies Repoint the GLASS-mock harmonic stats at the new src/sp_validation/pseudo_cl.py primitives so the bandpower binning can no longer drift from the estimator the rest of the package uses. - powspace_bins: deleted. Its square-root bandpower spacing is exactly pseudo_cl.make_namaster_bin(..., "powspace", power=0.5) on the pseudo_cl_geometry(nside) range. Replaced by a thin _mock_powspace_bin wrapper that returns the same (bin, ell_eff, lmax, b_lmax) 4-tuple the mock helpers consume, now sourced from the shared primitive. Verified bitwise-identical binning (ell_eff and per-bin ell membership) across nside/lmin/n_bins configs. - get_n_gal_map: already delegates to the weighted primitive with weights=None (unweighted counts); left as the count-default twin used by the map estimator. - compute_two_point_cl / compute_two_point_cl_map: kept their own NaMaster field construction (documented) — the mock field is UNWEIGHTED and these return the COUPLED pseudo-Cl alongside the decoupled one with the ell axis prepended, which the primitives' (ell_eff, cl_all, wsp) contract does not expose. They now take their binning and galaxy-count map from the shared primitives. Value-drift gate green (18 passed, 1 skipped); full non-slow suite 136 passed, 1 skipped, 1 xpassed, only the two known-environmental failures (test_bmodes_workflow_dry_runs, test_configured_paths_exist_on_candide). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
apply_random_rotation called np.random.seed() with no argument, re-seeding from OS entropy on every call, so the pseudo-Cl noise-debiasing realizations were non-reproducible run-to-run. Drop the bare seed; the primitive now uses its rng argument (default_rng() when None), and the debiasing + covariance sampling paths thread a generator seeded from a new CosmologyValidation cell_seed (default 8192). Same numerical scheme, now reproducible. The test_pseudo_cl reproducibility test replaces the old non-determinism guard.
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
Thins the
cosmo_valorchestrator by moving stateless computation out of the mixin methods into shared primitive modules — the architecture this package is built around (cosmo_val/= the stateful orchestrator class; top-level modules = stateless primitives that analysis scripts also import directly). Two extractions:survey.py.n_eff_density,area_from_coords,ellipticity_dispersion,additive_bias,effective_survey_stats— pulled out ofcatalog_characterization; the mixin methods are now thin wrappers. (scratch/guerrinihand-reimplements several of these — they can now import them.)pseudo_cl.py.pseudo_cl_geometry,make_namaster_bin,get_n_gal_map,apply_random_rotation,get_pseudo_cls_map,get_pseudo_cls_catalog— the NaMaster estimator, now a reusable primitive module. The mixin orchestrates over it.glass_mockunified. It carried drifted copies of the estimator;powspace_binsandget_n_gal_mapnow route through the shared primitives.compute_two_point_cl[_map]are deliberately kept (they use unweighted fields and return the coupled spectrum, a genuine divergence the primitive's contract doesn't cover) — documented inline.New test coverage, and what it caught
The pseudo-Cℓ estimator — a Paper II deliverable — had zero numeric pins. This PR adds
test_pseudo_cl.py(golden values for all three binning schemes, the occupancy map, and the map/catalog EE·EB·BB spectra). Pinning it surfaced — and this PR fixes — a real reproducibility bug:apply_random_rotationcallednp.random.seed()with no argument, re-seeding from OS entropy on every call, so the noise realizations differed run-to-run. The fix drops the bare seed; the primitive uses itsrngargument and the debiasing + covariance-sampling paths thread a generator seeded from a newCosmologyValidation.cell_seed(default 8192). Same numerical scheme, now reproducible. (This is the one deliberate behavior change in the PR; everything else is structure-only.)NmtFieldCatalog), so those pins sit atrtol=1e-6; the binning, occupancy map, and map-path estimator are bitwise-identical (rtol=1e-9).Verification
ruff check src/sp_validationclean; all four modules import (pseudo_cl,glass_mock,survey,cosmo_val). Full non-slow suite: 136 passed, the new pins all pass, value-drift (test_cosmo_val,test_b_modes) green. One non-blocking note:n_eff_densityadds asum_w2 > 0guard the inline code lacked — identical for any real catalogue, a safety improvement.Stacked on #207. Draft for review.
— Claude on behalf of Cail