Skip to content

Avoid 1e-5 added to precipitation for all timesteps - #323

Open
LINAMARIAOSORIO wants to merge 9 commits into
mainfrom
fix_319
Open

Avoid 1e-5 added to precipitation for all timesteps#323
LINAMARIAOSORIO wants to merge 9 commits into
mainfrom
fix_319

Conversation

@LINAMARIAOSORIO

@LINAMARIAOSORIO LINAMARIAOSORIO commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator

Checklist

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?
  • Added tests for changed code.
  • Updated documentation for changed code.

Description

Interception.get_interception guards the interception-rate equation I_R = 1 - exp(-I_D * d_p / P_m) against a zero monthly precipitation by replacing P_m with 1e-5 in the denominator. The guard was written as prec = P * (P != 0) + (P * (P == 0) + 0.00001), so the + 0.00001 applied to every value and positive precipitation reached the denominator as P + 1e-5 mm. The guard is now pcr.ifthenelse(precipitation != 0, precipitation, pcr.scalar(1e-5)): positive precipitation is used unchanged and the epsilon only applies where P == 0.

Follow-up commits on top of the fix:

  • ruff format of the changed module (the lint job runs ruff format --check).
  • Regression tests in tests/unit/hydrological_processes/test_module_interception.py: positive precipitation reproduces the equation evaluated in float64; a small positive precipitation (P = 1e-4 mm) that the old guard lowered by about 6 % (this test fails on main); zero precipitation stays finite and zero.
  • Golden fixtures regenerated (28 of the 45 files moved), SHA256SUMS rewritten and a new entry at the top of tests/fixtures/AUDIT.md with the provenance, the impact table and the replaced checksums.
  • Changelog entry under Unreleased › Fixed.
  • After review: two sentences of the audit notes corrected (the sampled-series justification and the rejected cell, which is also in srn0000001.tif), and origin/main merged so the branch is up to date for the protected merge.

Related Issue

Motivation and context

The 1e-5 mm perturbation of the denominator diverges from the documented interception rate (doc/source/overview.rst, equation interception-r; supplementary document, equation SE3). The hydrological effect is numerical noise (at most 3.05e-5 mm on the regression rasters, 2.4e-4 mm on smc), but the semantic golden comparison of the CI matrix (rtol = 1e-5) rejected the previous goldens on one srn cell, so the fixtures had to be regenerated.

How has this been tested

  • Local suite (pytest --ignore=tests/integration/doc): 997 passed, 1 skipped.
  • test_interceptionCalc_small_positive_precipitation_keeps_denominator fails with the module from main (4.90e-5 instead of 5.23e-5) and passes with this branch.
  • Goldens regenerated with tests/fixtures/regenerate_golden.py in a Docker container of ubuntu:24.04 (glibc 2.39-0ubuntu8.8) with the environment from ci/golden-env.lock and ci/golden-pip.lock. Before promoting them, the container reproduced the previous goldens byte-for-byte on main; the 28 regenerated digests are identical to the ones the exact job reported for this branch before the regeneration (run 33662562795) and the 17 untouched files keep their digests.
  • The lint, docs and exact CI jobs were also run locally with act (ubuntu-24.04 runner image): green.
  • CI on this pull request: ci-success green.

Screenshots

  • N/A

@LINAMARIAOSORIO LINAMARIAOSORIO self-assigned this Sep 2, 2026
@soaressgabriel soaressgabriel changed the title Fix 319 Avoid 1e-5 added to precipitation for all timesteps Avoid 1e-5 added to precipitation for all timesteps Sep 2, 2026
@codecov

codecov Bot commented Sep 5, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.47%. Comparing base (9deebb4) to head (1613d18).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #323      +/-   ##
==========================================
- Coverage   92.48%   92.47%   -0.01%     
==========================================
  Files          60       60              
  Lines        3910     3908       -2     
  Branches      487      487              
==========================================
- Hits         3616     3614       -2     
  Misses        238      238              
  Partials       56       56              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@soaressgabriel soaressgabriel changed the title Avoid 1e-5 added to precipitation for all timesteps Avoid 1e-5 added to precipitation for all timesteps Sep 5, 2026
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.

1e-5 is added to precipitation for all timesteps in interception calculation

2 participants