Skip to content

r.sim: Add walker weights to the water depth atomically - #7974

Merged
wenzeslaus merged 2 commits into
OSGeo:mainfrom
wenzeslaus:simwe-depth-atomic
Sep 30, 2026
Merged

wenzeslaus merged 2 commits into
OSGeo:mainfrom
wenzeslaus:simwe-depth-atomic

Conversation

@wenzeslaus

@wenzeslaus wenzeslaus commented Sep 30, 2026 •

Copy link
Copy Markdown
Member

With nprocs > 1, walkers on different threads add their weights to the same cells of the r.sim water depth (or sediment concentration) grid without synchronization, so additions are lost and parallel runs lose water. This makes the addition atomic. Single-threaded results do not change.

This adds 6-7% of runtime for nprocs=1 and 5-8% for 4 and 8 threads for small raster (200×200). An alternative could have a branch for nprocs=1 which would not use the atomic. Notably, this additional cost will not go away with the new random numbers (WIP), but the new RNG code should make the parallel code faster (much faster comparing to what is now on main which is RNG with atomics, and comparing to state before atomics with shared generator without locking, the new RNG code should be somewhat faster in serial, and in parallel, it should be actually faster parallel then serial as in #7884).

So, while the atomics slightly decrease performance comparing to hitting same variables with race condition, they add correctness and a better parallel implementation will outweigh the cost of atomics. The choice is between one simple code with slight cost for single-threaded run (which will still do part of the atomics operations; cost 6% on 4 s run) and duplicating the code slightly to most out of the single-threaded run and keeping the atomics to parallel runs (with <1% cost comparing to main for a single-thread).

Locally, the tests run about 2 s adding to the 11.5 s of other pytest tests (so maybe 3 s in CI).

Details (AI generated)
  • grids->gama[k][l] += in main_loop() (raster/r.sim/simlib/hydro.c) becomes an omp atomic capture. The captured sum is what the diffusion decision uses, as in the serial code.
  • Parallel results still depend on the order in which threads add the weights, but no water is lost.
  • New raster/r.sim/r.sim.water/tests/r_sim_water_nprocs_test.py:
    • It uses a bowl that keeps all walkers in the region, so the total accumulated water does not depend on the random numbers.
    • It compares the totals at nprocs=4 and nprocs=1 (rel 1e-6), with and without diffusion.
    • It also checks a walker count that is not a multiple of the thread count.
    • On main, all three tests fail, with totals 13% to 21% short at nprocs=4. Here they pass (5 of 5 runs).
  • Single-threaded depth, discharge (with and without infiltration), sediment flux and erosion/deposition on nc_spm are byte-identical to main. The r.sim.water pytest suite and both gunittest suites pass.
  • The infiltration updates have a data race of their own, which this does not change.
  • The atomic addition makes single-threaded runs about 6% slower than main, measured on 200×200 cells with 200k walkers, because the locked instruction runs even without contention. Branching on the thread count, with a plain addition when only one thread runs, recovers that (within 1% of main) at the cost of duplicating the two-line update. The choice is between one simpler code path with a ~6% single-threaded cost and slightly duplicated code without it. Parallel runs pay the atomic either way.

Cost of the atomic, and an alternative

Median of 7 interleaved runs, 200x200 cells, 200k walkers, 10 min simulated. The third column is an experiment, not in this PR: it reads the thread count once per parallel region and uses a plain += when only one thread runs.

Case main This PR (always atomic) Branch on thread count
nprocs=1, no infiltration 4.156 s 4.417 s (+6.3%) 4.187 s (+0.8%)
nprocs=1, infiltration 20 2.545 s 2.701 s (+6.1%) 2.568 s (+0.9%)
nprocs=4, no infiltration 9.195 s 9.928 s (+8.0%) 9.855 s (+7.2%)
nprocs=4, infiltration 20 5.583 s 6.061 s (+8.6%) 6.054 s (+8.4%)
  • nprocs=1: the atomic costs about 6%, because the locked instruction runs even without contention. The branch variant is within 1% of main, at the cost of duplicating the two-line update (+13 −2 lines).
  • nprocs=4: both pay the atomic. All parallel runs are dominated by the shared random number generator, which is why nprocs=4 is slower than nprocs=1 in every column.

Squash-merge commit message

r.sim: Add walker weights to the water depth atomically (#7974)

With nprocs > 1, walkers on different threads added their weights to the
same cells of the water depth (or sediment concentration) grid without
synchronization. This is a data race: of two concurrent additions to a
cell, one can be lost, so parallel runs lost water, 12% to 14% of the
total on a small bowl at nprocs=4.

Add the weight with an atomic capture, so that no addition is lost and
the diffusion decision uses the updated sum, as in the serial code.
Parallel results still depend on the order in which threads add the
weights. The atomic makes single-threaded runs about 6% slower.

Single-threaded results do not change: the depth, discharge, sediment
flux and erosion/deposition maps computed from the nc_spm elevation,
with and without infiltration, are byte-identical to those before this
change. New tests compare the total water at 4 threads with a
single-threaded run on a bowl which keeps all walkers in the region,
including a walker count which is not a multiple of the thread count.

Written with the help of Claude Code.

The analysis, code and tests were done with Claude Code (Opus) and reviewed by me.

With nprocs > 1, walkers on different threads added their weights to the
same cells of the water depth (or sediment concentration) grid without
synchronization. This is a data race: of two concurrent additions to a
cell, one can be lost, so parallel runs lose water. On a small bowl with
2000 walkers, the total was 13% to 21% lower at nprocs=4 than at
nprocs=1.

Add the weight with an atomic capture, so that no addition is lost and
the diffusion decision uses the updated sum, as in the serial code.
Parallel results still depend on the order in which threads add the
weights, as before, but no water is lost.

Single-threaded results do not change: the depth, discharge, sediment
flux and erosion/deposition maps computed from the nc_spm elevation,
with and without infiltration, are byte-identical to those before this
change.

Written with the help of Claude Code.
@github-actions github-actions Bot added raster Related to raster data processing Python Related code is in Python C Related code is in C module tests Related to Test Suite labels Sep 30, 2026
Compare the total water accumulated with 4 threads against a
single-threaded run on a bowl which keeps all walkers in the region.
Without diffusion, the random numbers do not affect the walkers, and all
of them end up in the few cells at the bottom of the bowl, so
unsynchronized updates of the same cell are likely. Also test a walker
count which is not a multiple of the thread count, which lost the last
walker before the chunk size was rounded up.

Without the atomic addition of walker weights, both tests fail with
totals 12% to 14% short at nprocs=4 in every run. The tests take about
2 s.

Written with the help of Claude Code.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C Related code is in C module Python Related code is in Python raster Related to raster data processing tests Related to Test Suite

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants