Skip to content

Avoid kc becomes zero when NDVI == 1.1 * NDVI_min - #324

Open
LINAMARIAOSORIO wants to merge 8 commits into
fix_319from
fix_320
Open

Avoid kc becomes zero when NDVI == 1.1 * NDVI_min#324
LINAMARIAOSORIO wants to merge 8 commits into
fix_319from
fix_320

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

In dynamic() the crop coefficient used two strict masks (NDVI < 1.1 * NDVI_min and NDVI > 1.1 * NDVI_min), so a cell whose NDVI equals 1.1 * NDVI_min got kc = 0 and no vegetated-area evapotranspiration. The branch follows the documented rule with pcr.ifthenelse(current_ndvi <= 1.1 * self.ndvi_min, min_crop_coef, partial_crop_coef): kc = kc_min whenever NDVI <= 1.1 * NDVI_min.

Follow-up commits on top of the fix:

  • A behaviour test in tests/unit/core/test_dynamic_model.py: three synthetic runs with the first-step NDVI exactly on the threshold, at NDVI_min and above it; the first-step eta of the threshold run must equal the NDVI_min run and differ from the interpolated run (fails on main, where the threshold run has eta = 0).
  • Merge of fix_319 and a changelog entry under Unreleased › Fixed.

Stacked on #323 (base branch fix_319): the diff of this pull request is rubem/_dynamic_model.py, the test and the changelog line.

Related Issue

Motivation and context

doc/source/overview.rst (equation kc) and the supplementary document (equation SE11) state kc = kc_min if NDVI <= 1.1 * NDVI_min; the equality case was left without a branch. The regression dataset does not contain the equality (the exact job reports byte-identical outputs with and without this change), so the behaviour is covered by the new test.

How has this been tested

  • Local suite (pytest --ignore=tests/integration/doc): 998 passed, 1 skipped.
  • test_ndvi_at_the_crop_coefficient_threshold_takes_the_minimum_branch fails with rubem/_dynamic_model.py from main and passes here. The threshold value is PCRaster's own float32 product of 1.1 and the ndvi_min raster, so the model's comparison sees exact equality.
  • Golden fixtures are not affected by this change: byte-exact reproduction verified in the golden environment container with this branch.
  • 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 320 Avoid kc becomes zero when NDVI == 1.1 * NDVI_min Avoid kc becomes zero when NDVI == 1.1 * NDVI_min Sep 2, 2026
@soaressgabriel
soaressgabriel changed the base branch from main to fix_319 September 5, 2026 16:44
@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 (1613d18) to head (ed3be1c).

Additional details and impacted files
@@             Coverage Diff             @@
##           fix_319     #324      +/-   ##
===========================================
- Coverage    92.47%   92.47%   -0.01%     
===========================================
  Files           60       60              
  Lines         3908     3906       -2     
  Branches       487      487              
===========================================
- Hits          3614     3612       -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.

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.

kc becomes zero when NDVI == 1.1 * NDVI_min

2 participants