Repository navigation
r.sim.water: fix flaky buffer overflow and OpenMP thread-safety issues - #7521
anzalkabeer wants to merge 3 commits into
Conversation
anzalkabeer
commented
Jun 8, 2026
anzalkabeer
commented
- Add missing bounds check exit (continue) in hydro.c loop
- Replace thread-unsafe G_drand48() in OpenMP threads with thread-safe LCG PRNG
- Prevent concurrency data races on shared grids by adding OpenMP atomic and critical sections
- Add missing bounds check exit (continue) in hydro.c loop - Replace thread-unsafe G_drand48() in OpenMP threads with thread-safe LCG PRNG - Prevent concurrency data races on shared grids by adding OpenMP atomic and critical sections
github-actions
Bot
added
raster
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| decr; /* decrease infilt. in cell | ||
| and eliminate the walker */ |
There was a problem hiding this comment.
[pre-commit] reported by reviewdog 🐶
| decr; /* decrease infilt. in cell | |
| and eliminate the walker */ | |
| decr; /* decrease infilt. in | |
| cell and eliminate the | |
| walker */ |
| addac; /* use just proportional part | ||
| of the walker weight */ |
There was a problem hiding this comment.
[pre-commit] reported by reviewdog 🐶
| addac; /* use just proportional part | |
| of the walker weight */ | |
| addac; /* use just proportional | |
| part of the walker | |
| weight */ |
| #if defined(_OPENMP) | ||
| int i; | ||
| int max_threads = omp_get_max_threads(); | ||
| if (max_threads > MAX_SIMWE_THREADS) max_threads = MAX_SIMWE_THREADS; |
There was a problem hiding this comment.
[pre-commit] reported by reviewdog 🐶
| if (max_threads > MAX_SIMWE_THREADS) max_threads = MAX_SIMWE_THREADS; | |
| if (max_threads > MAX_SIMWE_THREADS) | |
| max_threads = MAX_SIMWE_THREADS; |
|
Glad to see you managed to do the new branch and PR |
|
Thanks @anzalkabeer, and glad the property-based approach made sense to you. I looked into the CI, and I don't think it is rate limiting. There seem to be two real causes:
Happy to help once those are sorted. |
|
@Valyrian-Code i will implement these things into the next PRr in some time thanks for spotting the real problem in the PR |
|
No problem @anzalkabeer, glad it helped. Ping me when the next PR is up and I will take another look, and I am happy to help if the reference regeneration turns out to be fiddly. |
github-actions
Bot
added
Python
|
@Valyrian-Code have a look at the new PR please |
| self.assertGreaterEqual(depth["min"], 0) | ||
| self.assertEqual(depth["null_cells"], 0) | ||
| self.assertGreater(depth["sum"], 0) | ||
|
|
There was a problem hiding this comment.
[pre-commit] reported by reviewdog 🐶
| { | ||
| #if defined(_OPENMP) | ||
| int tid = omp_get_thread_num(); | ||
| if (tid >= MAX_SIMWE_THREADS) tid = MAX_SIMWE_THREADS - 1; |
There was a problem hiding this comment.
[pre-commit] reported by reviewdog 🐶
| if (tid >= MAX_SIMWE_THREADS) tid = MAX_SIMWE_THREADS - 1; | |
| if (tid >= MAX_SIMWE_THREADS) | |
| tid = MAX_SIMWE_THREADS - 1; |
|
Generating reference data in the test doesn't make sense. Small, focused PRs would be easier to review. |
