[Valyrian-Code]

Removes the flaky parallel (nprocs=2) comparison from test_nodxdy in the r.sim.water gunittest. The per-cell depth values from a parallel run depend on walker scheduling and the RNG, so assertRastersEqual against a serial reference fails intermittently in CI (#7506). The deterministic serial cell-wise check is kept.

Per review, broader pytest coverage is left to the comprehensive r.sim.water tests @wenzeslaus is working on, so this PR is now scoped to just the flaky test removal.

Fixes #7506.

[Copilot]

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Updates the r.sim.water test strategy to avoid brittle pixel-perfect comparisons for parallel runs by introducing a property-based pytest test and removing the old parallelized reference-compare check.

Changes:

• Removed the previous parallelized dxdy test and related temp map name from the legacy testsuite.
• Added a new pytest test which validates physical invariants (non-negative depth, downhill accumulation) for both serial and parallel runs.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.

File	Description
raster/r.sim/r.sim.water/testsuite/test_r_sim_water.py	Removes the old parallel nprocs=2 depth comparison and an associated temp raster name.
raster/r.sim/r.sim.water/tests/r_sim_water_test.py	Adds a new property-based pytest covering both nprocs=1 and nprocs=2 runs.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Valyrian-Code]

Thanks for the review. A few clarifications:

• Raster name reuse across the parametrized runs: the session_tools fixture (from raster/conftest.py) is function-scoped, so each nprocs case runs in its own temporary mapset. The names do not collide and there is no order dependence, which is why the test passes for both nprocs=1 and nprocs=2. So no overwrite or unique names are needed here.

• tools.r_univar(..., format="json").json: the grass.tools result supports .json (it is used the same way in already-merged tests), so this does not raise. The result is also subscriptable, which is why the existing gunittest in this tool uses stats["sum"] directly. Both styles work; I kept .json for explicitness.

• The 33 and 67 cutoffs: fair point, they are tied to the fixed 0..100 region in this test. I am happy to derive them from the region extent (thirds of the north/south bounds) if you would prefer; I kept them as literals since the region is set just above.

Happy to adjust any of these if you would rather I do.

[petrasovaa]

Hi, @wenzeslaus has been working on a comprehensive set of tests similar to this one and so I would suggest to only keep the removal of the flaky test in the PR.

[Valyrian-Code]

Thanks Anna, that makes sense. I have narrowed this to only remove the flaky parallel comparison (the nprocs=2 block whose per-cell depths depend on walker scheduling and the RNG). I dropped the new pytest so it does not overlap with the comprehensive tests @wenzeslaus is working on. Ready for another look whenever you have time.