Fix indexing issue for output monitoring

include/bout/solver.hxx

@@ -451,19 +451,19 @@ protected:
   ///
   /// There are two important things to note about how \p iter is
   /// passed along to each monitor:
-  /// - The solvers all start their iteration numbering from zero, so the
-  ///   initial state is calculated at \p iter = -1
+  /// - The initial state is written at \p iter = 0, and solver output
+  ///   steps are numbered from 1 to NOUT
   /// - Secondly, \p iter is passed along to each monitor *relative to
   ///   that monitor's period*
   ///
   /// In practice, this means that each monitor is called like:
   ///
   ///     monitor->call(solver, simulation_time,
-  ///                   ((iter + 1) / monitor->period) - 1,
+  ///                   iter / monitor->period,
   ///                   NOUT / monitor->period);
   ///
-  /// e.g. for a monitor with period 10, passing \p iter = 9 will
-  /// result in it being called with a value of `(9 + 1)/10 - 1 == 0`
+  /// e.g. for a monitor with period 10, passing \p iter = 10 will
+  /// result in it being called with a value of `10/10 == 1`
   int call_monitors(BoutReal simtime, int iter, int NOUT);
 
   /// Should timesteps be monitored?

manual/sphinx/user_docs/time_integration.rst

@@ -1278,7 +1278,9 @@ implement the outputMonitor method of PhysicsModel::
     int outputMonitor(BoutReal simtime, int iter, int nout)
 
 The first input is the current simulation time, the second is the output
-number, and the last is the total number of outputs requested.
+number, and the last is the total number of outputs requested. If an initial
+dump is written, it is output number ``0``. Solver output steps are numbered
+from ``1`` to ``nout``, so ``iter == nout`` indicates the final output.
 This method is called by a monitor object PhysicsModel::modelMonitor, which
 writes the restart files at the same time. You can change the frequency at which
 the monitor is called by calling, in PhysicsModel::init::
@@ -1303,7 +1305,9 @@ returns an int::
 
 The first input is the solver object, the second is the current
 simulation time, the third is the output number, and the last is the
-total number of outputs requested. To get the solver to call this
+total number of outputs requested. As for ``outputMonitor()``, output number
+``0`` is reserved for the initial dump when it is written, and solver output
+steps are numbered from ``1`` to ``NOUT``. To get the solver to call this
 function every output time, define a `MyOutputMonitor` object as a member of your
 PhysicsModel::
 

src/solver/impls/adams_bashforth/adams_bashforth.cxx

@@ -355,7 +355,7 @@ int AdamsBashforthSolver::run() {
   [[maybe_unused]] int nwasted = 0;
   [[maybe_unused]] int nwasted_following_fail = 0;
 
-  for (int s = 0; s < getNumberOutputSteps(); s++) {
+  for (int s = 1; s <= getNumberOutputSteps(); s++) {
     BoutReal target = simtime + getOutputTimestep();
 
     bool running = true;

src/solver/impls/arkode/arkode.cxx

@@ -510,7 +510,7 @@ int ArkodeSolver::run() {
     throw BoutException("ArkodeSolver not initialised\n");
   }
 
-  for (int i = 0; i < getNumberOutputSteps(); i++) {
+  for (int i = 1; i <= getNumberOutputSteps(); i++) {
 
     /// Run the solver for one output timestep
     simtime = run(simtime + getOutputTimestep());

src/solver/impls/cvode/cvode.cxx

@@ -489,7 +489,7 @@ int CvodeSolver::run() {
     throw BoutException("CvodeSolver not initialised\n");
   }
 
-  for (int i = 0; i < getNumberOutputSteps(); i++) {
+  for (int i = 1; i <= getNumberOutputSteps(); i++) {
 
     /// Run the solver for one output timestep
     simtime = run(simtime + getOutputTimestep());

src/solver/impls/euler/euler.cxx

@@ -66,7 +66,7 @@ int EulerSolver::init() {
 
 int EulerSolver::run() {
 
-  for (int s = 0; s < getNumberOutputSteps(); s++) {
+  for (int s = 1; s <= getNumberOutputSteps(); s++) {
     BoutReal target = simtime + getOutputTimestep();
 
     bool running = true;

src/solver/impls/ida/ida.cxx

@@ -236,7 +236,7 @@ int IdaSolver::run() {
     throw BoutException("IdaSolver not initialised\n");
   }
 
-  for (int i = 0; i < getNumberOutputSteps(); i++) {
+  for (int i = 1; i <= getNumberOutputSteps(); i++) {
 
     /// Run the solver for one output timestep
     simtime = run(simtime + getOutputTimestep());

src/solver/impls/imex-bdf2/imex-bdf2.cxx

@@ -751,7 +751,7 @@ int IMEXBDF2::run() {
 
   int internalCounter = 0; // Cumulative number of successful internal iterations
 
-  for (int s = 0; s < getNumberOutputSteps(); s++) {
+  for (int s = 1; s <= getNumberOutputSteps(); s++) {
     BoutReal cumulativeTime = 0.;
     int counter = 0; // How many iterations in this output step
 

src/solver/impls/petsc/petsc.cxx

@@ -234,7 +234,7 @@ PetscErrorCode PetscMonitor(TS ts, PetscInt UNUSED(step), PetscReal t, Vec X, vo
     s->load_vars(const_cast<BoutReal*>(x));
     PetscCall(VecRestoreArrayRead(interpolatedX, &x));
 
-    if (s->call_monitors(output_time, i++, s->getNumberOutputSteps()) != 0) {
+    if (s->call_monitors(output_time, ++i, s->getNumberOutputSteps()) != 0) {
       PetscFunctionReturn(1);
     }
 

src/solver/impls/power/power.cxx

@@ -45,7 +45,7 @@ int PowerSolver::run() {
   // Make sure that f0 has a norm of 1
   divide(f0, norm(f0));
 
-  for (int s = 0; s < getNumberOutputSteps(); s++) {
+  for (int s = 1; s <= getNumberOutputSteps(); s++) {
 
     load_vars(std::begin(f0));
     run_rhs(curtime);

src/solver/impls/pvode/pvode.cxx

@@ -294,7 +294,7 @@ int PvodeSolver::run() {
     throw BoutException("PvodeSolver not initialised\n");
   }
 
-  for (int i = 0; i < getNumberOutputSteps(); i++) {
+  for (int i = 1; i <= getNumberOutputSteps(); i++) {
 
     /// Run the solver for one output timestep
     simtime = run(simtime + getOutputTimestep());

src/solver/impls/rk3-ssp/rk3-ssp.cxx

@@ -61,7 +61,7 @@ int RK3SSP::init() {
 
 int RK3SSP::run() {
 
-  for (int s = 0; s < getNumberOutputSteps(); s++) {
+  for (int s = 1; s <= getNumberOutputSteps(); s++) {
     BoutReal target = simtime + getOutputTimestep();
 
     BoutReal dt;

src/solver/impls/rk4/rk4.cxx

@@ -73,7 +73,7 @@ int RK4Solver::init() {
 }
 
 int RK4Solver::run() {
-  for (int s = 0; s < getNumberOutputSteps(); s++) {
+  for (int s = 1; s <= getNumberOutputSteps(); s++) {
     BoutReal target = simtime + getOutputTimestep();
 
     BoutReal dt;

src/solver/impls/rkgeneric/rkgeneric.cxx

@@ -80,7 +80,7 @@ void RKGenericSolver::resetInternalFields() {
 }
 
 int RKGenericSolver::run() {
-  for (int s = 0; s < getNumberOutputSteps(); s++) {
+  for (int s = 1; s <= getNumberOutputSteps(); s++) {
     BoutReal target = simtime + getOutputTimestep();
 
     BoutReal dt;

src/solver/impls/slepc/slepc.cxx

@@ -579,7 +579,7 @@ void SlepcSolver::monitor(PetscInt its, PetscInt nconv, PetscScalar eigr[],
   static bool first = true;
   if (eigenValOnly && first) {
     first = false;
-    resetIterationCounter();
+    resetIterationCounter(1);
   }
 
   // Temporary eigenvalues, converted from the SLEPc eigenvalues
@@ -701,7 +701,7 @@ void SlepcSolver::analyseResults() {
   output << "Converged eigenvalues :\n"
             "\tIndex\tSlepc eig (mag.)\t\t\tBOUT eig (mag.)\n";
 
-  resetIterationCounter();
+  resetIterationCounter(1);
 
   // Declare and create vectors to store eigenfunctions
   Vec vecReal, vecImag;

src/solver/impls/snes/snes.cxx

@@ -886,7 +886,7 @@ int SNESSolver::run() {
 
   BoutReal target = simtime;
   recent_failure_rate = 0.0;
-  for (int s = 0; s < getNumberOutputSteps(); s++) {
+  for (int s = 1; s <= getNumberOutputSteps(); s++) {
     target += getOutputTimestep();
 
     bool looping = true;

src/solver/impls/split-rk/split-rk.cxx

@@ -95,7 +95,7 @@ int SplitRK::init() {
 
 int SplitRK::run() {
 
-  for (int step = 0; step < getNumberOutputSteps(); step++) {
+  for (int step = 1; step <= getNumberOutputSteps(); step++) {
     // Take an output step
 
     BoutReal target = simtime + getOutputTimestep();

tests/unit/solver/test_fakesolver.hxx

@@ -19,6 +19,13 @@ public:
     if ((*options)["throw_run"].withDefault(false)) {
       throw BoutException("Deliberate exception in FakeSolver::run");
     }
+    if ((*options)["call_final_monitor"].withDefault(false)) {
+      const int ret =
+          call_monitors(simtime, getNumberOutputSteps(), getNumberOutputSteps());
+      if (ret != 0) {
+        return ret;
+      }
+    }
     return (*options)["fail_run"].withDefault(0);
   }
   bool run_called{false};

tests/unit/solver/test_solver.cxx

@@ -971,6 +971,26 @@ TEST_F(SolverTest, BasicSolve) {
   EXPECT_TRUE(solver.run_called);
 }
 
+TEST_F(SolverTest, SolveCleansUpMonitorsAtFinalIteration) {
+  Options options;
+  options["call_final_monitor"] = true;
+  FakeSolver solver{&options};
+
+  StrictMock<MockMonitor> monitor;
+  solver.addMonitor(&monitor);
+
+  NiceMock<MockPhysicsModel> model{};
+  solver.setModel(&model);
+
+  Options::cleanup();
+
+  EXPECT_CALL(monitor, call(_, _, 0, nout));
+  EXPECT_CALL(monitor, call(_, _, nout, nout));
+  EXPECT_CALL(monitor, cleanup());
+
+  solver.solve(nout, timestep);
+}
+
 TEST_F(SolverTest, GetRunID) {
   Options options;
   FakeSolver solver{&options};