Following up on dev->smdc port - #56
Merged
Merged
Conversation
… check is reachable everywhere db_register_socsim_files.py checked INPUTBUCKET (a5108ec) below eleven imports — boto3, healpy, astropy, psycopg2 through rapid_db — so on any interpreter without those packages the script died at 'import boto3' with exit 1 and nothing on stdout, and the two fail-closed regression tests, which run the script as a subprocess, failed outside the container. Their docstring claimed the packages were part of the test extra; that extra is pytest only. The stub tier has been red at the base of this branch for exactly these two tests. The check now runs first, before any third-party import: a misconfigured run fails in milliseconds with the reason on stdout, and the test holds in every environment. Verified with the astroconda interpreter (2 passed) and with the bare system python3, which has none of the packages (exit 64, message on stdout). Test docstring corrected; the pyproject import-mode comment now names database/sims/test/ alongside database/modules/utils/test/ as the test directories that carry an __init__.py under an unpackaged parent, which a5108ec made true without saying so. Stub tier: 2677 tests across 101 modules, PASS (was FAIL at the base). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…the kept-file count honestly Dev 399d4ed (carried by 287b592) closed dm immediately after reading dm.meta.wcs, nine lines before gwcs_obj.pixel_to_world used the WCS object it came from. The close now follows the sky computation, after the last use, and original_dm — the file-backed model, never closed on either branch across a whole-bucket loop — is closed with it. The counter dev added and labelled 'socsims skipped' is incremented on the append to input_asdf_files, so it counts the files KEPT; printed now as 'to process'. Port-followup item A1; see the reconcile audit. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…, not an inlined copy _evaluate_catalogs_at_mjd on this branch carried dev f0630ba verbatim: the sinusoid and Gaussian formulae inlined and the injectionLightCurveModels import dropped, which duplicated the photometric model and made GaussianLightCurve's time_bounds branch unreachable from the injector. It now calls SinusoidalLightCurve and GaussianLightCurve with arrays, once per light-curve type per catalogue — the same speed-up, one source of truth. Also: an empty catalogue is skipped before the WCS call, an unknown type raises before any evaluation rather than after both blocks have run, and the coordinate arrays are explicit float64. New stub-tier test pipeline/test/test_l2_injection_evaluation.py pins the vectorised result against the per-source loop through the same model functions on a 400-source synthetic catalogue (all five arrays equal), the empty-catalogue shape, the unknown-type error, and that the sinusoid is not inlined. Heavy imports are stubbed only when not installed. Port-followup item A2. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Dev 2c89139 (carried by e6924cc) set per-filter zero points for F213 and F087 only and left F184, F158, F129, F062, F106 and F146 at a 0.0 placeholder written into the header with no diagnostic. A zero AB zero point is not a missing value but a wrong one that downstream flux calibration will use. The placeholder is kept, so behaviour is unchanged, but it now prints a warning naming the filter. Supplying the six values (or making the missing case an error) is an open team item. Port-followup item A3. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…metry Quantities sip_distortion_degree = 5 is hoisted to module scope (dev 3f22b1b, e09845b, c58ef15 — three commits whose net effect on the value is none; only the end state is carried). The derived ZPTMAG path (dev 8428314) did float(conv) and float(pixarea) directly; roman_datamodels carries conversion_megajanskys and pixel_area as astropy Quantities (MJy/sr, sr) in recent versions, and float() on a dimensioned Quantity raises, which would have sent exactly the files this path was written for to the nominal table. The bare values are now taken first, in the units the formula assumes. Port-followup item A4. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…arn on crossconv without segmentation [sfft] in cdf/science/pipeline.toml carried its six keys with no comment; the rationale lived only in the master .ini, which is scheduled for deletion. The comments now state dev 7d85269's finding (crossconv_flag = true requires sfft_use_segmentation = true: SFFT's decorrelation sigmas come from the mask's background pixels and it raises only when that set is empty, so without segmentation ~20% of depth is lost silently), the per-data-set mask values dev e3c1595 made explicit, and that segmentation is independent of cross-convolution. Comments only: parsed content and the release digest are unchanged. _sfft_argv logs a warning for the crossconv-on / segmentation-off combination. Loud rather than fatal; whether it should refuse is an open team item. Port-followup items A5 and A6 (dev 7d85269). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…talogue timings, psfcat reads, ZOGY caveats, generator order Port-followup items A7-A10, each small and none changing a product: A7 referenceImageSubs.py — elapsed-time prints around generateSExtractorReferenceImageCatalog and generatePhotUtilsReferenceImageCatalog in the existing '=====>' style (dev f07e1ce); the 8/13/26 rimtimsim test showed reference PhotUtils catalogue generation dominating that pipeline. A8 stages/science.py — the five DAOStarFinder tuning parameters are read through context.science_value(psfcat_diffimage, ...) so a missing key is a named ConfigError, matching 349acf1's rule for the SFFT keys; both call sites now say the one fact that makes the change reviewable: only min_separation (1.0 vs the 0.0 default) alters behaviour, and why. A9 stages/science.py run_zogy — the release toggle is coerced with sfftCommandSubs.config_bool rather than bool(); the two caveats carried from dev are recorded beside the call (maps NaN-filled with the clipped scatter in prepare_zogy_inputs; scalefacref deliberately not applied on the gain-matched map path). A10 generateInjectionCatalogsForSims.py — docstring no longer carries the literal 'injection_catalogs_subdir' text that produced dev's URL bug; the field union is sorted so catalogues are generated in a deterministic order. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The two converters, the injector and the rimtimsim registrar carried the 2026-07-09 socsim and 2026-04-01 rimtimsim bucket names as code defaults while the .ini's injection_catalogs_subdir had already moved to the 2026-08-11 catalogues — a mixed state a run would not have noticed. Bucket names are now REQUIRED from the environment (INPUTBUCKET / OUTPUTBUCKET; the injector's input stays the fixed public SOC-simulation release), in the fail-closed shape database/sims/db_register_socsim_files.py already uses (a5108ec): a forgotten variable on an unattended run exits 64 with the reason on stdout instead of silently converting or admitting some other dataset. The current set names (dev 3f22b1b, c3cd546, d3c2317, f2ce484) are recorded in comments for the run manifest, not as defaults. Where this is heading (team decision recorded with port-followup item A11): per-simulation release content will carry the science-affecting values (gain, read noise, zero points, injection ranges, catalogue subdirectory) in per-release toml files, per the toml's own placement rule; bucket names cannot alter a science product and belong with the data — the sealed generation's _manifest.json that the staging procedure already writes, which these scripts should eventually read instead of the environment. The required variables are the bridge until then. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…guard returns None; ordering docstring — digest re-ratified One ratified edit to the frozen file (rule 17), authorised by Emily for this branch on the strength of the 2026-09-09 diff-level audit, absorbing three fixes to existing methods and adding no capability: * _register_numpy_adapters (d4b7baa) registered numpy.float16/32/64 as AsIs(repr(float(value))), so NaN and Infinity became the bare SQL tokens nan / inf — 'column nan does not exist' on every row carrying one (skymean, zptmag, WCS terms). numpy.float64 had adapted correctly via psycopg2's subclass fallback until that exact-type registration overrode it. Reproduced: adapt(np.float64('nan')) -> b'nan' after importing this module. Floats now go through psycopg2's own Float adapter: 'NaN'::float / 'Infinity'::float, plain text otherwise. Contract test gains the non-finite assertions (4 pass against the real driver). * execute_sql_queries' empty-list guard (6a2cce9) returned [] with exit_code 64; none of the 15 call sites checks exit_code, so a caller error read as 'found nothing'. It returns None, like the query-failure path. The duplicated inner records = [] is removed (dev 0cad34b moved that line; 6a2cce9 added a second copy). * get_overlapping_l2files' docstring said 'ordered by distance from tile center'; the query has ordered by mjdobs then distance since 87156b5. RAPID_DB_BRANCH_POINT_SHA256 -> 8d7dc0fc…, with the ratification note listing exactly the above and the previous value. The second digest test's branch-diff form will report 'modified on this branch' until this lands on smdc; that is the deliberate friction, accepted for this one merge. Port-followup items B12, B16, B23. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
… name the date when a date-scoped phase gathers nothing live_w9_ramp.py defaulted W9_PROC_DATE to the OBSERVATION window's start date (b2335bb). pipeline/operator/gathering.processing_date_for records that rule as refuted live: the post-DB chain enumerates against diffimages.created — the day the pipeline did the work — so the old default matched nothing, gathered zero units and exited 0 with 'nothing ready in the window'. A ramp step that submitted nothing looked like a passing ramp step. The default is now processing_date_for(None), the same function the operator daemon uses; W9_PROC_DATE still overrides it. A zero-unit gather for a date-scoped phase now prints a WARNING naming the processing date and the override. Exit stays 0: crossmatch legitimately gathers nothing until catalog-load completes, and that case must not fail the ramp. Port-followup item B13. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…e overlapfields comments e578ecc added traceback.print_exc() to the socsim registrar's per-file handler so a failure names its call site; print_exc() writes to stderr, and the per-thread log fh is the artifact an operator reads after a ProcessPoolExecutor run. The formatted traceback is written to fh as well. The overlapfields comments in all three registrars said 'rapid_systems migration 100 gives l2files a column'; the migrations were renumbered (4c5d2d1) after the collision with main's 100 and the column is 101. e633319 fixed the code references and left these three sentences. Port-followup items B18, B19. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ce the row records
Both sweeps called catalog_db.delete_superseded_rows with merges.sid as the
join column against an IMAGE identity — diffimages.pid for the merge sweep,
l2files.rid for the source sweep — and the source sweep deleted from
merges_<field>, the merge sweep's own table. sid is a sources sequence
value; comparing it with an image id deletes whichever rows happen to
collide. Verified on PostgreSQL 18.6 with colliding ids (2026-09-15): the
merge sweep deleted a row under a PINNED image (vbest 2) and a current row
and kept the demoted one; the l2files form deleted every row. This is the
defect the 2026-09-12 project ruling unregistered both sweeps from the
operator daemon for.
Two purpose-built deletes replace the generic call at the two sites:
delete_superseded_merge_rows(cursor, merges_<field>)
DELETE ... WHERE NOT EXISTS (SELECT 1 FROM sources s JOIN diffimages d
ON d.pid = s.pid WHERE s.sid = m.sid AND d.vbest IN (1, 2))
delete_superseded_source_rows(cursor, field)
DELETE FROM sources s WHERE s.field = %s AND NOT EXISTS (SELECT 1 FROM
diffimages d WHERE d.pid = s.pid AND d.vbest IN (1, 2))
— the sources children are per (date, SCA) and the unit is per field,
so the DELETE targets the inheritance parent; EXPLAIN shows PostgreSQL
routing it to every child. This is dev's pruneNotBestSources intent
(0d67e4e), which nothing on smdc performed until now.
On the same check both corrected deletes removed exactly the demoted row,
in either order. vbest IN (1, 2) is 'current' because migration 006
defines 2 as a LOCKED best the update functions refuse to demote; dev's
scripts express the same rule as 'delete where vbest = 0'. The post-write
verification re-runs each corrected predicate as a count; the one-hop
verifier is removed. The generic delete_superseded_rows stays for its
existing tests. Three new stub tests pin the two-hop shape and refuse the
one-hop one.
The operator daemon's registry still leaves both sweeps out; re-registering
them is the team's decision, and the three comments describing the defect
now say it is corrected. Port-followup item B14.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ad images, and prune objects with no current detection compute_statistics aggregated the raw merges-sources-diffimages join. Bulk COPY cannot enforce per-row uniqueness, so two sources close enough to hash to one aid in one difference image both land (dev 3a4cbfc names the cause), and both were averaged and counted. The aggregate now reads a DISTINCT ON (aid, pid, isdiffpos) derived table ordered by qfit ASC, sid ASC: one point per object per image per sign, the best fit kept, positive and negative detections of one object never collapsed. Dev 39ac549 added exactly this and dev's own 0feead1 dropped it when rewriting over enumerated children; smdc is better than dev's tip here and says so. Two more terms: — registration's good/bad QA flag on the difference image (006-core-tables; pipeline/registration/products.py writes 1 unless release content says otherwise), dev 00afb0d's term applied to the image the source came from; and, in the same transaction, astroobjects_<field> rows with no statistics row are deleted (dev's step 5): with the currency join that population grows on every demotion, and such an object is one nothing can measure. Its merge rows are the merge sweep's to remove by the same rule. Verified on PostgreSQL 18.6 (2026-09-15) before the tests were written: a duplicate pair collapses to the lower-qfit row (nsources 2, meanflux 107.5, not 3), a positive/negative pair on one image counts as two, an object whose only detection is on a demoted image gets no row and is pruned. Effect record gains orphan_objects_removed. Three new stub tests pin the shape. Port-followup item B15. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The round-trip test exempts the whole [science] section as 'authored' because it was relocated from SSM rather than extracted from the .ini — true of min_images_to_coadd, not of refimage_psf_filename, which 36e4995 added as the toml twin of [JOB_PARAMS] refimage_psf_filename (forcedPhotometryForField.py still reads the .ini copy). The two homes were bound by nothing; b2335bb's message records that they agreed by luck. A drift changes which PSF every difference is made with. One targeted assertion pins them, as the [ref_image] twin already is. Whether [ref_image] should return to dev's north-up template is the open item recorded on the plan; the value is not changed here. Port-followup item B17. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
6fd9335 made record_l2file_admission refuse to guess a checksum's algorithm after the 2026-09-09 D6 socsim run mislabelled 444 md5 digests as sha256. Every layer under it kept the same default: the repository's add_manifest_entry and admit_l2file, the identity helpers normalized_checksum / l2file_payload / l2file_identity, and the bridge's own enumerate_source. The repository is reachable without the bridge (backfill_g0001_admission.py calls it directly), so a caller there would have met the identical silent mislabel. - checksum_algorithm is now a required keyword on both repository methods and on l2file_identity; normalized_checksum and l2file_payload take it as a required positional; enumerate_source requires algorithm. - Every production caller already passed it explicitly. The 35 contract test call sites that leaned on the default now say "sha256", which is what their 64-hex fixtures are. - The AST sweep in test_admission_bridge_checksum_algorithm now covers admit_l2file, add_manifest_entry and enumerate_source as well, and the signature check is parametrised over all seven functions. Not changed: the DEFAULT 'sha256' on the two checksum_algorithm columns in rapid_systems migration 051. The repository always writes the column, so the default is never exercised; dropping it is a migration and a team decision. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…truncating casts Four findings from the 2026-09-09 audit of db_register_sciimg_psfs.py and pipeline/repositories/psfs.py, all still present on smdc: - --filter and --fid were never checked against each other. The filename token was matched to --filter, but nothing confirmed --fid names that band in Filters, so `--filter f146 --fid 7` would have filed eighteen F146 PSFs under Z087. PsfRepository.filter_name reads the Filters row and the script compares the three-digit wavelength code (Filters says W146, the files say f146) before the first write; a mismatch or an unknown fid exits 64 with nothing registered. - generation_is_sealed answered False to EVERY ClientError, so a missing read grant read as "abandoned or still being staged". Only 404/NoSuchKey/NotFound means unsealed now (the classification pipeline/runtime/boundaries.py uses); anything else is re-raised and main() reports it as "unknown, not no" with exit 66. - The mkdtemp download directory was never removed; it is a TemporaryDirectory released once the checksums are in hand. - The repository cast filename and checksum to character varying(255) and (32). An explicit bounded cast truncates silently in PostgreSQL; the stored functions take unbounded varchar and psfs.filename is text since migration 023, so the casts are unbounded and an over-long value is refused by the column instead of shortened. The test module stubbed psycopg2 as a bare module, which cannot satisfy rapid_db's `import psycopg2.sql`; the script now imports its database modules and boto3 inside main(), after argument checking and the dry run, so the pure helpers and their tests import anywhere without stand-ins. New tests cover the seal classification, the filter/fid agreement, the Filters read-back and the absence of bounded casts. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The declination span was taken from the boundary alone (corners plus each edge's own extremum). When a pole lies strictly inside the image that span stops short of 90, so the cap tile and any ring above the boundary's highest declination were skipped. Measured on a 4088-px SCA centred on the pole: the cap was reported at |dec| = 89.98 and missed from 89.985 on. test_over_the_pole sat at 89.98, 0.005 deg from the failure, and asserted membership only. - `_contains` tests whether a pole lies inside the spherical quadrilateral: same side of all four edge planes AND in the image's own hemisphere. The hemisphere check is load-bearing; the same-side test alone admits the antipode, and without it a south-pole image took the north branch and reported every ring on the sky (caught by the new tests before commit). - When a pole is inside, the result is the cap plus every tile of every ring down to the boundary's far edge: whole rings, so a few tiles over near the pole and never one under. Away from the poles nothing changes: 1,500 random pointings, both insets, both hemispheres up to |dec| 89.9, are identical to the previous module, and the pinned sliver footprint is unchanged. - Tests: both caps at 89.98 / 89.985 / 89.99 / 89.999 / 90-1e-9; the sampled-subset invariant and the whole of ring 1 for pole-containing images; monotonicity across the pole leaving the inset rectangle; an image beside the pole unchanged; one pole never claims the other. Documentation, in the module and the backfill: - The 101 column comment says "EXACT"; read it as exact away from a pole and conservative over one (the comment itself lives in rapid_systems). - One column, two callers: registrars pass each file's header NAXIS, the backfill the release's naxis1/2_sciimage (l2files stores no extent). Equal for every data set so far; another detector size must be registered, not backfilled. - The full 101 -> 102 -> deploy rapid -> backfill -> 103 -> 104 (--acknowledge-coordination) -> 105 sequence, written down once, with the note that the smdc registrars need 102 and have no gate. - `field = ANY(overlapfields)` is true by construction for this writer; the falsifiable check is the backfill's preflight on the un-unioned geometry. Co-Authored-By: Claude Fable 5.1 <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
2 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.
Follow-ups after the dev→smdc reconcile
Note: these were made largely before today's meeting and can be abandoned / the branch deleted if we prefer. Adding Ben as a reviewer so he can see if any of these are useful.
Seventeen commits on top of
smdc@ 6ca495e, made against a written checklist after the diff-level comparison ofsmdc-devportwith thesmdc-devport-reconcile/socsim-20260821-rerunline. With the team moving back todev, this PR is handed over as-is for triage rather than pursued further. The plan site (Stage 3½ "Re-base") has the per-commit ledger and what was checked before each.Each commit is independent and can be taken or dropped on its own.
Deltas from
smdc-devportthat the reconcile line lackedINPUTBUCKETcheck before the heavy imports, so the fail-closed tests pass outside the containerZPTMAG = 0.0(six filters)sip_distortion_degreeconstant; Quantity guard beforefloat()[sfft]rationale comments in release content (digest unchanged); warn on crossconv without segmentationscience_value; ZOGY caveats +config_bool; generator docstring/sortedINPUTBUCKET/OUTPUTBUCKET(exit 64 when unset), no code defaultsFixes to defects found in the reconcile line
rapid_db.py, digest re-ratified: numpy floats via psycopg2Float(wasAsIs(repr)→ barenan);execute_sql_queries([])returnsNone; ordering docstringW9_PROC_DATEdefaults toprocessing_date_for; zero-unit date-scoped gather is namedsources.sid → pid → diffimages.pid; current =vbest IN (1,2); source sweep targets thesourceschildDISTINCT ON (aid, pid, isdiffpos)byqfit; explicitstatus > 0; orphan-object prune[science] refimage_psf_filenamebound to its.initwinchecksum_algorithmrequired below the bridge (repository, identity helpers,enumerate_source); AST sweep widened--fidvs Filters by wavelength code; 403 ≠ 404; temp dir released; unbounded varchar castsoverlapping_fields: report the cap when a pole is inside the image; hemisphere check in the interior testDeliberately not done
DEFAULT 'sha256'on 051's twochecksum_algorithmcolumns (rapid_systems) is untouched.rapid_db.py(test_deletion_exclusivity) fails on this branch until it merges; the digest test passes.Gate at tip: stub tier 2709 tests / 102 modules PASS; ruff 383 = baseline at 6ca495e.
🤖 Generated with Claude Code