fix: dimensionally wrong T_to_fwhm distorted the PSF-leakage size axis#198
Merged
Conversation
galaxy.py's local T_to_fwhm was T / 1.17741 * 2.355, which treats the
ngmix area parameter T = 2 sigma^2 as if it were already the Gaussian
scale sigma (the factor 2.355 / 1.17741 = 2.000 makes the mistake
visible; the function's own MKDEBUG comment doubted it). The correct
area-to-length conversion needs a square root:
FWHM = 2.35482 sigma = 2.35482 sqrt(T / 2)
This function builds the fwhm_PSF column (extract_info.py, from
NGMIX_T_PSFo_NOSHEAR) that serves as the size_PSF_col regressor in the
scale-dependent PSF-leakage fit (run_object.py / calibration.py). The
bug is a monotonic but nonlinear distortion of the PSF-size axis, so
any alpha(PSF size) trend and per-bin leakage coefficients computed
with it are biased; spatially-constant leakage is unaffected. Results
produced with the old function should be regenerated.
Both T_to_fwhm and sigma_to_fwhm now come from cs_util.size (the
shared, tested size-conversion web; requires cs_util with the size
module, branch feat/size-conversions) and are re-exported from
sp_validation.galaxy, so star-import consumers (extract_info.py,
main_set_up.ipynb's size_to_fwhm dispatch) keep working unchanged.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The package-level imports in __init__.py are commented out, so CI's 'import sp_validation' smoke test never touches galaxy: a cs_util without cs_util.size passed CI while leaving galaxy un-importable on a fresh install. The new test imports galaxy directly and pins the dimensionally correct conversion (T_to_fwhm(2) = 2.35482, where the old linear form gave 4.0). cs_util.size ships in cs_util 0.2.1 (CosmoStat/cs_util#65); the pin guarantees a fresh install resolves a size-bearing cs_util. NOTE: resolution additionally requires shear_psf_leakage@develop to relax its poetry cap cs-util ^0.1.0 (< 0.2.0) — until then installs of this branch conflict; see PR body. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This was referenced Jun 11, 2026
The 6.0.3 release on PyPI ships dist-info only — no python module — so 'import sphinxawesome_theme' fails after a clean-looking install and the docs build dies with an ExtensionError. Pin !=6.0.3 so resolution falls back to a working release (and heals itself when 6.0.4 lands). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
galaxy.py's localT_to_fwhmwas dimensionally wrong:T / 1.17741 * 2.355treats the areaT = 2σ²as if it wereσ. The correct conversion carries a square root:FWHM = 2.35482 √(T/2). The two coincide only near σ = 0.5 (T = 0.5 → 1.0008 vs 1.0) and diverge sharply elsewhere (T = 2 → old 4.0 vs correct 2.355) — a monotonic but nonlinear distortion of the PSF-leakage size axis (per-bin α(size) trends). Results produced through the old function should be regenerated.The local converters are deleted and re-exported from
cs_util.size(CosmoStat/cs_util#65, released as cs_util 0.2.1) — the shared single source of truth for the T/σ/r50/FWHM web, also adopted on the ShapePipe side (CosmoStat/shapepipe#743). The pin moves tocs_util>=0.2.1so a fresh install resolves a size-bearing cs_util (PyPI 0.2 predates the module).A new
tests/test_galaxy.pycloses the hole that let this hide: the package-level imports in__init__.pyare commented out, so CI'simport sp_validationsmoke test never touchedgalaxy— a brokencs_util.sizeimport would have passed CI. The test importsgalaxydirectly and pinsT_to_fwhm(2.0) = 2.35482.Note:
sigma_to_fwhmloses its unusedpixel_sizekwarg (never passed anywhere in src/ or notebooks; it was only ever used single-argument via thesize_to_fwhmdispatch table).Merge discipline — do not merge yet
shear_psf_leakage@develop's poetry capcs-util = "^0.1.0"(→<0.2.0) conflicts with thecs_util>=0.2.1pin, so dependency resolution of this branch currently fails. Merge after CosmoStat/shear_psf_leakage relaxes the cap ondevelop(one line,cs-util = ">=0.1.0,<0.3"; its cs_util usage —args,calc,cat,cosmo,logging,plots— is signature-compatible with 0.2.x). With that cap relaxed, the full install + test suite passes from scratch: verified here with a fresh uv env (override simulating the relaxation) — cs_util 0.2.1 resolved from PyPI,galaxyimports, 35 passed / 6 deselected (-m 'not slow').— Claude on behalf of Cail