[com4FlowPy] Fix issue #1344 by ahuber-bfw · Pull Request #1348 · OpenNHM/AvaFrame

ahuber-bfw · 2026-11-09T12:08:18Z

PR Summary

⚠️ Important: PR might introduce potential backward-compatibility issues with com4FlowPy.checkInputParameterValues() for external callers, since the updated function now expects an additional input forestParams if modelParams['forestBool'] - if modelParams['forestBool'] == False then the legacy behaviour is unchanged.

This PR fixes issue #1344 and also applies a Bugfix previously applied in the python engine (PR #1342) to the numba engine (fixed typo in calculation of 3D distances).

Summary of Changes

Commit	Subject
`684ea5cb`	fixes issue #1341 also for numba implementation
`ef93b3af`	added forest input parameter checks in com4FlowPy.py
`c9c8a69d`	deleted main entryPoints from com4 pytests and added tests for forest input parameter checks

Changed files:

avaframe/com4FlowPy/com4FlowPy.py
avaframe/com4FlowPy/flowCoreNumba.py
avaframe/tests/test_com4FlowPy.py
avaframe/tests/test_com4FlowPyNumba.py

lines changed:
+388
-29

1. Forest input parameter checks (`com4FlowPy.py`)

added new input data checks for forestModule related input parameters and data to com4FlowPy.checkInputParameterValues(), that is triggered when 'forestBool' is set.

added checks - grouped by forestModule:

All modules
- skipForestDist >= 0.
- alphaMax is taken from the global alpha, or — when varAlphaBool is true — as the
  maximum of the variable-alpha raster inside the release-area mask.
forestFriction / forestDetrainment
- FSI raster values must lie in [0, 1] (cells with FSI <= 0 are masked out, matching the
  existing flowClass.py convention).
- alphaMax + maxAddedFriction < 90 (total friction must not reach 90°).
- velThForFriction >= 0.
- minAddedFriction <= maxAddedFriction.
- Detrainment extras: velThForDetrain >= 0, minDetrainment <= maxDetrainment, and both
  detrainment bounds within [0, 1].
forestFrictionLayer
- velThForFriction >= 0.
- forestFrictionLayerType == "absolute": layer values within [0, 90].
- forestFrictionLayerType == "relative": 0 <= alphaMax + max(layer) < 90.
- Any other type raises ValueError.

All failures raise ValueError with a message naming the offending parameter and config key.
On success, a short summary is written to the log.

2. Numba engine bug fix (`flowCoreNumba.py`)

The 3D minimum-distance calculation in the numba engine flowCoreNumba.py - _bfs_single() used dy twice
(sqrt(dy² + dy² + dz²)), replicating an upstream Bug of flowClass.calcDistMin().
It now uses sqrt(dx² + dy² + dz²), mirroring the fix already on master in flowClass.py
(issue #1341). This affects the skipForestDist gate in the Numba engine.

3. Tests (`test_com4FlowPy.py`, `test_com4FlowPyNumba.py`)

modified/extended test_com4FlowPy.py
- New helpers makeForestParameters(), makeForestModelPaths() and
  _makeForestReadRaster() to build valid inputs and fake IOf.readRaster.
- New tests for checkForestInputParameters() covering all three forest modules, both
  global and variable alpha, all valid branches, and the invalid cases (out-of-range FSI,
  friction/detrainment limits, unknown layer type, negative thresholds), using
  pytest.mark.parametrize.
- Removed the if __name__ == "__main__": block that re-ran a hand-picked subset of tests. $\rightarrow$ this is not required when invoking tests through pytest
test_com4FlowPyNumba.py
- Removed the corresponding if __name__ == "__main__": block. $\rightarrow$ this is not required when invoking tests through pytest

4. Main Changes

Invalid forest parameter and input data settings now abort with ValueError() instead of failing silently or producing unintended results.

PR Checklist

Please confirm before requesting review:

I ran pytest locally without fails
I added/updated tests where needed
I updated documentation where needed

Confirm before the final merge/rebase into master

Commits are sensibly squashed and rebased onto latest master
Standardtest run without difference (with recompiled cython code)

… input parameter checks

qltysh · 2026-11-09T12:10:16Z

Analysis for project `AvaFrame`

❌ 1 blocking issue (1 total)

Tool	Category	Rule	Count
ruff	Lint	Local variable `fParams` is assigned to but never used	1	❌

fso42

This adds backwards incompatible code (checkInputParameterValues now takes an extra argument). Please highlight this in the PR summary, as external calles must update this

fso42 · 2026-11-09T13:43:31Z

+        # convert to binary following the > 0 convention used
+        fsiMask = (fsi > 0)
+        # check if FSI is provided and in correct scaling (0,...,1)
+        isValid = validateInputArray(fsiMask, fsi, 1)


Mh, what happens if the forest layer exists, but has no forest cells? the mask is empty and np.max(fsi...) might crash, right?

fso42 · 2026-11-09T13:45:32Z

+                    "Error: (some) provided 'forestFrictionLayer' values are outside the allowed range [0, ..., 90]"
+                )
+                raise ValueError(
+                "Invalid forest Parameters ('forestFrictionLayer') found in  {modelPaths['forestPath']}\n"


the f prefix is missing, the literal {modelPaths...} will be printed, that's not wanted, right?

fso42 · 2026-11-09T13:45:55Z

+            _dAlphaForMax = np.max(fsi[fsiMask])
+            if (_alphaMax + _dAlphaForMax) >= 90 or (_alphaMax + _dAlphaForMax) < 0:
+                raise ValueError(
+                "Invalid forest Parameters ('forestFrictionLayer') found in  {modelPaths['forestPath']}\n"


same here, f prefix

qltysh · 2026-11-09T15:49:54Z

+    """
+    test intended behavior of com4FlowPy.checkInputParameterValues()
+    """
+    fParams = makeForestParameters(


Local variable fParams is assigned to but never used [ruff:F841]

Suggested change

fParams = makeForestParameters(

makeForestParameters(

PaulaSp3 · 2026-11-09T15:12:07Z

    alpha, uMaxLimit/ zDeltaMaxLimit, exponent
    are within a physically sensible range

    Parameters


Add forstParameters

PaulaSp3 · 2026-11-09T15:16:08Z

    """check if the input parameters are valid
    are valid and within physically sensible limits
    alpha, uMaxLimit/ zDeltaMaxLimit, exponent
    are within a physically sensible range


maybe we could update this description to:

check if the input parameters are valid and within a physically sensible range.

PaulaSp3 · 2026-11-09T15:35:16Z

-    log.info(24*"=")
+    log.info(24*"=")
+
+def checkForestInputParameters(forestParameters, modelParameters, modelPaths):


Maybe for all checks it could make sense to write the error messages into the log file?
Similar like:

message = "..."
log.error(message)
raise ValueError(message)

PaulaSp3 · 2026-11-09T15:39:02Z

+                "unknown value for 'forestFrictionLayerType'\n"
+                "must be 'absolute' or 'relative'"
+            )
+


could add also here an else: to check if the forestModule is valid?

ahuber-bfw added 3 commits November 2, 2026 16:21

fixes issue #1341 also for numba implementation

684ea5c

added forest input parameter checks in com4FlowPy.py

ef93b3a

deleted main entryPoints from com4 pytests and added tests for forest…

c9c8a69

… input parameter checks

ahuber-bfw requested a review from PaulaSp3 November 9, 2026 12:08

ahuber-bfw changed the title ~~[com4FlowPy] Fix issue1344~~ [com4FlowPy] Fix issue #1344 Nov 9, 2026

qltysh Bot reviewed Nov 9, 2026

View reviewed changes

linting issues addressed

40c91e9

fso42 assigned ahuber-bfw Nov 9, 2026

fso42 added the bugfix label Nov 9, 2026

fso42 linked an issue Nov 9, 2026 that may be closed by this pull request

[com4]: no input data validity check for Forest Layers - esp. FSI #1344

Open

fso42 requested changes Nov 9, 2026

View reviewed changes

comments FSO

0b2e393

qltysh Bot reviewed Nov 9, 2026

View reviewed changes

PaulaSp3 requested changes Nov 9, 2026

View reviewed changes

Conversation

ahuber-bfw commented Nov 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary

Summary of Changes

1. Forest input parameter checks (com4FlowPy.py)

2. Numba engine bug fix (flowCoreNumba.py)

3. Tests (test_com4FlowPy.py, test_com4FlowPyNumba.py)

4. Main Changes

PR Checklist

Uh oh!

qltysh Bot commented Nov 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Analysis for project AvaFrame

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fso42 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

qltysh Bot Nov 9, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ahuber-bfw commented Nov 9, 2026 •

edited

Loading

1. Forest input parameter checks (`com4FlowPy.py`)

2. Numba engine bug fix (`flowCoreNumba.py`)

3. Tests (`test_com4FlowPy.py`, `test_com4FlowPyNumba.py`)

qltysh Bot commented Nov 9, 2026 •

edited

Loading

Analysis for project `AvaFrame`