Skip to content

Fix sqrt(2) precision, add pre-coregistration, and correct variogram plotting in uncertainty propagation - #9

Open
dshean wants to merge 2 commits into
rhugonnet:coreg_study_ajustmentsfrom
uw-cryo:uncertainty-propagation-fixes
Open

Fix sqrt(2) precision, add pre-coregistration, and correct variogram plotting in uncertainty propagation#9
dshean wants to merge 2 commits into
rhugonnet:coreg_study_ajustmentsfrom
uw-cryo:uncertainty-propagation-fixes

Conversation

@dshean

@dshean dshean commented Jul 1, 2026

Copy link
Copy Markdown

Fixes and tests for elevation-uncertainty propagation (the uncertainty module), building on GlacioHack#759.
These came out of testing _propag_uncertainty_coreg / _infer_uncertainty on the Longyearbyen sample
DEMs and on 0.5 m data. Defaults are unchanged throughout, so existing results are bit-identical unless
the new options are used.

Changes

  1. precision_of_other="same" was a no-op. The √2 "same precision" correction (Hugonnet et al.,
    2022, Eq. 7-8) was commented out in _infer_uncertainty and never threaded down, so for two datasets
    of similar precision (e.g. DEM–DEM) the inferred per-pixel error was ~√2 too large, inflating the
    propagated translation uncertainty. Re-enabled in the heteroscedasticity step (matching the legacy
    dem.py path). Default "finer" is unchanged; an unsupported value now raises instead of silently
    behaving like "finer".

  2. Error structure was inferred from un-coregistered inputs. _propag_uncertainty_coreg ran
    _infer_uncertainty once on the raw inputs, conflating true error with the misalignment/terrain
    (and, in the test case, glacier-change) signal. Added an opt-in precoreg flag: when True,
    co-register once, then infer the error structure and simulate on the aligned residual (this automates
    the manual fit→apply→propagate workaround). With precoreg=True, summary["mean"] becomes the
    residual transform (≈0), which is documented. Default precoreg=False preserves current behaviour.
    (Kept opt-in because apply's edge regridding can make NuthKaab diverge on very small extents.)

  3. Variogram plotting. _infer_uncertainty returns the correlation function, which renders
    incorrectly when passed to plot_variogram (which expects a variogram). No code change is needed —
    the variogram is reconstructible from the already-public get_variogram_model_func(params). Added a
    test of the γ(h) = sill·(1 − ρ(h)) invariant that underpins the correct plotting recipe.

  4. Monte Carlo robustness. A single non-converging simulation aborted the whole run. Diverged
    simulations are now skipped with a warning, with a guard that requires a minimum number of successful
    simulations before reporting. The success counts are also exposed as summary.attrs (nsim,
    n_success, frac_success) so callers can gate on the skip fraction — a high fraction is itself a
    conditioning signal (ill-posed geometry, e.g. flat terrain), so it should be machine-readable rather
    than only visible as a logging warning (402c389).

Tests

tests/test_uncertainty/test_uncertainty_notebook.py (cropped sample data, small nsim): √2 scaling,
invalid precision_of_other raises, precoreg equivalence + determinism, diverged-sim skip, and the
variogram-plotting invariant.

Notes

No public API change: _propag_uncertainty_coreg is private and precision_of_other was already
documented; precoreg / error_applied_to docstrings added.

Disclosure: prepared with AI assistance (Claude). I have reviewed and understand all changes and take
responsibility for them.

- Enable the sqrt(2) "same precision" correction (Hugonnet 2022, Eq. 7-8) in the
  heteroscedasticity step; it was commented out, so precision_of_other="same" was a no-op.
  Default "finer" is unchanged, and an unsupported value now raises.
- Add an opt-in precoreg flag to co-register before inferring the error structure, so it is
  estimated on the aligned residual rather than the raw inputs. Default False (unchanged).
- Skip a non-converging Monte Carlo simulation with a warning instead of aborting the whole
  run, with a minimum-successful-simulations guard.
- Add tests on the Longyearbyen sample data (sqrt(2) scaling, precoreg equivalence and
  determinism, diverged-simulation skip, and the variogram-plotting invariant).
@dshean
dshean force-pushed the uncertainty-propagation-fixes branch from 4d074ba to f48336c Compare July 1, 2026 19:35
Complements the diverged-simulation skip: report nsim, n_success, and
frac_success as summary.attrs metadata so callers can gate on the skip
fraction. A high skip fraction is itself a conditioning signal (ill-posed
geometry, e.g. flat terrain), so it should be machine-readable rather than
only visible as a logging warning.
@dshean
dshean force-pushed the uncertainty-propagation-fixes branch from 1d5dbe3 to 402c389 Compare July 9, 2026 19:24
@dshean

dshean commented Aug 18, 2026

Copy link
Copy Markdown
Author

Hi @rhugonnet. I see others are testing this branch. Are you planning to review and merge these on your branch before it goes upstream?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant