Skip to content

StructureFunction2DStack.fit_spiral / calculate_modal_power has no per-annulus fault tolerance #39

Description

@richteague

Problem

StructureFunction2D.fit_spiral (structurefunction.py:2066) raises ValueError
when a ring has fewer than len(modes) + 1 finite azimuthal S_2 points
(line ~2101: "Not enough finite points to fit {} modes."). StructureFunction2DStack.fit_spiral
(structurefunction.py:2852) loops over self.results calling this per-ring with no
try/except, so one masked/incomplete annulus aborts the whole stack fit rather
than just that ring. calculate_modal_power (structurefunction.py:2882) inherits
the same gap, since it calls self.fit_spiral directly.

This is exactly the kind of thing real interferometric data hits: masked pixels,
partial azimuthal coverage at the inner/outer edge of the analysis annulus, etc.
Clean, full-coverage synthetic fields (the only case in test_structurefunction.py
today) never trigger it, so it went unnoticed.

Context: calculate_modal_power had no in-repo caller until now (see
a00a695's commit message on fix-modal-power-unpack, which fixed an unrelated
tuple-unpacking bug in the same method). It now has a real one -- Paper 2's
"Modal Power" analysis (StructureFunction/paper2/notebooks/run_s2_modal_power.py)
-- which is what surfaced this.

Suggested fix

Catch the per-ring failure in StructureFunction2DStack.fit_spiral (and by
extension calculate_modal_power) and fill a NaN row instead of propagating,
matching the censoring-free convention already used elsewhere in this project
(e.g. noise_mc._accumulate_null, spiral_null._SpiralNull.build_null in
StructureFunction/paper2/notebooks): a draw/ring that can't be fit contributes
NaN, not a crash.

Reference implementation

run_s2_modal_power.robust_modal_power in the StructureFunction repo
reproduces calculate_modal_power's reduction with this fix (per-ring
try/except around r.fit_spiral), and has been checked bit-identical to
calculate_modal_power on a fully-covered synthetic stack
(max abs diff frac_of_data == 0.0). Worth pulling that fault-tolerance
into eddy directly rather than leaving it as a local workaround, along with
a test case that exercises a ring with insufficient azimuthal coverage
(currently untested).

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions