Repository navigation
fix(psf): deterministic star train/validation split; align tile STAR_THRESH with the 80% convention - #873
Closed
cailmdaley wants to merge 1 commit into
Closed
fix(psf): deterministic star train/validation split; align tile STAR_THRESH with the 80% convention#873cailmdaley wants to merge 1 commit into
cailmdaley wants to merge 1 commit into
Conversation
…THRESH with the 80% convention The 80/20 star split drew from unseeded np.random, so the PSF star sample differed between identical runs; it is now a permutation seeded from the unit's file number (SEED_FROM_POSITION philosophy). The tile PSF-interp configs kept the pre-split STAR_THRESH=20 that fdc8655 raised to 22 in the validation config; align the science path on 22. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jrs8TeTCccRMC45QVGKPeY
cailmdaley
added a commit
that referenced
this pull request
Aug 29, 2026
#873 aligned the tile PSF-interp threshold with the 80/20 split convention in example/cfis, the only place those configs exist on develop. This branch carries a committed fork of that chain under workflow/config/cfis (#848 D2), so the science path the Snakemake run actually reads kept the stale 20 after the cherry-pick. Mirror it. The general hazard: every science-config change landing against example/cfis must be mirrored here until the two chains are unified. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VUNNyG8tw6Mdbvo4SVNrhp
cailmdaley
added a commit
that referenced
this pull request
Aug 30, 2026
…e store The fuse moved five of six SQLite stores onto node-local NVMe and bought 4.3x. It did not solve the I/O problem, and `read_bytes = 0` was the wrong instrument for concluding that it had -- NFS reads are RPCs and never reach the block layer, so psutil's counter cannot see them. Thread-state sampling closes the budget properly. 3,200 samples across the eight chunks of job 20799387 give 0.56 threads on CPU and 0.44 in uninterruptible I/O wait, R + D = 1.00, with 147 of 176 D-samples inside `rpc_wait_bit_killable`. Open descriptors name exactly one hot NFS file left: log_exp_headers-<tile>.sqlite, 11.3 MB, which ngmix reads once per object PER EPOCH for the WCS. Size was never the problem and is not the problem now; the operation count is, and on a network filesystem every operation is a round trip. So $SP_WCS_DIR joins $NGMIX_VIGNET_DIR: tile_local() copies the file beside the store it already builds, and config_tile_Ng_template.ini's fourth INPUT_DIR points at the copy instead of at Mh_exp on scratch. A copy and not a symlink, because a link resolves straight back to NFS. Sub-second for 11 MB, against ~44% of the elapsed time of the rule that is 99.3% of tile-side core-hours -- so ~1.8x on tile_ngmix, unmeasured until the next campaign runs it. tile_vignets (PiViVi) reads the same file and still reads it over NFS. It is 3.4 minutes of a ~1.7 hour group, so it is left alone deliberately rather than overlooked; the staged copy is already there if that changes. Orchestration only -- no science config changes, so nothing to mirror into example/cfis (cf. the config-fork hazard that left #873's STAR_THRESH on 20). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NYcxuAbZbQivC5UFtgecxH
cailmdaley
added a commit
that referenced
this pull request
Aug 30, 2026
smk-g4 landed two changes after its campaign finished and neither has ever run: the tile-level WCS sqlite staged node-local, and tile_ngmix's mem_mb cut 14000 -> 5000. Point the campaign roots at a fresh smk-g5 tree holding one tile, 186.307. That tile is the control because it ran ALONE as smk-g4 job 20799387 rather than inside the 34-tile campaign, so its baseline has no concurrency confound: 8063 s elapsed, chunk median 6813 s at 59% mean load, MaxRSS 26.6 GiB. The prediction, registered in job_head.sh before the run: load above 90%, chunk median near its own 4050 s of CPU, group near 83 min, MaxRSS unmoved (it is tile_vignets' peak, not ngmix's), and final_cat bit-identical to smk-g4's -- which #873's position-seeded split makes checkable for the first time. Fresh roots rather than a resume, because tile_local() changed and `params` is an active rerun trigger: pointing this at smk-g4 would delete the 34 finished tiles' catalogues rather than merely re-run them. clean is off -- ~54 GiB of exposure stores against 979 GiB free buys a re-measurement that re-runs only the shape chain.
Contributor
Author
|
bundling into #852 |
cailmdaley
added a commit
that referenced
this pull request
Sep 26, 2026
ShapePipe's scientific choices — detection thresholds, masking geometry, star selection, PSF model, ngmix priors and seeding, flag semantics, completeness floors — live in code and committed configs with their reasoning nowhere, or spread across PRs, papers and comments. astra.yaml gathers them: 50 decisions across eight sub-analyses, each with its rationale, the alternatives that were rejected and why, and a greppable anchor back to the code or config that implements it. universes/committed.yaml pins the option this branch selects for every one. The record is ASTRA (astra-tools; `uvx astra-tools@0.2.17 guide`), applied here at codebase level rather than to a single analysis. Conventions are stated in the file's header: anchors as `path::symbol` / `path#SECTION.KEY` and never line numbers, [HARDCODED] for a scientific value with no config exposure, [LINT] for a place where the record and the code — or the code and itself — disagree, [PENDING #NNN] for state not yet on develop. Authoring it surfaced nine such lints, two of which #873 fixes, and mapped ten places where the published Guinot+22 / Farrens+22 descriptions have drifted from the code since publication; 16 decisions carry verbatim paper quotes as prior insights. CLAUDE.md gains the standing instruction: a scientific change is not finished until the record is, amended in the same PR. The membership test is whether a different defensible choice would change which objects enter the shear catalogue, or the numbers attached to them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y2muA2sRojbxRNxU2SKQeP
cailmdaley
added a commit
that referenced
this pull request
Sep 26, 2026
ShapePipe's scientific choices — detection thresholds, masking geometry, star selection, PSF model, ngmix priors and seeding, flag semantics, completeness floors — live in code and committed configs with their reasoning nowhere, or spread across PRs, papers and comments. astra.yaml gathers them: 50 decisions across eight sub-analyses, each with its rationale, the alternatives that were rejected and why, and a greppable anchor back to the code or config that implements it. universes/committed.yaml pins the option this branch selects for every one. The record is ASTRA (astra-tools; `uvx astra-tools@0.2.17 guide`), applied here at codebase level rather than to a single analysis. Conventions are stated in the file's header: anchors as `path::symbol` / `path#SECTION.KEY` and never line numbers, [HARDCODED] for a scientific value with no config exposure, [LINT] for a place where the record and the code — or the code and itself — disagree, [PENDING #NNN] for state not yet on develop. Authoring it surfaced nine such lints, two of which #873 fixes, and mapped ten places where the published Guinot+22 / Farrens+22 descriptions have drifted from the code since publication; 16 decisions carry verbatim paper quotes as prior insights. CLAUDE.md gains the standing instruction: a scientific change is not finished until the record is, amended in the same PR. The membership test is whether a different defensible choice would change which objects enter the shear catalogue, or the numbers attached to them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y2muA2sRojbxRNxU2SKQeP
cailmdaley
added a commit
that referenced
this pull request
Sep 28, 2026
…he code by @sc tags (#875) * docs(astra): record the pipeline's scientific decisions in astra.yaml ShapePipe's scientific choices — detection thresholds, masking geometry, star selection, PSF model, ngmix priors and seeding, flag semantics, completeness floors — live in code and committed configs with their reasoning nowhere, or spread across PRs, papers and comments. astra.yaml gathers them: 50 decisions across eight sub-analyses, each with its rationale, the alternatives that were rejected and why, and a greppable anchor back to the code or config that implements it. universes/committed.yaml pins the option this branch selects for every one. The record is ASTRA (astra-tools; `uvx astra-tools@0.2.17 guide`), applied here at codebase level rather than to a single analysis. Conventions are stated in the file's header: anchors as `path::symbol` / `path#SECTION.KEY` and never line numbers, [HARDCODED] for a scientific value with no config exposure, [LINT] for a place where the record and the code — or the code and itself — disagree, [PENDING #NNN] for state not yet on develop. Authoring it surfaced nine such lints, two of which #873 fixes, and mapped ten places where the published Guinot+22 / Farrens+22 descriptions have drifted from the code since publication; 16 decisions carry verbatim paper quotes as prior insights. CLAUDE.md gains the standing instruction: a scientific change is not finished until the record is, amended in the same PR. The membership test is whether a different defensible choice would change which objects enter the shear catalogue, or the numbers attached to them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y2muA2sRojbxRNxU2SKQeP * test(astra): validate decision anchors and universe pins * test(astra): resolve Snakemake rule anchors; JSON report mode Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * docs(astra): rewrite the decision record against develop - masking: describe healsparse queries (mask_query MASK_EXT on exposures, make_cat MASK_<band> on tiles) and the instrument flag image as the only pixel mask, replacing the deleted in-house mask generation - detection: tiles follow the MegaPipe (Gwyn) SExtractor parameters (#896); option ids no longer encode the retired values - shape_measurement: import defect_fill, blend_handling and epoch_masked_fraction_cut from the digital twin with their literature insights; defaults are what the committed code selects - prune to the membership test: drop psf_diagnostics, survey_geometry, the workflow-policy decisions and the root findings; split compound decisions; reserve excluded for considered-and-rejected; strip chronology - re-point anchors to the current configs; the anchor test passes Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014bvNTrAmZxcfb1ee83ApPK * docs(claude): point the scientific-decisions section at the anchor test Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014bvNTrAmZxcfb1ee83ApPK * docs(astra): correct seven rationale claims against the code - epoch_provenance: names keep their trailing p; EXP_PREFIX is a no-op [LINT] - fit_initialisation: only the PSF guesser takes the catalogue flux; an exception in Ngmix.process drops the object with no row - star_galaxy_classification: thresholds come from SM_STAR_THRESH / SM_GAL_THRESH, which the committed config does not set - psf_train_validation_split: seeded from the unit's file number - stamp_positioning: an out-of-image stamp centre raises - object_position_columns: tile stamps are cut at XWIN_IMAGE (COORD=PIX) - mark the PSFEx built-in SAMPLE_* behaviour and the 33-px trim unverified - record the galaxy prior reused for PSF fits and the silent epoch drops before the 1/3 cut; carry stale completeness, exposure.smk, _mode and pixel-scale comments as [LINT] Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014bvNTrAmZxcfb1ee83ApPK * docs(astra): rephrase unverifiable claims; add Guinot+22 insights; sentinels and completeness precision Follow-up to the correction pass: claims the repo cannot check are stated as what the config assumes; five Guinot+22 prior insights with page-verified quotes replace bare paper citations; failure_sentinels says an NGMIX_N_EPOCH > 0 cut removes failed objects; per_unit_completeness counts only rules that run shapepipe_run. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(contracts): validate @sc contracts against the decision record tests/helpers/contracts.py parses @sc/@cc contracts with sc-list's line grammar from Python docstrings, Snakemake comment blocks and CONTRACTS files under src/, workflow/ and scripts/. test_contracts.py fails on malformed lines, missing or duplicate ids, tag lines hidden in .py comments (invisible to sc-list), and decision: metas naming no decision in astra.yaml. A report-only test prints decisions no contract cites and contracts off the record's anchored symbols. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014bvNTrAmZxcfb1ee83ApPK * docs(sc): first scientific contracts at the record's anchors Sixteen @sc contracts in the docstrings of declarations astra.yaml anchors, each citing its decision: star-selection mode and split seeding, SExtractor weight wiring and epoch membership bounds, CCD splitting and WCS source, epoch provenance, stamp rounding, the PSF acceptance gate, catalogue classification scope, never-fit sentinels, mask-column and mask-flag semantics, and per-unit completeness. Where the record carries a [LINT] at the declaration, the contract states the intended behaviour and names the lint. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014bvNTrAmZxcfb1ee83ApPK * docs(astra): record the header saturation level; note PSF_ACCURACY - detection.saturation_level: SATUR_KEY SATURATE with no SATUR_LEVEL sets the FLAGS saturation bit that star selection rejects on; header presence on exposures and tiles is unverified here - psf_model_complexity: PSF_ACCURACY 0.01 with its anchor Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014bvNTrAmZxcfb1ee83ApPK * test(contracts): utilities import boundary src/shapepipe/utilities/CONTRACTS declares utilities-do-not-import-modules (forbid: shapepipe.utilities.* -> shapepipe.modules.*). test_contracts.py reads the forbid rule from that file and resolves every import under src/shapepipe/utilities with ast, relative imports included. It holds today; loom's check-imports agrees. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014bvNTrAmZxcfb1ee83ApPK * docs(astra): central_defect_veto; 4-fold symmetrisation for defect_fill - shape_measurement.central_defect_veto: default disabled (committed develop has no veto); radius_10px, implemented on feat/symmetrized-defect-fill, is the smallest radius with |m| < 1% - defect_fill: the recommended option is the 4-fold OR (symmetrized_4fold_noise); a single rot90 leaves coherent c2 of -0.006 to -0.012 for off-centre columns, 4-fold gives |c| < 2e-4 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014bvNTrAmZxcfb1ee83ApPK * test(contracts): contracts on config keys via governs: Resolve semicolon-separated, directory-relative governed refs with the shared ASTRA anchor resolver. Reject empty refs and repeated metadata keys, and include config contracts in the report-only coverage check. Co-Authored-By: GPT-6 Astra <noreply@openai.com> * test(astra): assert config values in the anchor grammar Check active config values and static Python literals independently of anchor resolution. Normalize numeric and boolean spellings while retaining list shape and SETools comparison operators. Document the grammar and exercise it on PSF_NOISE. Co-Authored-By: GPT-6 Astra <noreply@openai.com> * docs(sc): contracts for the config-anchored decisions Keep canonical contracts beside the CFIS configs, with an inherited workflow contract beside the PSF selector. Cover the 17 previously uncovered decisions and the exposure pixel-scale/diagnostic coupling without changing scientific settings; retain the known config inconsistencies explicitly. Match contract coverage against anchor locators through the shared parser, including the value-assertion grammar added concurrently. Keep the tile-overlap config projection visible as a report-only anchor gap. Co-Authored-By: GPT-6 Astra <noreply@openai.com> * docs(astra): assert committed values on anchors Assert 130 values across 29 decisions without changing defaults or option ids. Cover coupled stamp sizes, detection, star cuts, PSF settings, and literal ngmix priors/metacal settings. Clarify that the CCD's 2048-index span is inclusive, whereas the committed cut excludes both endpoints. Co-Authored-By: GPT-6 Astra <noreply@openai.com> * docs(astra): defect fill, central veto and masked-fraction cut follow the measured design - defect_fill: noise on the unsymmetrized defect set stays default; interpolate (feat/defect-interpolation) describes the bounded-run fill with quarter-turn weight orbit; four-fold symmetrization is excluded on its measured m and c1; the model option is dropped - central_defect_veto: fixed radii (10 px noise, 7 px interpolated) on the defect mask only; size-scaled radius excluded; calibration and known limits stated; default stays disabled - epoch_masked_fraction_cut: the branch counts the raw defect set, with EPOCH_MASKED_FRACTION_CUT configurable; default stays 1/3 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014bvNTrAmZxcfb1ee83ApPK * test(astra): assert gate keys and absent keys; ambiguous subscript bindings; INI booleans per getboolean - Assert the booleans that make asserted values live: WEIGHT_IMAGE (weight map), MAKE_POST_PROCESS (CCD_SIZE), vignetmaker MASKING (STAMP_SIZE). - `= absent` asserts a config key has no active line; the record uses it for MASK_EXT in every star-selection mask block and SATUR_LEVEL in both .sex files. New contract psf-stars-vetoed-on-instrument-flags-only. - NAME[...] = / NAME.attr = in the binding's scope makes a value read of NAME ambiguous. - INI booleans follow ConfigParser.getboolean; Y/N only for .sex/.psfex. - A real-record mutation test covers each drift that previously passed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014bvNTrAmZxcfb1ee83ApPK * docs(astra): PSFEx compiled SAMPLE_* defaults, verified by psfex -dd psfex_candidate_vetting and psfex-vetting-is-not-fully-disabled state the PSFEx 3.21.1 compiled defaults (psfex -dd in the develop-runtime image) and assert each omitted SAMPLE_* key absent from default.psfex, so pinning one is visible to the record. Which cuts act with SAMPLE_AUTOSELECT N is marked as from the PSFEx source, not re-read here. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014bvNTrAmZxcfb1ee83ApPK * docs(astra): PSFEx vetting points at #919; compiled values are not assertable Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014bvNTrAmZxcfb1ee83ApPK * docs(astra): retire lints resolved by #907, #909, #918 The merged fixes made six record lints false and broke four value assertions and one contract ref. Re-anchor the MCCD exposure chain to what it now reads (split image/weight/flag, mask_query before setools), the setools FWHM plot to 0.187, and CFIS EXP_PREFIX to a location-only ref (blank). Pin the MCCD completeness counts the rationale now names. Drop the resolved lints from the record, the @sc blocks and CONTRACTS; the IMAFLAGS_ISO export (#912), ngmix's 0.186 pixel scale against star selection's 0.187, and the ngmix noisefill/noise-window doc lints remain. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs(astra): tighten the record's prose Rationales no longer restate values their Anchor sentence asserts, and literature comparisons that a cited insight already carries become pointers. The header keeps the anchor grammar and markers; the value grammar's fine print moves to tests/helpers/astra_record.py, beside the parser that enforces it. Anchors, ids, options and evidence are unchanged (206 tests, astra validate). Corrected while tightening: the fit_initialisation default label (the galaxy guess takes its flux from a PSF-flux fit), and the blend_handling `none` description, which now claims only what Jarvis et al. 2016 support. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AkrwnKzU1wfUaHUPt3yYk3 * test: link science guardrails to ASTRA decisions * feat(decisions): add site-tag checker and Values resolver * fix(decisions): tighten tag discovery and section scopes * fix(decisions): resolve multiline INI values within sites * fix(decisions): scope Python selectors to tagged declarations * fix(decisions): ignore values outside Python site grammar * migrate record anchors to site tags * refactor: dissolve config contract sidecars into site tags * docs: document decision tags and Values checks * fix(decisions): tighten absence and duplicate checks * fix(config): narrow decision tag placements * fix(science): refine local decision-site contracts * docs: simplify and wrap Values assertions * docs(astra): record tile/exposure header evidence and the pixel-scale history A sampled tile and exposure carry SATURATE, so the SExtractor fallback level never applies; the exposure's FSCALE matches its PHOTZP against the tiles' zero-point 30. The fit_priors lint now names what #858 settled (the WCS is the source of truth) and what still overrides it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test: run committed SExtractor configs with real tools * docs(astra): correct pipeline decision rationale * style(config): remove redundant migration blank lines * fix(decisions): match pipeline config semantics --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: GPT-6 Astra <noreply@openai.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.
Two small fixes to how the PSF star sample is built, found while auditing the scientific decisions embedded in the pipeline configs.
1. Seed the train/validation split.
SETools._make_rand_splitdrew the 80/20 star split from unseedednp.random, so the star sample entering the PSF model — and everything downstream of it — differed between identical runs. The split is now a permutation seeded from the unit's file number: deterministic per CCD, independent of processing order, same philosophy as ngmix'sSEED_FROM_POSITION. (This changes the realized split once — equivalent to one more random draw, now frozen.)2. STAR_THRESH 20 → 22 in the tile PSF-interp configs. fdc8655 (2020) deliberately raised the per-CCD acceptance threshold to 22 to account for the 80% training split, but only in
config_exp_psfex.ini(the validation pass). The tile configs that feed galaxies kept the stale 20, so the science path admits CCDs under the pre-split standard while validation gates at 22. We think the right fix is to align on 22.Both change outputs slightly (a new frozen split; CCDs with 20–21 accepted stars now excluded, matching validation).
🤖 Generated with Claude Code
https://claude.ai/code/session_01Jrs8TeTCccRMC45QVGKPeY