Repository navigation
r.sim: Fix null Manning's n and infiltration handling - #7981
Merged
Merged
Conversation
Nulls in the Manning's n raster map were read as the internal undefined value -9999, but unlike nulls in the rainfall excess, partial derivative and sediment input maps, they were not copied into the elevation grid, so these cells stayed in the simulation domain with n = -9999. The inverse 1/n was then -1e-4, which made the velocity there about 1e-5 of the velocity for n = 0.1 and pointed it uphill. The cells also entered the mean velocity, and so the time step and the number of iterations for the whole domain, and the mean Manning's n in the -p summary and the map history. The discharge output was negative in them. Copy the nulls of the Manning's n map into the elevation grid as it is done for the other input maps. The cells are now outside of the domain: the outputs are null there and walkers which reach them leave the domain. r.sim.water and r.sim.sediment share this code, so this applies to both. Only runs with nulls in the Manning's n map change. With a Manning's n map without nulls, the r.sim.water and r.sim.sediment outputs from the nc_spm elevation are byte-identical to those before this change. Written with the help of Claude Code.
A run with a null patch in the Manning's n map must give the same depth, discharge and run summary as a run with null elevation over the same patch. The partial derivatives are given as inputs, so that they are the same in both runs. The test also checks that the outputs are null in the patch, that the discharge is nowhere negative, and that the reported mean Manning's n is the value used outside of the patch. Written with the help of Claude Code.
Nulls in the infiltration raster map were read as the internal undefined value -9999. The walker loop skipped these cells, so they already had no infiltration, but the value entered the mean infiltration reported in the -p summary and the map history, which came out negative. Set the null cells to zero infiltration when the map is read. The depth and discharge do not change. A new test checks that null infiltration gives the same outputs and run summary as zero infiltration in the same cells. Written with the help of Claude Code.
Replace the sentence about null Manning's n with one paragraph in NOTES of each manual which lists the input maps whose null cells are excluded from the simulation and says what happens there. For r.sim.water, it also says that null infiltration means no infiltration. Written with the help of Claude Code.
petrasovaa
approved these changes
Oct 1, 2026
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.
Null cells in the Manning's n map were read as the internal no-data value -9999 and, unlike nulls in the other input maps, stayed in the simulation. Water there moved very slowly and uphill, the discharge came out negative, and the cells changed the time step and the reported mean Manning's n for the whole run. This masks them like the nulls of the other inputs, so they are excluded from the simulation, the outputs are null there, and walkers which reach them leave the simulated area.
The simulation already treated null cells in the infiltration map as no infiltration, but they made the reported mean infiltration negative. They are now read as zero infiltration, so only the reported mean changes. While this handling of infiltration is inconsistent with the other null handling, the "no infiltration" intuitively means zero as opposed unknown.
Runs without nulls in the Manning's n map give the same outputs as before.
Details (AI generated)
input_data()inraster/r.sim/simlib/input.ccopies the nulls of the man map into the elevation grid withcopy_matrix_undef_float_values(), as it is done for rain, dx, dy and the sediment inputs. The cells leave the domain, the outputs are null there and walkers reaching them leave the domain.grad_check()inverted them to 1/n = -1e-4 (only 0 is checked). The velocity there was about 1e-5 of that for n = 0.1 and pointed uphill. It entered the mean velocity, hence the time step and the number of iterations, and the mean Manning's n in the run summary (r.sim: Add -p flag with format=json run summary and normalize history #7972) and the map history. The storedcchez * sqrt(sinsl)made the discharge output negative there.input_data(), so the fix applies to it as well.r.sim.water.mdandr.sim.sediment.mdlists the input maps whose null cells are excluded from the simulation (the outputs are null there and walkers that reach them leave the area), and for r.sim.water says that null infiltration means no infiltration, mirrored in the.htmlfiles.-psummary compared to main.Tests
test_null_mannings_n_is_like_null_elevation(new, 1.2 s):test_null_infiltration_is_no_infiltration(new, about 1 s):Squash-merge commit message
Title:
Body:
The analysis, code and tests were done with Claude Code (Opus) and reviewed by me.