ngmix v2.0: one column grammar across estimators (PSF source-of-truth, single size, E↔G)#761
Conversation
…749) Metacalibration has two PSFs and ngmix was exporting only one of them under both column families, mislabeled. Fix the substance and the names together. Substance — the original image PSF is fit again. average_original_psf fits each epoch's pristine gal_obs.psf (the psfex/mccd model stamp) with the existing psf_runner, BEFORE boot.go reconvolves it, weight-averaged by the galaxy obs.weight.sum() — the same scheme average_multiepoch_psf uses for the reconvolution kernel. A shared _average_psf_fits core backs both averagers; do_ngmix_metacal now returns (resdict, psf_res, psfo_res), with psfo_res computed before boot.go so it sees the un-reconvolved PSF. Names — the two families are now self-naming and never alias: *_psf_orig original image PSF (average_original_psf) *_psf_reconv reconvolution kernel (average_multiepoch_psf) Each carries g1/g2 (+_err), T (+_err) and r50 (+_err). The back-fill and compile_results emit loop copy these keys straight through, so the column name is the value's provenance. Galaxy ellipticity is the scalar g1/g2 pair. The previous code emitted one reconvolution-kernel result under both g*_psfo_ngmix and g1_psf/g2_psf; that double-emit is gone. Docstrings (compile_results, the two averagers, do_ngmix_metacal) now name both PSFs explicitly; the old "psfo is the original image psf" note — false since the original fit was dropped — is replaced by an accurate one. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…mar (#749) Adopt NGMIX[m]_<COMPONENT>[_ERR]_<OBJECT>_<SHEAR>, with OBJECT one of GAL, PSF_ORIG (original psfex/mccd image PSF) or PSF_RECONV (metacal reconvolution kernel), reading the self-named keys ngmix now writes: galaxy ELL (2-vec) -> scalar NGMIX_G1/G2_GAL, NGMIX_G1/G2_ERR_GAL galaxy NGMIX_T_ -> NGMIX_T_GAL (+ _ERR), SNR/FLUX/MAG/FLAGS_GAL NGMIX_ELL_PSFo (mis- -> genuine NGMIX_G1/G2_PSF_ORIG (from the restored labeled reconv) original fit) + NGMIX_G1/G2_PSF_RECONV NGMIX_T_PSFo, NGMIX_ -> NGMIX_T_PSF_ORIG and NGMIX_T_PSF_RECONV, each its Tpsf own family's size (+ _ERR on both) The reconvolution-kernel ellipticity gains its _ERR columns too, for symmetry with PSF_ORIG. The d6531b1 dedup (NGMIX_T_PSFo <- "Tpsf") is reverted: PSF_ORIG size now comes from the original fit ("T_psf_orig"), not the reconvolution-kernel "Tpsf" — they are different PSFs again. N_EPOCH / MCAL_FLAGS / MCAL_TYPES_FAIL keep their names (no PSF meaning). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
_fake_metacal_result now emits both self-named PSF key families with
distinct values (original PSF elliptical and smaller; reconvolution kernel
rounder and larger), so any regression to the old single-source aliasing
fails loudly. New tests:
test_compile_results_psf_families_are_unaliased — g1/g2_psf_orig differ
from g1/g2_psf_reconv, each tracing its own source
test_average_original_psf_fits_each_gal_psf_with_galaxy_weight — the
runner fits every epoch's gal_obs.psf, weight-averaged by the galaxy
inverse-variance weight, failed-PSF epochs dropped
The size-columns test now checks both families' r50/T (orig != reconv).
Every do_ngmix_metacal call site unpacks the (res, psf_res, psfo_res)
3-tuple: test_ngmix, test_ngmix_weight_validation, test_mbias,
test_star_response.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…749) The two in-repo param files are the renamed columns' shipped consumer (read by scripts/python/create_final_cat.py). Map every NGMIX token to the NGMIX_<COMPONENT>[_ERR]_<OBJECT>[_<SHEAR>] grammar: NGMIX_ELL_PSFo_NOSHEAR -> NGMIX_G1/G2_PSF_ORIG_NOSHEAR NGMIX_ELL_<SHEAR> -> NGMIX_G1/G2_GAL_<SHEAR> NGMIX_ELL_ERR_NOSHEAR -> NGMIX_G1/G2_ERR_GAL_NOSHEAR NGMIX_T[_ERR]_<SHEAR> -> NGMIX_T[_ERR]_GAL_<SHEAR> NGMIX_Tpsf_<SHEAR> -> NGMIX_T_PSF_RECONV_<SHEAR> NGMIX_T_PSFo_NOSHEAR -> NGMIX_T_PSF_ORIG_NOSHEAR NGMIX_FLUX[_ERR]_<SHEAR> -> NGMIX_FLUX[_ERR]_GAL_<SHEAR> NGMIX_FLAGS_<SHEAR> -> NGMIX_FLAGS_GAL_<SHEAR> Non-NGMIX columns (NGMIX_MCAL_FLAGS, NGMIX_N_EPOCH, NGMIX_MOM_FAIL, and all SExtractor/external columns) are untouched. Every uncommented NGMIX column now matches a column make_cat._save_ngmix_data produces. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
) No make_cat test existed for the renamed ngmix columns. Add four tests driving SaveCatalogue._save_ngmix_data against a synthetic ngmix catalogue (distinct orig vs reconv sentinels): - produced columns follow the new grammar; no old name (_PSFo, _Tpsf, NGMIX_ELL_) survives - NGMIX_G1_PSF_ORIG_* traces the orig source and NGMIX_G1_PSF_RECONV_* the reconv source, and the two differ (the un-aliasing #749 restores) - NGMIX_T_PSF_ORIG_* != NGMIX_T_PSF_RECONV_* - an obj_id absent from the ngmix cat keeps its sentinel fill - end-to-end: make_cat reads a catalogue ngmix's own compile_results + save_results serialised, guarding the reader against key-set drift Soften the _save_ngmix_data grammar docstring so OBJECT and SHEAR read as optional (NGMIX[m]_<COMPONENT>[_ERR][_<OBJECT>][_<SHEAR>]) — the metadata columns NGMIX_MCAL_FLAGS / NGMIX_N_EPOCH / NGMIX_MCAL_TYPES_FAIL carry neither. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…#749) SCIENCE-CRITICAL. average_original_psf called psf_runner.go(gal_obs), and PSFRunner.go sets .gmix (and .meta['result']) on the observation it fits — here gal_obs.psf, the PSF metacal later deep-copies and consumes via boot.go(gal_obs_list). A stray gmix survives that copy and is reused as the MetacalFitGaussPSF fallback when its own admom+ML PSF fits both fail, so objects the base branch dropped (BootPSFFailure) silently survived on this branch, changing the galaxy/shear result set. A rename+add-column refactor must be bit-identical on galaxy results, so fit gal_obs.psf.copy() and read from the copy, leaving gal_obs pristine (verified: gal_obs.psf carries no gmix and no leaked meta['result'] afterward, matching base-branch state). Also in do_ngmix_metacal: - return a MetacalResult NamedTuple (resdict, reconv, orig) so the two PSF families are named not positional, guarding against a reconv/orig transposition; still a tuple, so positional unpacking is unchanged. - rename psfo_res -> psf_orig_res everywhere (no trailing-o shorthand). - soften the average_original_psf weighting note: same form as average_multiepoch_psf (weight.sum(), skip flags!=0) but the reconv path weights by the fixnoise-combined inverse variance of the noshear metacal image while the orig path uses the raw galaxy inverse variance. - note the r50_psf_orig/reconv columns are intermediate-only (make_cat reads T_psf_*/g, not r50, so they do not reach the final catalogue). Tests: a do_ngmix_metacal guard asserting gal_obs.psf carries no gmix after the original-PSF pre-fit; and, on an elliptical PSF stamp, T_psf_reconv > T_psf_orig and |g_psf_reconv| < |g_psf_orig| (true by construction — the reconvolution kernel is round and dilated), which catches a psf_res / psf_orig_res transposition. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
r50 = SIGMA_TO_R50*sqrt(T/2) is a deterministic function of T, which is already exported; the r50_* keys were written to the intermediate ngmix catalogue and read by nobody (confirmed across shapepipe + sp_validation). Remove the computation, schema entries, and the local SIGMA_TO_R50 constant; downstream r50 is derivable via cs_util.size.T_to_r50. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Four property-test modules, each mutation-verified non-vacuous: - grammar: orig/reconv never crossed for any distinct inputs; every emitted column matches the NGMIX_<COMPONENT>[_ERR]_<OBJECT>[_<SHEAR>] grammar; every NGMIX token in final_cat.param is producible by the writer. - physics: T_reconv > T_orig and |g_reconv| <= |g_orig| over random elliptical PSFs (the dilation clause is the robust transposition catcher). - no-op: average_original_psf leaves gal_obs.psf pristine (no gmix leak), so the restored original-PSF fit cannot alter the galaxy/shear results. - averaging: epoch-weighted mean within per-epoch range, flagged epochs excluded, single survivor passes through, sentinel fills for absent objects. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
pyproject pins cs_util>=0.1.9 and the lock already resolved it from PyPI, but to 0.2 — the last release *before* the size module. So the dev image installs cs_util, yet `import cs_util.size` raises ModuleNotFoundError: every path that would use the size-conversion helpers is broken in-container. cs_util 0.2.1 (the current PyPI release, "first release with cs_util.size") ships cs_util/size.py with the Gaussian size web (T_to_sigma / sigma_to_fwhm / sigma_to_r50 / ...). `uv lock --upgrade-package cs-util` bumps only that pin; its dependency set is unchanged, so no new heavy deps land. After the CI image rebuild (`uv sync --frozen` in the Dockerfile) `import cs_util.size` resolves. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The Gaussian size conversions are now sourced from cs_util.size, the single source of truth shared with sp_validation, instead of being re-derived locally: - ngmix.py get_noise window: np.sqrt(guess[4]/2) -> cs_size.T_to_sigma (guess[4] is the ngmix area parameter T = 2 sigma^2; bit-exact). - make_cat.py PSF_FWHM: galaxy.sigma_to_fwhm -> cs_size.sigma_to_fwhm (bare conversion, no pixel_scale). - utilities/galaxy.sigma_to_fwhm becomes a thin wrapper that keeps the pixel_scale rescaling and ShapePipe input validation but delegates the bare 2*sqrt(2 ln 2) factor to cs_util.size; this preserves its second caller (spread_model.py, which uses pixel_scale) and the test_utilities contract. Matches the old hard-coded 2.35482004503 constant to <1e-11 relative. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The PSF property tests (reconv rounder/larger than orig, pristine prefit) need a non-round PSF to exercise. Add an optional psf_shear=(0,0) param to the make_data test helper that shears the Moffat PSF; the default is a no-op, so every existing caller is unchanged. Test-helper only, no pipeline behaviour change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- do_ngmix_metacal: seed the original-PSF pre-fit from a snapshot of the metacal RNG so it no longer advances the rng boot.go consumes — galaxy/shear results are now bit-identical to the pre-PR base (only the *_PSF_ORIG columns are added). Conservative: galaxy science unchanged vs base. - remove the redundant galaxy.sigma_to_fwhm wrapper; inline cs_size.sigma_to_fwhm(sigma) * pixel_scale at call sites (spread_model); cs_util unchanged. - drop leftover psf_fit_prior knob references (docstrings/comments/tests). - canonicalize the ESTIMATOR_COMPONENT_OBJECT grammar spec string across all sites. - trim over-explained copy-PSF / leakage / weighting prose to one canonical spot; document that bit-identity rests on BOTH pristine gal_obs.psf AND the snapshot RNG. - add tests: PSF columns survive a failed-fit NaN-fill; all-epochs-failed raises; grammar token check runs over both shipped param files. Fix a stale cross-ref. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The 12 hand-written g*/T_psf_{reconv,orig} assignments collapse to a single
template applied over (family, source) — same keys, same values, but the two
PSF families are now generated symmetrically, so one can't gain or misname a
column the other lacks. Addresses a drift/hardcoding risk Cail flagged in review.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The two parallel 6-call blocks (PSF_ORIG / PSF_RECONV) collapse to one template over (family, obj): the FITS column name and the res-key it reads are now generated from the same pair, so the write seam (where the grammar drift bit us) can't desync the two families. Behaviour-identical column set + values. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Hello @aguinot @martinkilbinger, this is my first attempt at a fix to all the PSF column issues that cropped up in #741. I decided to be very ambitious and define a common grammar for all column names (not just the PSF columns) to reduce ambiguity; this is quite far reaching since it changes everything that consumes the catalog, and will require a second PR in sp_validation to propagate these changes. However I think it will be worth it and is largely mechanical to propagate. If you think this is too aggressive please let me know and we can consider descoping to only touching the PSF columns. The only real science changes are the new ngmix fit to the original PSF (and the averaging over epochs) in ngmix.py. Cheers, Cail |
|
Hello, |
|
Some notes on the column labelling for the star catalogue. I discussed with @fabianhervaspeters that the HSM shapes are e-types ellipticities, whereas ngmix returns g-type ellipticities. The new labelling shows that, for ngmix, we should ensure the naming is consistent for the HSM quantities and potentially add a function to easily convert between them. |
New cs_util.shape module, parallel to cs_util.size, as the single home for the distortion (e-type) <-> reduced-shear (g-type) conversion used across the UNIONS / ShapePipe stack: g = e / (1 + sqrt(1 - e^2)) (e_to_g) e = 2 g / (1 + g^2) (g_to_e) Component-wise (the WL-native two-component form): both ellipticity components are scaled by the magnitude ratio, preserving orientation. Producers (ShapePipe HSM e-type vs ngmix g-type) and consumers (sp_validation) import from here rather than re-deriving the factors. No version bump: downstream (shapepipe) tracks cs_util @ develop rather than pinning a release, so new helpers are available without a ship. Closes the converter half of Sacha Guerrini's review point on CosmoStat/shapepipe#761. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BXY93SBtDvLLrxWX8Vh4KW
…-container Document how to run the container against a different pure-Python library build (a worktree, an unreleased branch, this repo's own src/ against a stale image) by prepending the host checkout to PYTHONPATH — the local-testing counterpart of a git-ref dependency like `cs_util @ develop`. Includes the -o addopts='' tip for when a pytest plugin the config references isn't in the image. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MgDK9SKn2PsDZGEbr4WFb3
…shear SCIENCE-AFFECTING — the one value change in the column-grammar refactor. The HSM star/PSF moment columns are named E1/E2_*_HSM (e-type, the distortion convention the star/PSF residual ρ/τ statistics expect), but the producers were storing galsim's reduced shear `observed_shape.g1/.g2` (g-type). Name and value disagreed. This swaps the five producer sites to `observed_shape.e1/.e2` so the stored value is the true e-type distortion (|e| = 2|g|/(1+|g|²)), matching the column name. Sites (galsim.Shear.e1/.e2, distortion): psfex_interp.py _get_psfshapes (325-326), _get_starshapes (440-441) mccd_interpolation_script.py PSF (222-223), STAR (280-281) shapepipe_auxiliary_mccd.py PSF (421-422) Closes Sacha's review point (#761): HSM is e-type, ngmix is g-type — name and value must agree. Downstream consequence: ρ/τ-statistic numbers change. Fabian /Axel to confirm the e-type convention is what the residual stats want. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MgDK9SKn2PsDZGEbr4WFb3
…ed as T Rename every HSM moment column to the ShapePipe-v2 grammar ESTIMATOR_COMPONENT_OBJECT and store size as T = 2*sigma^2 (an area) sourced from cs_util.size, across the PSFEx and MCCD producers and their consumers: E1/E2_PSF_HSM -> HSM_E1/E2_PSF E1/E2_STAR_HSM -> HSM_E1/E2_STAR SIGMA_PSF/STAR_HSM -> HSM_T_PSF / HSM_T_STAR (value now T, not sigma) FLAG_PSF/STAR_HSM -> HSM_FLAG_PSF / HSM_FLAG_STAR Size is converted at the earliest honest point: the producers (psfex_interp, mccd_interpolation_script, shapepipe_auxiliary_mccd) store cs_size.sigma_to_T under the HSM_T_* keys -- in the FITS columns AND the SHAPES SqliteDict -- so every consumer reads T straight through. The hand-coded conversions in the consumers drop out: merge_starcat's np.sqrt(size) / **2 and mccd_plot_utilities' 2*sigma^2 arithmetic become plain HSM_T_* reads. One intended numeric change: the merge_starcat MCCD branch derives its output size from the external PSF_MOM_LIST/STAR_MOM_LIST[:,2] (a sigma), now via cs_size.sigma_to_T -- so that validation catalogue's size column changes from sigma^2 to T = 2*sigma^2. Extends test_psfex_interp with an HSM e-type property test (elliptical-PSF fixture, since e vs g vanishes on round shapes) and a producer test pinning the new grammar + T routing. Downstream consumers (sp_validation) break on both the rename and the sigma->T semantics; that ripple is absorbed by the separate #205 PR. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MgDK9SKn2PsDZGEbr4WFb3
* feat(shape): add e<->g ellipticity converters New cs_util.shape module, parallel to cs_util.size, as the single home for the distortion (e-type) <-> reduced-shear (g-type) conversion used across the UNIONS / ShapePipe stack: g = e / (1 + sqrt(1 - e^2)) (e_to_g) e = 2 g / (1 + g^2) (g_to_e) Component-wise (the WL-native two-component form): both ellipticity components are scaled by the magnitude ratio, preserving orientation. Producers (ShapePipe HSM e-type vs ngmix g-type) and consumers (sp_validation) import from here rather than re-deriving the factors. No version bump: downstream (shapepipe) tracks cs_util @ develop rather than pinning a release, so new helpers are available without a ship. Closes the converter half of Sacha Guerrini's review point on CosmoStat/shapepipe#761. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BXY93SBtDvLLrxWX8Vh4KW * Remove the use of np.hypot to avoid a redundant use of a squareroot, use the components directly instead --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: Sacha Guerrini <sacha.guerrini@cea.fr>
|
Thanks @sachaguer, this looks great. The argument for recording I think the last thing on this PR is for a shapepipe expert to take a look at the changes to ngmix.py since I added new functionality to average multi-epoch PSF measurements. @martinkilbinger @aguinot @fabianhervaspeters, could one of you please review that when you have a chance? Once this is merged then we should be able to finally merge #741! |
|
Agreed on the noise properties. I think this product is mainly used for leakage estimation. If any other use I am not aware of would be simplified by saving both quantities, I don't mind saving both. |
* ngmix: give the PSF observation a finite weight map (#749) make_ngmix_observation built psf_obs weightless, so ngmix defaulted to unit weight on the unit-flux PSF stamp and the galaxy GPriorBA(0.4) prior handed to the diagnostic PSF fitter swamped the likelihood. The recovered PSF shape collapsed toward the prior (zero ellipticity), corrupting the *_psf_orig diagnostic columns (#749), and the small finite-weight noise budget present in the pre-v2.0 esheldon/aguinot code was lost in the rewrite (the now-retired #774 — the same bug from the noise-budget angle). Build psf_obs with a flat weight = 1/PSF_NOISE**2 (PSF_NOISE = 1e-5, the esheldon/aguinot value Axel's reproduction used), forced float for parity with the galaxy weight — the PSF analogue of the galaxy weight built just below. Weight only; PSFEx/MCCD models already carry a little noise, so no explicit stamp-noise injection is needed (Axel's guidance on #749). The exact value is non-critical: validated flat across 1e-4..1e-6 on the twin. Calibration is untouched: metacal fits this same psf_obs with a prior-free AdmomFitter, which is insensitive to a flat weight's absolute scale, so the calibrated shear is invariant (verified bit-for-bit on the twin). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017KQn6mXoL4XMgscZDsaTkB * tests: guard the PSF-obs weight end-to-end (#749) Three assertions in test_ngmix_weight_validation.py, on the real production path now that this sits on the column grammar (#761): - structural: make_ngmix_observation builds psf_obs with the finite 1/PSF_NOISE**2 weight, never unit weight; - recovery (#749 done-condition): through do_ngmix_metacal / average_original_psf, the *_psf_orig column recovers the injected PSF moment ellipticity to 1e-3 with the weight, and collapses below 10% without it (the contrast that proves the weight is load-bearing for the diagnostic columns); - invariance: every metacal-type shear is bit-for-bit identical between the unit-weight (pre-fix) and shipped PSF_NOISE builds (metacal's prior-free AdmomFitter is insensitive to the flat weight's scale). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017KQn6mXoL4XMgscZDsaTkB --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Downstream of CosmoStat/shapepipe#761, which makes the ngmix output columns a single source of truth (true original-PSF fit stored separately, ELL split into scalar G1/G2, ESTIMATOR_COMPONENT_OBJECT grammar). Captures the old->new column map and the sp_validation consumer checklist. Blocked on #761 landing. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Hey, |
|
Thanks Axel, I think your suggestion is what Sacha implemented. |
ShapePipe has no galsim shape-measurement runner -- only ngmix_runner measures shapes and writes NGMIX_* columns. The galsim family in make_cat (_save_galsim_shapes, the process() "galsim" branch) could only be populated by reading an external shape catalogue that nothing in the pipeline produces; every production config pins SHAPE_MEASUREMENT_TYPE = ngmix. #761 renamed these columns onto the v2 grammar, but they are not creatable end-to-end -- confirmed from the sp_validation side, where the mirror galsim consumer was removed as dead code (sp_validation #201). Remove the dormant writer rather than carry grammar-renamed columns no pipeline can exercise. - delete SaveCatalogue._save_galsim_shapes and its process() "galsim" dispatch branch; process modes are now "ngmix" or "psf" - drop the now-orphaned cs_util.size import (only the galsim block used it) - delete tests/module/test_galsim_grammar_properties.py, which drove the removed writer Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0191bMtgT2gcmcT7fNYbGLS1
ngmix is now the pipeline's only shape estimator, so make_cat_runner validates SHAPE_MEASUREMENT_TYPE against ngmix alone and raises on anything else. The knob is kept (rather than hardcoded) as the extension point for a future estimator family, mirroring sp_validation's retained `prefix`. The shape-write loop drops the galsim-vs-ngmix catalogue selection -- shapes are always read from the single ngmix input. Docstring and the cat_matched.param comment lose their galsim references. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0191bMtgT2gcmcT7fNYbGLS1
The static grammar test covered ngmix, galsim, and HSM. With the galsim writer gone, drop its regex branch and its valid/invalid example columns; the frozen grammar now spans the two families ShapePipe actually produces (ngmix, HSM). Also drop the docstring pointer to the deleted test_galsim_grammar_properties.py. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0191bMtgT2gcmcT7fNYbGLS1
|
Follow-up: remove the dead galsim shape path #761 brought the
Full suite green in-container (310 passed, 2 deselected). Considered but not done: — Claude on behalf of Cail |
|
Hello,
|
Restores the galsim shape-serialization path that commits e036a95, df84064 and 1c69af5 removed: - _save_galsim_shapes() writer in make_cat.py (new-grammar form) - GALSIM in SHAPE_MEASUREMENT_TYPE (make_cat_runner, __init__) - test_galsim_grammar_properties.py and the galsim frozen-grammar case Per Axel Guinot's 2026-07-02 review of #761: the galsim dead-path removal is unrelated to the reconvolved-PSF / column-grammar purpose of this PR ("too many things at once"). The removal is good work and stands on its own — it moves to a dedicated PR against ngmix_v2.0 rather than riding here. This revert keeps #761 scoped to the PSF column grammar. This reverts the net effect of e036a95, df84064 and 1c69af5. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GQSyaXLnLZtksntYAYD41E
Undoes the spread_model removal that this branch carried (85a18ab in full, plus the save_sm_data half of 4f0756d), so #761 no longer touches spread_model at all: - spread_model_package/ + spread_model_runner.py restored - make_cat_runner: spread_model_runner input, sexcat_sm pattern, SM_* config, and the save_sm_data() call restored - make_cat.save_sm_data() writer + SPREAD_MODEL / SPREADERR_MODEL / SPREAD_CLASS columns restored (re-added by hand alongside the new grammar, since 4f0756d deleted the writer while unifying the grammar) - example configs, .param allow-lists, pipeline tutorial restored Per Axel Guinot's 2026-07-02 review: his position is that spread_model must not be *cut on* for shear selection — not that it should be deleted ("it could still be useful for other things... it does not hurt to keep it... this is not the right PR to remove it"). The earlier deletion over-read that point. Whether spread_model should be removed in a dedicated future PR is a separate question, addressed in a standalone report; #761 is scoped to the PSF column grammar and leaves it in place. The grammar unification of 4f0756d (ngmix / galsim / HSM-sink columns) is untouched — only the spread_model writer it also deleted is restored. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GQSyaXLnLZtksntYAYD41E
* docs: record the ngmix PSF/shape column migration plan Downstream of CosmoStat/shapepipe#761, which makes the ngmix output columns a single source of truth (true original-PSF fit stored separately, ELL split into scalar G1/G2, ESTIMATOR_COMPONENT_OBJECT grammar). Captures the old->new column map and the sp_validation consumer checklist. Blocked on #761 landing. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs: rewrite column-migration map to the as-shipped v2 grammar The plan committed 2026-06-20 predated the "drop GAL" decision on shapepipe#761: it showed every galaxy column gaining a _GAL token and said nothing about HSM. Rewrite it to the authoritative as-shipped grammar, grounded in the #761 producer source: - galaxy is the implicit object (no _GAL token); ellipticity splits into named scalars NGMIX_G1/G2_{shear}, errors likewise; - NGMIX_Tpsf -> NGMIX_T_PSF_RECONV (value-safe); ELL_PSFo/T_PSFo -> G1/G2/T_PSF_ORIG (value change, shapepipe#749); - HSM E*_{PSF,STAR} -> HSM_G*_{PSF,STAR} (native g, pure rename), SIGMA_*_HSM -> HSM_T_* (units: now T=2sigma^2), FLAG_*_HSM -> HSM_FLAG_*; - spread_model removed; sizes route through cs_util.size; square_size retired. e->g conversion is not needed anywhere (both estimators store g). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M8BwwtExg961NknUeu5GCv * refactor(src): adopt ShapePipe-v2 column grammar in the package Consumer half of shapepipe#761. Every shape-column read in the package moves onto the one grammar: - metacal (calibration.py): galaxy ellipticity read as named scalars NGMIX_G1/G2_{shear} and errors NGMIX_G1/G2_ERR_{shear} (was packed NGMIX_ELL_*[:, c]); NGMIX_Tpsf -> NGMIX_T_PSF_RECONV. The obsolete col_2d flag (packed-vs-split ellipticity) is removed from the metacal ctor and get_variance_ivweights — v2 is always named scalars. - galaxy.py: NGMIX_ELL_PSFo[:, 0] -> NGMIX_G1_PSF_ORIG scalar; the spread_model branch/param dropped (shapepipe stopped writing it) — classification falls back to the size-based path. - catalog.py: match_spread_class deleted (SPREAD_CLASS gone); match_subsample takes two scalar keys (g1_key, g2_key) instead of a packed ell_key; check_invalid drops the now-meaningless comp arg. - glass_mock.py: HSM E1/E2_PSF_HSM -> HSM_G1/G2_PSF. - rho_tau.py: sizes are always T=2sigma^2 now, so square_size is False and the not_square_size list is gone. - tests: synthetic catalogues + configs rebuilt on the v2 grammar in lock-step (the migration's internal-consistency check). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M8BwwtExg961NknUeu5GCv * refactor(config): rename HSM/ngmix columns to v2 grammar, drop square_size - cat_config.yaml: every HSM block onto the v2 grammar — E*_{PSF,STAR}_HSM -> HSM_G*_{PSF,STAR}, SIGMA_*_HSM / T_*_HSM -> HSM_T_*, FLAG_*_HSM -> HSM_FLAG_*. The DES/piff block is untouched (Piff isn't this grammar). The dead per-dataset square_size flags are dropped: sizes are always T=2sigma^2 now, so nothing squares (this also fixes a latent factor error in the SP_v1.3 block, which squared its already-T T_PSF_HSM). - mask_v1.X.{2..11}.yaml: NGMIX_ELL_PSFo_NOSHEAR_0/_1 -> NGMIX_G1/G2_PSF_ORIG_NOSHEAR. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M8BwwtExg961NknUeu5GCv * refactor(scripts): migrate scripts, papers, cosmo_inference to v2 grammar Non-library consumers onto the same grammar: - scripts/calibration/{extract_info,params,calibrate_comprehensive_cat}.py: Tpsf -> T_PSF_RECONV; ELL_PSFo -> G1/G2_PSF_ORIG; T_PSFo -> T_PSF_ORIG; bare galaxy ELL -> G1/G2 scalars (match_subsample/check_invalid callers updated for the new signatures); HSM E*_PSF -> HSM_G*_PSF; spread_model wiring removed; col_2d args dropped. - scripts/apply_alpha_snr_size_bin.py, scripts/examples/demo_*.py: same Tpsf/PSFo renames, col_2d dropped. - cosmo_val/compute_theory_cov.py, cosmo_inference/scripts/masking.py: Tpsf -> T_PSF_RECONV; square_size hardcoded False. - papers/catalog/*, papers/harmonic/*: HSM ellipticity renames and the sigma->T units fix (hand-rolled SIGMA_*_HSM**2 -> read HSM_T_* directly). Out of scope, left as-is: GALSIM_* columns, scratch/, and the cosmo_inference notebooks (own migration fiber). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M8BwwtExg961NknUeu5GCv * refactor(src): remove dead galsim path, finish v2 grammar tightening Two tightening-pass changes to complete ShapePipe-v2 column adoption in the package: - Remove the galsim estimator path as dead code. `shape` is hardcoded to "ngmix", extract_info.py raises for any other value, and nothing outside scratch/ instantiates metacal(prefix="GALSIM") or calls classification_galaxy_galsim. Migrating it would carry an untestable path whose shared `col_1p = {prefix}_T_PSF_RECONV_1P` read never matched the galsim producer output (GALSIM_T_PSF_*, not ..._T_PSF_RECONV_*) — already broken. Removed: metacal._read_data_galsim, the prefix=="GALSIM" dispatch (now else: raise — unknown prefixes fail loudly), the two galsim ellipticity sign flips in _shear_response/_selection_response, galaxy.classification_galaxy_galsim, the sh=="galsim" branch in catalog.get_snr (now else: raise), and the unused shape_method arg on get_calibrated_quantities/get_calibrated_m_c. ngmix is the sole estimator; the `prefix` param stays (names the column family; a future NGMIXm moments family could reuse it). - Rename the galaxy failure-flag read NGMIX_MOM_FAIL -> NGMIX_MCAL_TYPES_FAIL in classification_galaxy_ngmix. The v1 column counted moments-initial-guess failures (get_guess, gone in v2); the producer reused the slot for a failed-metacal-types count. sp_validation cuts on == 0 either way (keep fully-measured objects), so the read migrates cleanly; the underlying failure mode changed, so this is the first line to check if the post-cut galaxy count looks off against a regenerated v2 catalogue. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019f8w2twg4b3Ga36dSxNbLW * refactor(config): NGMIX_MOM_FAIL -> NGMIX_MCAL_TYPES_FAIL in mask configs The v1 failure-flag column no longer exists in the ShapePipe-v2 header; the producer renamed the slot to NGMIX_MCAL_TYPES_FAIL. Propagate the rename into all ten calibration mask configs so the flag column resolves against a v2 catalogue. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019f8w2twg4b3Ga36dSxNbLW * refactor(scripts,papers): finish v2 grammar tightening in scripts - NGMIX_MOM_FAIL -> NGMIX_MCAL_TYPES_FAIL in params.add_cols_pre_cal (and its int-format set), the two demo column lists, and hist_mag's read lists. - Drop the galsim mentions left in params.py (shape comment) and extract_info.py (the stale "cuts common to ngmix and galsim" comment, whose spread-model line also went with the earlier spread_model removal). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019f8w2twg4b3Ga36dSxNbLW * refactor(notebooks): migrate cfis_analysis HSM PSF columns to v2 grammar cfis_analysis.ipynb read E1/E2_PSF_HSM into treecorr for the C_sys PSF test; rename to HSM_G1/G2_PSF (native g, pure rename). A sweep of all tracked notebooks found no other old column tokens. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019f8w2twg4b3Ga36dSxNbLW * docs: record galsim removal, MOM_FAIL rename, and *_PSF_ORIG reframe Update the authoritative migration map for the tightening pass: - *_PSF_ORIG reframed as a value change that is NOT a code blocker (the rename is correct as-is; a v2 catalogue only enables a look-at-numbers check). The real merge gate is cutover timing — merging makes develop require v2 columns. - NGMIX_MOM_FAIL -> NGMIX_MCAL_TYPES_FAIL added to the ngmix map with a semantics-change note. - galsim path documented as removed dead code (was "left untouched, flagged"). - notebooks added to the consumer-sites list. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019f8w2twg4b3Ga36dSxNbLW * refactor(rho_tau,cov): drop square_size, coordinate with leakage #27 shear_psf_leakage #27 removed the square_size parameter from build_cat_to_compute_{rho,tau} and CovTauTh (HSM_T_* already stores the area T=2σ², so the flag was dead weight). sp_validation still threaded square_size into those leakage calls; once #27's container lands, passing it would raise TypeError. Drop the square_size key from both param builders (rho_tau.get_params_rho_tau, cosmo_val/compute_theory_cov.py) and stop passing it to the handlers. Behavior-identical against the current container (square_size defaults to False), required for the post-#27 one. Also correct the migration doc: galsim shapes are not producible by ShapePipe (no shape-measurement runner; make_cat's galsim mode reads an external catalogue nothing generates; production configs pin ngmix) — the decisive reason the dead consumer path was removed rather than migrated; and reframe the shear_psf_leakage coordination now that #27 is a sibling PR landing the same grammar. Suite: 137 passed, 1 skipped (unrelated cmss12.tfm font gap). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GnCCTxKE13cHzPr8BUeKZE --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
The HSM column-grammar rename (4b17f7e) renamed the SqliteDict SHAPES key SIGMA_PSF_HSM -> HSM_T_PSF and changed its value from the Gaussian sigma to T = 2 sigma^2. spread_model still read the old key, so with spread_model restored to this PR it would KeyError at runtime. Read HSM_T_PSF and convert back to sigma via cs_util.size.T_to_sigma, which the rest of the module (get_model, the >0 guard, sigma_to_fwhm, the galsim Gaussian) expects. Mean-of-sigma semantics are preserved: each per-exp/ccd T is converted to sigma before averaging, exactly as before. This completes the grammar rename for spread_model's one consumer so #761 no longer ships it broken. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JSoGrXLyNr74SjiFw5FVkB
|
Thanks Axel. I split off the galsim cleanup to keep this PR focused, and renamed it to reflect the actual scope as tracked by issue #777. It would be very messy to separate the star and psf column renames into different PRs at this point. I also restored the spread model module and made it work with the new column names. I think we can merge this PR once CI finishes. |
_save_psf_data still read HSM_E1_PSF/HSM_E2_PSF from the multi-epoch PSF sqlite, but every producer (psfex_interp._interpolate_me, mccd) writes HSM_G1_PSF/HSM_G2_PSF since the return to g-type ellipticities (#761) — a guaranteed KeyError on the first unflagged PSF epoch whenever SAVE_PSF is enabled. Rename the consumer (and its output columns) to the g-type grammar. The frozen-grammar property test encoded the same stale E-type convention for HSM, rubber-stamping the mismatch: fix the regex and examples (HSM_E1_PSF is now an explicit invalid example), and align the stale e-type docstrings in test_psfex_interp with the g-type behavior the assertions actually pin. Found by the final integrated review of #741. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TVYoKzcNwYjuGiFi9CxsYP
…-07-04) Move Fabian's star metacal shear-response R1-vs-magnitude panel (45,835 stars) into the main status page's star-response section, with the benign-for-shear framing (bright-end climb is the point-source T->0 floor; resolved galaxies hold the flat symmetric-response regime). Add a dated m-bias note: the image-sims chain is being rebuilt on one consistent post-#761 new-grammar container vintage with raw-input staging fixed; m2~=-0.80 flagged as a stale-input plumbing smoke, not a calibration result (ideal m=-2e-3, guardrail |m|<5e-3). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013uTmBAtdP5Wvav2M76MgCA
ngmix: single source of truth for original vs reconvolved PSF columns
Follows the #741 review discussion of the ngmix PSF columns. The output conflated the original and reconvolved PSF — both
*_psfand*_psfocarried the reconvolved fit, sog1_psf(expected ~0 on the metacal image) silently held the original-PSF value, andTpsfdid not mean what its name says. This PR makes each PSF quantity a single source of truth whose name matches its meaning, and unifies the catalogue column grammar across the ngmix and HSM star/PSF columns.Where to focus review
The science lives entirely in
ngmix.py— specifically the twoaverage_*_psfweighting functions (how each PSF family is combined across epochs) and thedo_ngmix_metacalwiring (the metacal call + the new original-PSF pre-fit and its RNG handling).make_cat.py,psfex_interp.pyand the.paramfiles are mechanical serialization of the new column names; the rest is test coverage.Changes
ESTIMATOR_COMPONENT_OBJECTgrammar; update the shipped.paramfiles.E1/E2for e-type, size stored asTviacs_util.size), including the fix to store the true e-type distortion in theE*_HSMcolumns. This is the "one grammar across estimators" that this PR exists for (issue ngmix v2.0: one column grammar across estimators (PSF source-of-truth, single size, E↔G) #777,E↔Gfacet), and was reviewed with Sacha.r50columns (derivable fromTviacs_util.size).cs_util.size.Scope (updated after Axel's 2026-07-02 review)
This PR is now held to the reconvolved-PSF / column-grammar purpose. Two things it previously carried have been pulled back out:
ngmix_v2.0once this merges.Notes for review
*_PSF_ORIGcolumns are added. (Rationale in thedo_ngmix_metacalcomment.)ngmix_v2.0— lands in Ngmix v2.0 (CI mirror of #740) #741 before the v2.0 merge.sp_validation— tracked in a separate draft PR there (fixed bug in file handler - [merged] #201).— Claude on behalf of Cail
Closes #777