diagnostics: log inputs, computed max_order, and warn when write_new_parameters produces no output#177
Merged
Merged
Conversation
…parameters produces no output write_new_parameters currently derives its output count from a max_order heuristic on new_coeffs (any-non-zero-non-nan planes). When new_coeffs is entirely 0.0 or NaN, max_order = -1, the write loop becomes range(0), and NO files are written — with no exception raised and no log line emitted. Downstream consumers that assume newcoeff*.fits exist can be caught out badly by this: POSSUM_Polarimetry_Pipeline's fracpol3d_Irescale step, for example, will destroy its input coeff*.fits files in an ill-fated delete-then-rename block and only crash much later with a FileNotFoundError, far from the actual root cause. That has now been fixed defensively on the pipeline side, but the underlying silent no-op deserves visibility here too. Changes (all logging, no behaviour change on the happy path): * Add a module-level logger and a small _summarize_coeffs helper for compact array descriptors (shape, dtype, nan/zero/finite-nonzero counts). * rescale_I_model_3D logs input coeffs and output new_coeffs summaries at INFO. * write_new_parameters logs input summary, computed max_order, planned file count, and each file it writes at INFO; total file count at end. * When max_order < 0, write_new_parameters now emits both logger.warning and warnings.warn(UserWarning) explaining the condition, and returns explicitly. Preserves the historical 'no files written' behaviour but makes the reason loudly traceable. This surfaces the root cause of the tile 6614 (RMtools 1.4.11) incident observed in the CIRADA POSSUM pipeline, without changing what the function actually writes on any input where it did write something before.
for more information, see https://pre-commit.ci
Sebokolodi
added a commit
that referenced
this pull request
Jul 17, 2026
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
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.
Motivation
write_new_parameterscurrently derives the number of output files from amax_orderheuristic onnew_coeffs:When
new_coeffsis entirely0.0or NaN across every plane,max_orderresolves to-1, the write loop becomesrange(0), and no files are written, no exception is raised, no log line is emitted.Downstream consumers that assume
newcoeff*.fitsexist can be hurt badly by this silent path. In the CIRADA POSSUM pipeline (POSSUM_Polarimetry_Pipeline/pipeline/modules/fracpol3d_Irescale.py), a subsequent delete-then-rename block would destroy the inputcoeff*.fitsfiles and only crash much later with aFileNotFoundErroratpf.getdata(coeff0.fits), far from the root cause. That has now been fixed defensively on the pipeline side (POSSUM_Polarimetry_Pipeline#33), but the underlying silent no-op inside RMtools deserves visibility here too.Observed: tile 6614, 943MHz, RMtools 1.4.11 in production — the writer produced zero output files with no warning; only became visible when a downstream getdata crashed.
Changes (all diagnostics — no behaviour change on the happy path)
loggerand a small_summarize_coeffshelper that returns a compact one-line descriptor of a coefficient array (shape,dtype, total elements, nan count, zero count, finite-nonzero count).rescale_I_model_3D: log input coeffs summary at the start and outputnew_coeffssummary at the end (INFO).write_new_parameters: log input summary and the computedmax_orderup front; log every file written with its full path; log a final total-files-written count at the end (INFO).write_new_parameters: whenmax_order < 0, emit bothlogger.warningandwarnings.warn(UserWarning)with a clear message identifying the condition ("every plane of new_coeffs is 0.0 or NaN"), thenreturnexplicitly. The historical behaviour of writing no files is preserved; the reason is now visible even to callers that haven't configured logging.Non-goals
max_orderheuristic itself. If it turns out the heuristic is wrong (e.g. failing on valid low-order fits under some conditions), that's a separate fix.warnings.warn(UserWarning)is the loudest visibility change that doesn't break existing callers that treat the empty-output case as valid.Test plan
new_coeffsfilled with NaN, callwrite_new_parameters— expect aUserWarningand a logger warning, no files written (as before).new_coeffs— expect INFO log lines showing the summary,max_order, and per-file paths, plus the normal FITS output files.