[wenzeslaus]

With the -t flag, r.sim.water writes the series maps every so many iterations and names them by the simulated time rounded to minutes. The write times drift away from the output steps, so two writes can get the same name and the first map is overwritten, the map for the last step is often missing, and when the time step is longer than output_step no map is written at all while the tool succeeds.

This writes each output step at the iteration closest to its time and names and timestamps the maps by that time. The series always ends with the maps named by the duration. The depth and discharge values do not change and neither do the outputs without -t. It keeps the run summary (#7972) consistent.

Details (AI generated)

• What was wrong: the tool wrote a series map when i % iterout == 0 with iterout = (int)(output_step / time step) and named it by (int)(i * time step / 60 + 0.5) minutes. The truncation makes the write times drift from j * output_step, which leads to duplicate names (the earlier map is overwritten), skipped or odd minutes with an even output_step, a missing last step, and no maps at all when iterout truncates to 0 (for example mintimestep=17.5, time step 70 s, output_step=1), all with exit code 0.
• Change: output step j is written at the iteration whose simulated time is closest to j * output_step and is named and timestamped by j * output_step minutes with the existing name format. A map holds the state at most half a time step before or after the time in its name. output_data() gets the name time as a separate argument next to the simulated time. Setup.iterout is removed. The OpenMP region is untouched.
• Why the closest iteration: the time step comes from the cell size and the mean velocity (or mintimestep), so iterations usually do not fall on the output times. Writing a step at the first iteration at or after its time would put every map up to one time step late. With a time step of 59 s and 1-minute steps, the map named 01 would hold the state at 118 s; with the closest iteration it holds the state at 59 s. Shortening the time step to make the times fit would change the results of -t runs compared to runs without -t.
• Semantics decisions:
  • (a) Time step longer than output_step: one iteration is the closest one to several step times. It writes once, named by the step closest to it, and the tool warns once at setup that the time step is longer than output_step and that output steps may be skipped. The warning is not printed for output_step of zero or less, where only the final maps are written. With 1-minute steps, the series is 01, 02, 04 to 10 for a 70 s time step, 02, 04, 06, 08, 10 for 120 s and 03, 06, 09, 10 for 180 s. A step past the duration is never written, which matters when the duration is not a multiple of output_step.
  • (b) Final map: the series always ends with maps named by duration, written at the end of the run, when the duration is not a multiple of output_step (duration 10, step 3 gives 03, 06, 09, 10) and when all walkers left the domain early (the series then ends with the duration map right after the last written step). When the last iteration is the closest one to the duration, the duration maps come from the same rule and nothing extra is written. The duration map can have the same state as the step before it when both come from the last iteration.
  • (c) Outputs without -t: unchanged, including their timestamp, which still uses the simulated time rounded to minutes. The mismatch between the write time (miter * dt) and the timestamp ((miter + 1) * dt) was already fixed on main by the run summary (r.sim: Add -p flag with format=json run summary and normalize history #7972), which computes both from the completed iterations.
  • (d) Walkers and observation points: walker vector maps (walkers_output) used the same iterout test and now follow the new schedule (walkers_01, ...). Observation points are logged every iteration and did not use iterout, so they are unchanged.
  • (e) Run summary: there is no output interval field in the summary. outputs[].simulated_time stays the time the maps were written and outputs[].timestamp is now the time the maps are named by, so in a series the timestamp is a multiple of output_step and the simulated time is within half a time step of it (the final duration map can be up to one time step earlier). The docs of both fields and of outputs are updated. The example summary in the manual needs no change: with its time step of 1.063 s, the iteration closest to 10 minutes is iteration 564 at 599.6 s, which is the one the example already shows.
• r.sim.sediment: it has no -t flag (settings.ts is always false) and its output_step only affects a message, so its outputs go through the unchanged non-series path. Its testsuite passes and its manual needs no change.
• Verification:
  • Outputs without -t on the nc_spm test region (10 m, man_value=0.1, nprocs=1): depth and discharge fcell, cell, colr and cell_misc files are byte-identical to main, the timestamps are the same and the walker vector points are identical.
  • With the fix, the final series map is byte-identical to the non-series map of the same run, so series values are unchanged and only when and under which name maps are written changes.
  • On the nc_spm test region with default settings and man_value=0.1, main writes .09 twice and no .10 with output_step=1. With the fix it writes .01 to .10, each once.
  • test_complex in the gunittest suite writes a 5-minute series for 15 minutes. Its .15 reference did not need to change: on main, the 15-minute map was written at the last iteration (1731, at 899.6 s), and with the fix it is the final duration map written at the same iteration (the maps are byte-identical). The 5 and 10 minute maps can come from a neighboring iteration, but the suite does not compare them.
• Tests run: the r.sim.water pytest suite (61 passed, 22 s) and the r.sim.sediment pytest suite (6 passed), after merging main, the r.sim.water gunittest suite with nc_spm (5 tests, 1 skipped as before, OK) and the r.sim.sediment gunittest suite (1 test, OK). All five new tests fail on main and pass with the fix.

Reproducer: 500 m x 500 m region at 10 m, elevation row() * 0.1, man_value=0.1, duration=10, output_step=1, random_seed=1, nprocs=1, time step 10.17 s.

write	main: map	main: simulated time	this PR: map	this PR: simulated time
1	dep.01	50.9 s	dep.01	61.0 s
2	dep.02	101.7 s	dep.02	122.1 s
3	dep.03	152.6 s	dep.03	183.1 s
4	dep.03 (overwrites)	203.5 s	dep.04	244.1 s
5	dep.04	254.3 s	dep.05	295.0 s
6	dep.05	305.2 s	dep.06	356.0 s
7	dep.06	356.0 s	dep.07	417.1 s
8	dep.07	406.9 s	dep.08	478.1 s
9	dep.08	457.8 s	dep.09	539.2 s
10	dep.08 (overwrites)	508.6 s	dep.10 (end of run)	590.0 s
11	dep.09	559.5 s

Tests

New file raster/r.sim/r.sim.water/tests/r_sim_water_time_series_test.py with five tests, one of them with three cases. They run on the reproducer slope narrowed to 5 x 50 cells (a module fixture), where the time step is 10.69 s and main writes dep.04 twice.

Time: the file takes 1.1 s on its own. The r.sim.water pytest suite goes from 21.0 s (54 tests) to 22.1 s (61 tests), an increase of about 5%. All tests in the file fail with the old schedule.

• test_every_output_step_written_once (0.2 s):
  • Why: it is the reproducer case, a time step which does not divide 60 s.
  • Checks: the depth maps in the summary and in the mapset are exactly dep.01 to dep.10, the summary and raster timestamps are the step minutes, each intermediate map is written within half a time step of the time in its name, the last one at the end of the run, and the walker maps are exactly walkers_01 to walkers_10.
  • Catches: duplicate names and overwritten maps, drifting names, the missing last step and walker maps on a different schedule.
• test_final_step_named_by_duration (0.1 s): output_step=3 must give dep.03, dep.06, dep.09 and dep.10. Catches a series without the duration map when the duration is not a multiple of the step.
• test_time_step_longer_than_output_step (three cases, 0.35 s): time steps of 70 s, 120 s and 180 s with 1-minute steps. With 70 s, the iteration ending at 210 s is equally close to 3 and 4 minutes and writes the later one (series 01, 02, 04 to 10). With 120 s and 180 s, the iterations fall exactly on every second and third step (series 02, 04, 06, 08, 10 and 03, 06, 09, 10). Each case also checks the warning. Catches the old no-maps-at-all case, several maps from one iteration, and maps named by a later step than the one the iteration falls on.
• test_no_output_step_past_duration (0.1 s): with a time step of 160 s, a duration of 11 minutes and 3-minute steps, the last iteration ends at 640 s and the step closest to it is 12 minutes. Checks the series 03, 06, 09, 11. Catches maps named past the duration; it fails without the check against the duration.
• test_final_step_after_early_stop (under 0.1 s): with man_value=0.002, all walkers leave the slope before the first minute, so the series must still end with dep.02 timestamped 2 minutes. On main, no map is written.

Changed existing tests:

• test_json_summary_time_series now checks the timestamps against the step minutes instead of the rounded simulated time, since that is the documented meaning now. It passes on main and with the fix.
• test_duration_affects_time_series_progression keeps its assertions. Its docstring and comment said the final 20-minute map depends on walkers surviving, which is no longer true, and are updated.

Squash-merge commit message

Title:

r.sim: Write time series maps at their output step times

Body:

With -t, r.sim.water wrote the series maps every (int)(output_step /
time step) iterations and named them by the simulated time rounded to
whole minutes. The truncated interval made the write times drift away
from the output steps, so two writes could round to the same minute and
the first map was overwritten, names could skip a minute or land on odd
minutes with an even output_step, and the last step was often missing.
When the time step was longer than output_step, the interval was zero
and no map was written at all while the tool still succeeded.

Write output step j at the iteration whose simulated time is closest to
j times output_step, and name and timestamp the maps by that time. The
time step comes from the cell size and the mean velocity, so iterations
usually do not fall on the output times. When one iteration is the
closest one to several steps, write only the step closest to it and
warn that the time step is longer than output_step. Do not write steps
past the duration. Always end the series with maps named by the
duration which hold the state at the end of the run, also when the
duration is not a multiple of output_step or when the run stopped
early. Walker vector maps follow the same schedule.

For users, each output step now appears once, named and timestamped by
a multiple of output_step, and the maps named by the duration are always
present. A series map holds the state at most half a time step before
or after the time in its name instead of up to several minutes before
it. The depth and discharge values are unchanged and so are the outputs
without -t. In the run summary, simulated_time of an output is the time
the maps were written and timestamp the time they are named by. New
tests check the names, timestamps and write times of the series, a
duration which is not a multiple of output_step, time steps longer
than output_step including its multiples, a step past the duration, and
an early stop.

Written with the help of Claude Code.

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