Repository navigation
r.sim: Add -p flag with format=json run summary and normalize history - #7972
Merged
Merged
Conversation
Member
|
If you update from main, you will get fixed pytest (or at least significantly less flaky). |
petrasovaa
requested changes
Sep 29, 2026
cwhite911
force-pushed
the
r-sim-run-summary
branch
from
September 29, 2026 19:45
eacbb85 to
6ca2498
Compare
petrasovaa
reviewed
Sep 29, 2026
Add a -p flag and a format=plain|json option to r.sim.water and r.sim.sediment which print a summary of the run (walker counts, time step, iteration counts, mean velocity, mean Manning's n, mean source rate, early stop, and the names and simulated times of the written maps) to standard output. Without -p nothing is printed to standard output. Replace the three inconsistent history blocks in simlib with one helper whose labels are the same keys as the printed summary, and record the same values for every written map. Free the time series map names which were leaked before, and compute the simulated time of the final maps from the number of completed iterations rather than the loop counter. Convert stray printf calls in the walker loop to G_debug so standard output stays clean. Add pytest tests on the shared raster fixture, and document the keys and units in both manuals. Addresses OSGeo#7923. Seeds are left to OSGeo#7884. AI assisted.
Report time_step as the simulated time per iteration (the time step times the time coefficient) and drop time_coefficient, iterations_per_output, elevation_min and elevation_max. Rename walkers_active to walkers_remaining. Report transport_capacity and tlimit_erosion_deposition once at the top level of the sediment summary since they are written before the simulation starts. Emit null for a non-finite sediment time step instead of dropping the key. List only the given input maps in the history data source fields instead of formatting NULL names. Reuse the simulated time helper in the main loop, link parson only from the library, and add the SPDX header to summary.c. AI assisted.
…ests The number of iterations is rounded down, so the simulated time reached is slightly less than the requested duration. Reporting it as an integer hid that. Report it as a floating point number in the summary and in the history, and use the same value for the map timestamps. AI assisted.
cwhite911
force-pushed
the
r-sim-run-summary
branch
from
September 30, 2026 01:14
6ca2498 to
7ff32e3
Compare
petrasovaa
approved these changes
Sep 30, 2026
This was referenced Sep 30, 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.
Addresses #7923.
Summary
r.sim.waterandr.sim.sedimentgain a-pflag and aformat=plain|jsonoption which print a summary of the run to standard output after the last map is written.-prequests printing;formatonly chooses the shape, so without-pnothing goes to standard output regardless offormat(same asg.region). The same values are written to the history of thedepth,dischargeandsediment_fluxmaps under the same keys.Following the review feedback on the issue, the schema is flat and keeps only the setup and run values plus an
outputsarray. There is noregion,inputs,parameters,toolorschema_versionblock.{ "walkers_requested": 100000, "walkers_generated": 120000, "walkers_active": 112724, "duration": 1200, "simulated_time": 1199, "time_step": 0.26578203020127966, "time_coefficient": 4, "iterations_planned": 1128, "iterations_completed": 1128, "iterations_per_output": 564, "stopped_early": false, "elevation_min": 91.686592102050781, "elevation_max": 343.03448486328125, "mean_velocity": 9.4062040165270862, "mean_mannings_n": 0.050000000000000003, "mean_source_rate": 1.390000000000819e-05, "mean_infiltration": 0, "threads": 1, "outputs": [ { "simulated_time": 599, "timestamp": "10 minutes", "walkers_active": 113464, "depth": "depth.10", "discharge": "discharge.10", "error": null, "walkers": null }, { "simulated_time": 1199, "timestamp": "20 minutes", "walkers_active": 112724, "depth": "depth.20", "discharge": "discharge.20", "error": null, "walkers": null } ] }Top-level
walkers_activeandsimulated_timedescribe the end of the run; eachoutputsentry holds the values at the time its maps were written.r.sim.sedimentaddstime_step_sediment,velocity_maxandsigma_max, omitsmean_infiltration, and namestransport_capacity,tlimit_erosion_deposition,sediment_concentration,sediment_flux,erosion_depositionandwalkersin its singleoutputsentry.mean_mannings_nis the harmonic mean (inverse of the mean of 1/n) and isnullwhen undefined. Units are documented in the manuals, not in the key names.AI Summary
Changes
simlib/summary.ccollects one record peroutput_datacall and prints the JSON (viaG_json_*) or plainkey: valuelines.simlib/output.c(init.walkvsinit.walkers,rem. walkersvsremaining walkers, thetime-serietypo,mean mannmissing for depth,%evs%f) are replaced by one helper that writes identical lines to every map, labelled with the JSON keys. A small name helper removes the six duplicated time-series branches, and the time-series map names are now freed instead of leaked.main_looprecords an early stop and the number of completed iterations. The simulated time of the final maps is now computed from the completed iterations rather than the loop counter, which was one past the last iteration.printfcalls in the walker loop's out-of-bounds branch becomeG_debugso standard output stays clean for JSON.grad_checkstores the elevation range and, for sediment, the sediment time step, maximum sigma and maximum velocity onSetup..mdand.html).session_toolsfixture fromraster/conftest.pyand itsrows_rasterplane: key sets and values, time-series entries, an early-stop case, walker vector naming, plain output, empty stdout without-p, and history contents. 12 tests pass locally.Not in this PR
wall_timeis deferred since GRASS has no portable timer helper.Pre-existing behavior surfaced by the summary
Two things the summary makes visible that this PR does not change, since changing them would alter results and the reference tests:
time_coefficientis 4 whenever the water time step is used (so always forr.sim.water) and 1.25 when the sediment time step is smaller. The table in [Feat] r.sim.water, r.sim.sediment: addformat=jsonrun summary and normalize history metadata #7923 has this the other way round; the docs here follow the code.r.sim.sedimentcomputes the number of iterations from the water time step ingrad_checkbefore replacing it with the smaller sediment time step. When the sediment step governs, the run covers much less simulated time thanduration: on a fractal test surface a 10 minute run reportedsimulated_timeof 1 second. This looks like a long-standing bug worth a separate issue.Verification
Built with CMake (no warnings), pre-commit passes on all changed files. The gunittest suites need the NC dataset and are left to CI. Memory was checked with Apple's
leakson this arm64 Mac (valgrind and LeakSanitizer are unavailable there) against an unmodified build ofmain: leaked allocation counts match within noise for every case, and no reported stack passes through the new code.AI assisted.