Fix sqrt(2) precision, add pre-coregistration, and correct variogram plotting in uncertainty propagation - #9
Open
dshean wants to merge 2 commits into
Conversation
dshean
force-pushed
the
uncertainty-propagation-fixes
branch
from
July 1, 2026 19:30
5bebae1 to
4d074ba
Compare
- 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
force-pushed
the
uncertainty-propagation-fixes
branch
from
July 1, 2026 19:35
4d074ba to
f48336c
Compare
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
force-pushed
the
uncertainty-propagation-fixes
branch
from
July 9, 2026 19:24
1d5dbe3 to
402c389
Compare
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? |
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.
Fixes and tests for elevation-uncertainty propagation (the
uncertaintymodule), building on GlacioHack#759.These came out of testing
_propag_uncertainty_coreg/_infer_uncertaintyon the Longyearbyen sampleDEMs and on 0.5 m data. Defaults are unchanged throughout, so existing results are bit-identical unless
the new options are used.
Changes
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_uncertaintyand never threaded down, so for two datasetsof 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.pypath). Default"finer"is unchanged; an unsupported value now raises instead of silentlybehaving like
"finer".Error structure was inferred from un-coregistered inputs.
_propag_uncertainty_coregran_infer_uncertaintyonce on the raw inputs, conflating true error with the misalignment/terrain(and, in the test case, glacier-change) signal. Added an opt-in
precoregflag: whenTrue,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 theresidual transform (≈0), which is documented. Default
precoreg=Falsepreserves current behaviour.(Kept opt-in because
apply's edge regridding can make NuthKaab diverge on very small extents.)Variogram plotting.
_infer_uncertaintyreturns the correlation function, which rendersincorrectly 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 atest of the
γ(h) = sill·(1 − ρ(h))invariant that underpins the correct plotting recipe.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 aconditioning 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, smallnsim): √2 scaling,invalid
precision_of_otherraises, precoreg equivalence + determinism, diverged-sim skip, and thevariogram-plotting invariant.
Notes
No public API change:
_propag_uncertainty_coregis private andprecision_of_otherwas alreadydocumented;
precoreg/error_applied_todocstrings added.Disclosure: prepared with AI assistance (Claude). I have reviewed and understand all changes and take
responsibility for them.