[wenzeslaus]

When r.sim.water runs with nprocs > 1 and infiltration, two threads can check and use the same cell's infiltration capacity at the same time, so a cell can absorb more water than its capacity, or lose track of what it absorbed. The unsynchronized access is undefined behavior in C.

This runs the existing infiltration code unchanged in a critical section, entered only for cells which still have capacity. Single-threaded maps stay byte-identical to main, and the run time is the same within noise. Please review the diff with whitespace changes hidden: the original code only gets indented, and what remains is the lock, the flag replacing the two continue statements, and the two atomic writes.

Details: approach, physics, measurements, and tests (AI generated)

• Commit 1, the fix:
  • The walker loop reads the cell's capacity with omp atomic read. Only when some is left, it enters omp critical(infiltration), in which the original code runs, starting with its own check of the capacity, since another thread may have used it up.
  • The original code is unchanged except that the two continue statements set a flag checked after the critical section, because continue cannot leave it, and that the two writes to the capacity are atomic, because the read before the lock is outside it.
  • Why the lock is cheap: after the first step, few walkers are in a cell with capacity left. On the doc example with infiltration from landclass96, 201 of 496,626 active walkers in step 2 and 2 of 347,836 in step 710; with infiltration above the rain on most cells, 88 of 25,706 in step 2.
• Physics: the arithmetic and the conditions are unchanged. With nprocs > 1, the order in which walkers reach a cell depended on thread scheduling before and still does; each walker's check and update of the capacity now happens as a whole.
• Commit 2: a test that infiltration removes exactly the capacity of the cells, at 1 and 4 threads.
• Output: with nprocs=1, the depth from the doc example is byte-identical to main without infiltration, with infil_value=50, and with both landclass96 maps below. All tests pass.
• ThreadSanitizer (clang + libomp + Archer, only r.sim instrumented): no race in r.sim at nprocs 4 and 8, and no reports at nprocs=1, in a 30x30-cell run where rain on the higher half flows into cells with infiltration capacity, so that walkers from different threads compete for the same capacity. The same run on main reports the race between the capacity check and its updates. Plain water, full absorption, capacity used up in the first step, and r.sim.sediment also report nothing.

Timing

Doc example (elev_lid792_1m, 525k cells and walkers, rain_value=100, duration=5), median and range of 3 interleaved runs. I0: no infiltration. I50: infil_value=50. L1: landclass96 forest 150, other 50. L2: forest 50, other 120. Water cells 0 in both.

nprocs	scenario	main	this PR
1	I0	45.51 s [39.47–45.66]	46.73 s [39.89–47.71] (+2.7%)
1	I50	42.48 s [38.82–43.91]	40.15 s [39.79–43.13] (−5.5%)
1	L1	39.17 s [37.67–40.33]	41.14 s [37.71–44.11] (+5.0%)
1	L2	2.46 s [2.38–2.89]	2.50 s [2.37–2.75] (+1.5%)
4	I0	58.29 s [56.72–58.33]	60.22 s [59.32–60.66] (+3.3%)
4	I50	61.96 s [58.86–62.03]	61.44 s [60.92–62.22] (−0.8%)
4	L1	56.55 s [55.95–57.74]	57.84 s [56.74–59.59] (+2.3%)
4	L2	2.04 s [1.99–2.66]	2.10 s [2.05–2.75] (+3.0%)

The machine load varied during the runs; the differences go both ways and the ranges overlap except for I0 at nprocs=4, where this PR differs from main only by an atomic read.

Squash-merge commit message

Title:

r.sim: Lock the infiltration capacity of a cell

Body:

With nprocs > 1, walkers on different threads checked and reduced the
remaining infiltration capacity of a cell without synchronization. This
is a data race and undefined behavior in C: two walkers could both be
absorbed by the capacity left for one, or a reduction could be lost.

Run the unchanged infiltration code in a critical section, entered only
when an atomic read shows capacity left in the cell. Within the lock,
the code checks the capacity again, as another thread may have used it
up. The capacity is written atomically, since it is also read outside
of the lock. After the first step, few walkers are in a cell with
capacity left, so the lock is rarely taken.

The arithmetic and the conditions are unchanged. With one thread, the
depth maps computed from the elev_lid792_1m elevation, with and without
infiltration, are byte-identical to those before this change, and the
run time is the same within noise with 1 and 4 threads. With more
threads, the order in which walkers reach a cell still depends on
thread scheduling, as before. A new test checks that infiltration
removes exactly the capacity of the cells at 1 and 4 threads.

Written with the help of Claude Code.

The fix and the measurements were done with AI assistance (Claude Code with Opus 5.5) and reviewed by me.

[petrasovaa]

The change makes sense. For testing it could reuse an existing test to save time.