ngmix v2.0: one column grammar across estimators (PSF source-of-truth, single size, E↔G) - #761
Merged
Merged
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>
cailmdaley
marked this pull request as ready for review
June 20, 2026 17:21
Contributor
Author
|
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 |
Contributor
|
Hello, |
Contributor
|
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. |
cailmdaley
added a commit
to CosmoStat/cs_util
that referenced
this pull request
Jun 30, 2026
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
* 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>
4 tasks
cailmdaley
added a commit
to CosmoStat/sp_validation
that referenced
this pull request
Jul 1, 2026
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>
Contributor
|
Hey, |
Contributor
Author
|
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
Contributor
Author
|
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 |
martinkilbinger
approved these changes
Jul 2, 2026
Contributor
|
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
This was referenced Jul 3, 2026
cailmdaley
added a commit
to CosmoStat/sp_validation
that referenced
this pull request
Jul 3, 2026
* 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
Contributor
Author
|
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. |
cailmdaley
added a commit
that referenced
this pull request
Jul 3, 2026
_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
cailmdaley
added a commit
that referenced
this pull request
Jul 4, 2026
…-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
cailmdaley
added a commit
that referenced
this pull request
Jul 6, 2026
No runner produces galsim shapes (there is no galsim_shapes_runner) and the dual-shape (len==5) input branch in make_cat_runner never fires — ngmix is the pipeline's only shape estimator, so the galsim serialization path was dead code. - delete SaveCatalogue._save_galsim_shapes and its "galsim" process mode (drops the now-orphaned cs_util.size import from make_cat) - make_cat_runner validates SHAPE_MEASUREMENT_TYPE against ngmix alone and drops the dead second-shape-catalogue branch; the list knob is kept as the extension point for a future estimator family - drop test_galsim_grammar_properties.py and the galsim cases from the frozen column-grammar examples in test_psf_grammar_properties.py - lose the galsim references in the make_cat docstring, the make_cat_mccd example config, and the cat_matched.param comment Carved out of #761 to keep that PR scoped to the reconvolved-PSF column grammar; the removal deletes a test #761 introduced. Closes #783 Co-Authored-By: Claude <noreply@anthropic.com>
This was referenced Jul 6, 2026
cailmdaley
added a commit
that referenced
this pull request
Jul 15, 2026
* testing/simulate: added Pujol denoising estimator of shear bias * Running SP v2.0 on image simulations, up to -j 8 * SP v2.0 running on image simulations including ngmix * Running on im sims incl final cat * ÃÃÂadded datetime to Mh run * Âinit v2.0: cd in new subdir * ÂÂimage sims run updates * Âsmall changes * output mean e to ngmix log * doc * ÂÂrun script : added --retry * im_sims docker deployment * added .py to executables * fixe pyproject bug * pyproject moved back to exe scripts without extension * running create_final_cat.py on image_sims * Pipeline running on image sims * config files: change batch size to 1 (default) * snakemake setup for image simulations * fixed snakefile * test for zero size FITS output files * zero-size output fixes * more automatisation for image sims snakemake * ngmix+vignets: handling of empty input PSF cats; scripts: improved snakemake running * retarget(im_sims): revert incidental stale-base edits to shared modules Keep only genuine sims-enablement; revert shared-module changes that were side-effects of the stale ngmix_v2.0 base and collide with in-flight work: - make_cat: restore HSM_G*_PSF grammar (fix/ngmix-psf-columns) + spread_model (714847c, split-761); drops the input_file_List NameError - vignetmaker: drop broad try/except churn (_w_log AttributeError -> test fail) - mask: restore CDSCLIENT_PATH knob (live in 4 example configs) - restore pipeline_tutorial.md (toc.rst ref -> Sphinx build) + run_scratch_local.sh Verification branch only; not for push. Grounds the #766 review. * retarget(im_sims): slim to ShapePipe module enablement; orchestration -> sp_validation The image-sims workflow splits by concern along the repo dependency arrow (shapepipe produces catalogues; sp_validation consumes + validates them): - ShapePipe (this PR): only the pipeline-module changes needed to run on background-free sims -- the fake_psf module, ngmix BKG_SUB path, numeric sim-exposure-ID handling (exp_utils/sextractor/merge_headers), the cfis_image_sims module configs, create_final_cat's image_sims mode, and make_data_pujol. - sp_validation (separate PR): the snakemake orchestration + the merge/calibrate/m-bias half, rebuilt on sp_validation's native container workflow. The orchestration files removed here are preserved in git history (origin/im_sims) and move there. Incidental stale-base edits to shared modules/standard configs/tests reverted to ngmix_v2.0 (which already carries the fixes: 662fad4 HSM g-cols, 714847c spread_model). Retargeted onto ngmix_v2.0; uv.lock regenerated. * fix(im_sims): restore image-sims job dispatch in the ShapePipe runner The ngmix_v2.0 retarget slimmed run_job_sp_canfar_v2.0.bash and job_sp_canfar_v2.0.bash of their image-sims code paths, leaving #766 unable to run the image-sims pipeline at all: run_job had no -t/--type dispatch (so the sp_validation workflow's `-t image_sims` was silently swallowed and it ran standard-DATA mode), and its job-1024 completeness check referenced ngmix_interp_runner, a runner that exists nowhere in the codebase. job_sp_canfar rejected `-r symlink` ("vos only for v2.0"), named the Gie and make_cat configs for the data path only, and still invoked shapepipe_run.py (the console entry point is `shapepipe_run`, no extension, since 797eeff). run_job: restored the canonical image-sims runner (full -t/--type dispatch — cfis_image_sims config dir, symlink retrieve, tile_det=sx, faked Uz weight-uncompress, fake_psf under job 512, job-2048 make_cat, and the correct ngmix_runner check). Made it relocatable per #766's goal: a self-derived $SP_ROOT replaces ~/shapepipe for sourcing functions.sh/job_list_help.bash and resolving config dirs, and job_sp_canfar is invoked by explicit repo path (it is not on the container PATH). run_tile_job now re-anchors cwd to the tile work dir on entry: SP_RUN/SP_CONFIG derive from cwd, and run_exp_job leaves cwd in the last exposure dir, so when run_job is called once for the whole pipeline (-j 4095, as the workflow does) the post-exposure tile jobs would otherwise execute in the stale exposure dir. job_sp_canfar: kept the ngmix_v2.0 base — the #746 NUMBER_LIST mechanism that matches the new-grammar container (the canonical runner still uses the retired shapepipe_run -e flag and would crash) — and grafted the image-sims bits: accept `-r symlink`, config_exp_Gie_$retrieve, config_tile_Mc_$psf (the config present in cfis_image_sims and matching run_job's job-2048 check), the bare `shapepipe_run` entry point, and a self-derived $SP_ROOT. Verified on candide against the post-#761 container: `-t image_sims` selects cfis_image_sims + symlink + sx and runs Git -> faked-Uz -> Fe -> Gie(7/7) -> Sp -> Ma -> Mh_exp, each in its correct work dir with new-grammar shapepipe_run. The two scripts run from the bind-mounted repo, so the container supplies only the Python stack. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019Jd85D8xNZZ43FgesabSnk * fix(im_sims): add SM_DO_CLASSIFICATION to the image-sims make_cat config The image-sims make_cat config lacked SM_DO_CLASSIFICATION, which the newer make_cat_runner reads unconditionally (NoOptionError -> job 2048 crashed, so the per-tile final_cat was never built). The image-sims pipeline has no spread_model_runner stage (make_cat inputs are sextractor + fake_psf + ngmix only), so there is nothing to classify: set it False. With this, a single tile completes end-to-end (Git -> ... -> ngmix -> make_cat), producing a 2345-object final_cat-233-293.fits against the post-#761 container. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019Jd85D8xNZZ43FgesabSnk * fix(im_sims): new-grammar final_cat.param for the image-sims pipeline create_final_cat selects this column list from the per-tile make_cat output into final_cat_{sim}.hdf5; the list was still old-grammar (NGMIX_ELL_*, NGMIX_Tpsf_*, NGMIX_T_PSFo_*, NGMIX_MOM_FAIL) while the post-#761 make_cat writes NGMIX_G{1,2}_*, NGMIX_T_PSF_ORIG/RECONV_*, so the merge died on 'NGMIX_Tpsf_2M' is not in list. Rewrite to the v2 grammar, mirroring example/cfis/final_cat.param minus the two columns the simulation pipeline does not emit (IMAFLAGS_ISO: no imaging-flag masking stage; NGMIX_MOM_FAIL: superseded by NGMIX_MCAL_TYPES_FAIL, which is included). Verified: all 65 columns present in the per-tile cat, merge -> extract -> calibrate runs clean on 1z2z_grid_1. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D9atA9VPrg7VMC7oBArny9 * fix(mask): don't crash stdout handling when star masking is off With HALO_MAKE and SPIKE_MAKE both False (the image-sims mask config), find_stars never runs, so _CDS_stdout is never set and the unconditional read at the end of make_mask raised AttributeError for every CCD. The flag files were already written, so each mask stage 'failed' with 40 bookkeeping errors and exit 1, relying on the runner's continue-on-error to keep the pipeline alive. Guard the CDS block with hasattr, matching the WeightWatcher/rm blocks below it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TWURLMoixhuMqVrPVdRmCp * ngmix: position-seeded per-object RNG for Pujol noise cancellation (#796) Add an opt-in SEED_FROM_POSITION knob to the ngmix module (default False). When on, the object loop replaces the tile-level RNG with a per-object RandomState seeded from the object's sky position, and rebuilds the prior from that same RNG, so every stochastic step of the fit is a deterministic function of (ra, dec, ccd). Why: image-sim m-bias uses the Pujol estimator — the same scene under different applied shears, with shear response read from the branch difference of the SAME objects. Metacal's fixnoise adds a counter-noise realisation from an RNG seeded per tile, so an object gets different added noise in different branches (detection order differs) and the noise fails to cancel in the difference, inflating sigma_m. Seeding per object from position makes the same object draw the same added noise AND the same fit guesses in every branch, so both cancel and the m-bias error shrinks. Design and box math are Fabian's issue #796. The prior must be rebuilt per object, not just the RNG: the ngmix guesser draws its initial guess via prior.sample(), which consumes the RNG the prior was CONSTRUCTED with (ngmix 2.4.0 guessers.py / joint_prior.py). A per-object RNG alone would leave the guess drawing from the shared tile stream and break cancellation. The fixnoise counter-noise itself rides on the observation's attached noise image (use_noise_image=True), which prepare_ngmix_weights draws from the per-object RNG — so no fresh draw from the bootstrapper RNG is needed. Deviation from #796: Fabian combines the 3-arcsec position boxes as box_x + box_y, which collides along anti-diagonals — boxes (10,20) and (11,19) both give seed 30, so two distinct objects share a noise stream. This replaces the sum with a Cantor pairing (a bijection on the non-negative integers) after a zig-zag fold that maps signed box indices (Dec can be negative) onto non-negative ones, reduced mod 2**32 into the valid RandomState seed range. The box math is kept exactly as his: floor(ra*3600/3)+(ccd+1), floor(dec*3600/3)+(ccd+2), first-epoch coordinates (coord_list[0] convention), so all epochs of one object share the stream. Off by default => production path is byte-identical to develop (verified: metacal g for all five types matches unmodified develop bit-for-bit; the full fast suite passes, 326 tests). New property tests cover the seed's range, sub-box jitter invariance, adjacent-box distinctness, and the resolved anti-diagonal collision. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01J9qEVRCWEekT5ACNB3rjkq * sims config: enable SEED_FROM_POSITION for the A/B noise-cancellation run Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UvQvyJTEwYcLCwDEgpWMJ6 * running smoother on im sims * Fix nfev_fit int64 dtype lock crashing batch-saved ngmix output compile_results filled nfev_fit with np.nan on failed fits, the only int-like column with a float fallback. With SAVE_BATCH enabled, save_results locks column dtypes from batch 1; if that batch has no failures the column locks to int64, and the first later batch with a failure feeds NaN into it, raising "cannot convert float NaN to integer" on append. Use ngmix's own -1 failed/absent-nfev sentinel instead of NaN, keeping the column int64 across every batch. Fixes #795 * im sims: coverage scan script for (near-)empty input tiles Companion to the coverage rule in the Snakefile: scans input weight maps (subsampled), writes tile_coverage_grid_{num}.yaml with an exclude list read at DAG-build time. Weight-map location is taken from the get-images ini of the 1p2z variant. Plus two felt fibers documenting the blank-tile and thread-exhaustion findings. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * restore apptainer_noslurm.sh dropped in im_sims_candide merge The image_sims_pipeline Snakefile invokes all container commands through this wrapper; it strips SLURM/PMI/PMIX/OMPI env vars that crash OpenMPI inside apptainer on SLURM compute nodes. No replacement exists in run_scratch_local.sh or run_job_sp_canfar_v2.0.bash. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Retire in-repo image_sims Snakemake orchestrator The image-simulation pipeline's orchestration now lives in sp_validation (companion PR #225), which drives ShapePipe via `run_job -t image_sims --config_dir`. The in-repo Snakemake driver under scripts/image_sims_pipeline/ (Snakefile, info.py, monitor_mbias.py, config.yaml.template) is superseded and removed to keep a single source of orchestration truth. Its coverage-scan helper, check_tile_coverage.py, is the one piece of reusable logic with no equivalent elsewhere; it moves to scripts/python/ alongside the other run-directly helpers (create_final_cat.py, etc.) and gains the executable bit to match that directory's convention and its own shebang. docs/source/image_sims_calibration.md documented only this retired orchestrator (its Snakemake targets, info.py, monitor_mbias.py) and was an orphan page not wired into any Sphinx toctree; it is removed with the code it described. History of the old pipeline is preserved in the .felt/ notes, which are left untouched as a chronological record. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012Z3fwStiamMsEswbJ66BeF * run_job: add -c/--config_dir to override computed config dir The runner self-computes config_dir from --type (example/cfis for data, example/cfis_image_sims for image_sims) and threads it through the cfis symlinks, SP_CONFIG, and job_sp_canfar's own -c. That hardcodes the .ini tree to the in-repo example dirs. Add an optional -c/--config_dir that, when given, overrides the computed config_dir uniformly for any --type. The override is applied after the type branch, so when the flag is absent config_dir is byte-for-byte the historical value and every existing caller (test_tile_det.py, functions.sh, the sp_validation im_pipeline rule) is unaffected. This lets the sp_validation workflow point every stage at an explicit .ini tree, per the image-sims boundary rule (workflow selects and parameterizes the tree). The flag name matches job_sp_canfar_v2.0.bash's own -c/--config_dir. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012Z3fwStiamMsEswbJ66BeF * fake_psf: source PSF dictionary path from the environment The cfis_image_sims fake_psf config hardcoded a collaborator-specific absolute path to the SKiLLS PSF dictionary, coupling the in-repo config tree to one machine's layout. Replace it with PSF_DICT_PATH = $PSF_DICT, which the config layer's getexpanded (os.path.expandvars) resolves from the environment. The orchestrating workflow now exports PSF_DICT from its psf_dict config key, so the campaign fact lives upstream in the manifest, not in this repo -- matching the module-config/workflow boundary rule. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012Z3fwStiamMsEswbJ66BeF * docs(cfis_image_sims): document image-sims job-bit dispatch Add a README to example/cfis_image_sims/ giving the accurate, per-bit mapping of the image-sims chain: for each job bit (1..2048), the ShapePipe module, the .ini file job_sp_canfar_v2.0.bash selects under sim settings, and the special-casing run_job_sp_canfar_v2.0.bash applies. Every row is derived by reading the two bash scripts, so the table stays a truthful reference for the sims path rather than a guess. Captures the parts that surprise: the faked weight-uncompress at bit 2, the placeholder bit 64 with fake_psf actually running inside bit 512, the dashed tile-id convention, forced tile_det=sx, and symlink retrieval. Adds the parallelism model (per-tile fan-out, in-job SMP, no MPI) and the $PSF_DICT env requirement for fake_psf. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012Z3fwStiamMsEswbJ66BeF * im sims Snakefile small change * improved reading of merged hdf5 cat for info * added comment * fix(packaging): drop stale init_run_v2.0.sh from script-files The [tool.setuptools] script-files list referenced scripts/sh/init_run_v2.0.sh, which was removed in b0804ce when the im_sims retarget moved orchestration to sp_validation. setuptools' build_editable errors on the missing file, breaking every Docker build in CI (`uv pip install --no-deps -e .` → "file '...init_run_v2.0.sh' does not exist"). The other three listed scripts (update_runs_log_file.py, run_job_sp_canfar_v2.0.bash, job_sp_canfar_v2.0.bash) still exist, so this removes only the stale entry. Verified with an editable-install dry-run in the im_sims runtime container: resolves to shapepipe-1.1.0 with no build_editable error. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * chore(im_sims): strip MKDEBUG markers; refresh check_tile_coverage docstring Polish over the image-sims files this PR touches: - check_tile_coverage.py: correct the module docstring's stale "the Snakefile reads the exclude list" wording — orchestration moved to sp_validation (f01344a), so the caller is now the sp_validation workflow, not an in-repo Snakefile. - bin/shapepipe_run.py: drop the "MKDEBUG start/end" print statements that wrapped main(). - ngmix.py: drop two "# MKDEBUG TODO this needs to be checked!" tags appended to the zero-sigma / zero-noise weight guards; the preceding comments already explain why the guards exist. Only strips MKDEBUG introduced by the im_sims merges; the pre-existing marker in create_final_cat.py (also on develop) is left untouched. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * chore: untrack .felt fiber notes that leaked in via im_sims Two private fiber notes (blank-input-tiles-grid-4, snakemake-crash- runtimeerror) were committed on origin/im_sims in 9783a2e, before the develop-side .felt untrack (3d56ec6) landed there, so they survived the merge into this branch. The repo gitignores /.felt/ and treats it as a machine-local symlink into the private loom store (see CLAUDE.md); these tracked files bypass that ignore. Untrack them (kept on disk) so no fiber content rides into the shared CosmoStat repo. Their operational lore is preserved in sp_validation's workflow docs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: martinkilbinger <martinkilbinger@cea.fr> Co-authored-by: Cail Daley <cail.daley@cea.fr> 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
4 participants
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.
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