Merge branch 'ImprovementsInTimeLoop' into 'master'

Dmitri Naumov · Dmitri Naumov · commit e42fe5fe79a3 · 2025-08-12T16:14:14.000+02:00
Small improvements in TimeStepAlgorithm

See merge request ogs/ogs!5391
diff --git a/NumLib/TimeStepping/Algorithms/EvolutionaryPIDcontroller.cpp b/NumLib/TimeStepping/Algorithms/EvolutionaryPIDcontroller.cpp
@@ -20,9 +20,10 @@
 
 namespace NumLib
 {
-std::tuple<bool, double> EvolutionaryPIDcontroller::next(
-    double const solution_error, int const /*number_iterations*/,
-    NumLib::TimeStep& /*timestep_previous*/, NumLib::TimeStep& timestep_current)
+double EvolutionaryPIDcontroller::next(double const solution_error,
+                                       int const /*number_iterations*/,
+                                       NumLib::TimeStep& /*timestep_previous*/,
+                                       NumLib::TimeStep& timestep_current)
 {
     const bool is_previous_step_accepted = timestep_current.isAccepted();
 
@@ -50,7 +51,7 @@ std::tuple<bool, double> EvolutionaryPIDcontroller::next(
             "\t or the simulation will be halted.",
             _tol, h_new);
 
-        return std::make_tuple(timestep_current.isAccepted(), h_new);
+        return h_new;
     }
 
     // step accepted.
@@ -60,46 +61,42 @@ std::tuple<bool, double> EvolutionaryPIDcontroller::next(
     {
         _e_n_minus1 = e_n;
 
-        return std::make_tuple(timestep_current.isAccepted(), _h0);
+        return _h0;
     }
-    else
-    {
-        const double h_n = timestep_current.dt();
-        double h_new = h_n;
+    const double h_n = timestep_current.dt();
+    double h_new = h_n;
 
-        if (e_n > zero_threshold)
+    if (e_n > zero_threshold)
+    {
+        if (_e_n_minus1 > zero_threshold)
         {
-            if (_e_n_minus1 > zero_threshold)
+            if (_e_n_minus2 > zero_threshold)
             {
-                if (_e_n_minus2 > zero_threshold)
-                {
-                    h_new = std::pow(_e_n_minus1 / e_n, _kP) *
-                            std::pow(_tol / e_n, _kI) *
-                            std::pow(
-                                _e_n_minus1 * _e_n_minus1 / (e_n * _e_n_minus2),
-                                _kD) *
-                            h_n;
-                }
-                else
-                {
-                    h_new = std::pow(_e_n_minus1 / e_n, _kP) *
-                            std::pow(_tol / e_n, _kI) * h_n;
-                }
+                h_new =
+                    std::pow(_e_n_minus1 / e_n, _kP) *
+                    std::pow(_tol / e_n, _kI) *
+                    std::pow(_e_n_minus1 * _e_n_minus1 / (e_n * _e_n_minus2),
+                             _kD) *
+                    h_n;
             }
             else
             {
-                h_new = std::pow(_tol / e_n, _kI) * h_n;
+                h_new = std::pow(_e_n_minus1 / e_n, _kP) *
+                        std::pow(_tol / e_n, _kI) * h_n;
             }
         }
+        else
+        {
+            h_new = std::pow(_tol / e_n, _kI) * h_n;
+        }
+    }
 
-        h_new =
-            limitStepSize(h_new, is_previous_step_accepted, timestep_current);
+    h_new = limitStepSize(h_new, is_previous_step_accepted, timestep_current);
 
-        _e_n_minus2 = _e_n_minus1;
-        _e_n_minus1 = e_n;
+    _e_n_minus2 = _e_n_minus1;
+    _e_n_minus1 = e_n;
 
-        return std::make_tuple(timestep_current.isAccepted(), h_new);
-    }
+    return h_new;
 }
 
 double EvolutionaryPIDcontroller::limitStepSize(
diff --git a/NumLib/TimeStepping/Algorithms/EvolutionaryPIDcontroller.h b/NumLib/TimeStepping/Algorithms/EvolutionaryPIDcontroller.h
@@ -64,10 +64,10 @@ class EvolutionaryPIDcontroller final : public TimeStepAlgorithm
     {
     }
 
-    std::tuple<bool, double> next(double solution_error,
-                                  int number_iterations,
-                                  NumLib::TimeStep& timestep_previous,
-                                  NumLib::TimeStep& timestep_current) override;
+    double next(double solution_error,
+                int number_iterations,
+                NumLib::TimeStep& timestep_previous,
+                NumLib::TimeStep& timestep_current) override;
 
     bool isSolutionErrorComputationNeeded() const override { return true; }
 
diff --git a/NumLib/TimeStepping/Algorithms/FixedTimeStepping.cpp b/NumLib/TimeStepping/Algorithms/FixedTimeStepping.cpp
@@ -213,15 +213,16 @@ FixedTimeStepping::FixedTimeStepping(double t0, double t_end, double dt)
     }
 }
 
-std::tuple<bool, double> FixedTimeStepping::next(
-    double const /*solution_error*/, int const /*number_iterations*/,
-    NumLib::TimeStep& /*ts_previous*/, NumLib::TimeStep& ts_current)
+double FixedTimeStepping::next(double const /*solution_error*/,
+                               int const /*number_iterations*/,
+                               NumLib::TimeStep& /*ts_previous*/,
+                               NumLib::TimeStep& ts_current)
 {
     // check if last time step
     if (ts_current.timeStepNumber() == _dt_vector.size() ||
         ts_current.current() >= end())
     {
-        return std::make_tuple(true, 0.0);
+        return 0.0;
     }
 
     double dt = _dt_vector[ts_current.timeStepNumber()];
@@ -230,7 +231,7 @@ std::tuple<bool, double> FixedTimeStepping::next(
         dt = end()() - ts_current.current()();
     }
 
-    return std::make_tuple(true, dt);
+    return dt;
 }
 
 bool FixedTimeStepping::areRepeatDtPairsValid(
diff --git a/NumLib/TimeStepping/Algorithms/FixedTimeStepping.h b/NumLib/TimeStepping/Algorithms/FixedTimeStepping.h
@@ -52,9 +52,9 @@ class FixedTimeStepping final : public TimeStepAlgorithm
                       std::vector<RepeatDtPair> const& repeat_dt_pairs,
                       std::vector<double> const& fixed_times_for_output);
 
-    std::tuple<bool, double> next(double solution_error, int number_iterations,
-                                  NumLib::TimeStep& ts_previous,
-                                  NumLib::TimeStep& ts_current) override;
+    double next(double solution_error, int number_iterations,
+                NumLib::TimeStep& ts_previous,
+                NumLib::TimeStep& ts_current) override;
 
     static bool areRepeatDtPairsValid(
         std::vector<RepeatDtPair> const& repeat_dt_pairs);
diff --git a/NumLib/TimeStepping/Algorithms/IterationNumberBasedTimeStepping.cpp b/NumLib/TimeStepping/Algorithms/IterationNumberBasedTimeStepping.cpp
@@ -58,49 +58,45 @@ IterationNumberBasedTimeStepping::IterationNumberBasedTimeStepping(
     }
 }
 
-std::tuple<bool, double> IterationNumberBasedTimeStepping::next(
-    double const /*solution_error*/, int const number_iterations,
-    NumLib::TimeStep& ts_previous, NumLib::TimeStep& ts_current)
+double IterationNumberBasedTimeStepping::next(double const /*solution_error*/,
+                                              int const number_iterations,
+                                              NumLib::TimeStep& ts_previous,
+                                              NumLib::TimeStep& ts_current)
 {
     _iter_times = number_iterations;
 
-    if (_previous_time_step_accepted)
+    if (ts_previous.isAccepted())
     {
         ts_previous = ts_current;
     }
 
     // confirm current time and move to the next if accepted
     if (ts_current.isAccepted())
     {
-        _previous_time_step_accepted = true;
-        return std::make_tuple(_previous_time_step_accepted,
-                               getNextTimeStepSize(ts_previous, ts_current));
+        ts_previous.setAccepted(true);
+        return getNextTimeStepSize(ts_previous, ts_current);
     }
-    else
-    {
-        double dt = getNextTimeStepSize(ts_previous, ts_current);
-        // In case it is the first time be rejected, re-computed dt again with
-        // current dt
-        if (std::abs(dt - ts_current.dt()) <
-            std::numeric_limits<double>::epsilon())
-        {
-            // time step was rejected, keep dt for the next dt computation.
-            ts_previous =  // essentially equal to _ts_prev.dt = _ts_current.dt.
-                TimeStep{ts_previous.previous(), ts_previous.previous() + dt,
-                         ts_previous.timeStepNumber()};
-            dt = getNextTimeStepSize(ts_previous, ts_current);
-        }
 
+    double dt = getNextTimeStepSize(ts_previous, ts_current);
+    // In case it is the first time be rejected, re-computed dt again with
+    // current dt
+    if (std::abs(dt - ts_current.dt()) < std::numeric_limits<double>::epsilon())
+    {
         // time step was rejected, keep dt for the next dt computation.
-        ts_previous =  // essentially equal to ts_previous.dt = _ts_current.dt.
+        ts_previous =  // essentially equal to _ts_prev.dt = _ts_current.dt.
             TimeStep{ts_previous.previous(), ts_previous.previous() + dt,
                      ts_previous.timeStepNumber()};
-        ts_current = TimeStep{ts_current.previous(), ts_current.previous() + dt,
-                              ts_current.timeStepNumber()};
-
-        _previous_time_step_accepted = false;
-        return std::make_tuple(_previous_time_step_accepted, dt);
+        dt = getNextTimeStepSize(ts_previous, ts_current);
     }
+
+    // time step was rejected, keep dt for the next dt computation.
+    ts_previous =  // essentially equal to ts_previous.dt = _ts_current.dt.
+        TimeStep{ts_previous.previous(), ts_previous.previous() + dt,
+                 ts_previous.timeStepNumber()};
+    ts_current = TimeStep{ts_current.previous(), ts_current.previous() + dt,
+                          ts_current.timeStepNumber()};
+
+    return dt;
 }
 
 double findMultiplier(
diff --git a/NumLib/TimeStepping/Algorithms/IterationNumberBasedTimeStepping.h b/NumLib/TimeStepping/Algorithms/IterationNumberBasedTimeStepping.h
@@ -97,9 +97,9 @@ class IterationNumberBasedTimeStepping final : public TimeStepAlgorithm
 
     ~IterationNumberBasedTimeStepping() override = default;
 
-    std::tuple<bool, double> next(double solution_error, int number_iterations,
-                                  NumLib::TimeStep& ts_previous,
-                                  NumLib::TimeStep& ts_current) override;
+    double next(double solution_error, int number_iterations,
+                NumLib::TimeStep& ts_previous,
+                NumLib::TimeStep& ts_current) override;
 
     bool isSolutionErrorComputationNeeded() const override { return true; }
 
@@ -130,7 +130,6 @@ class IterationNumberBasedTimeStepping final : public TimeStepAlgorithm
     /// The number of nonlinear iterations.
     int _iter_times = 0;
 
-    bool _previous_time_step_accepted = true;
     std::vector<double> const _fixed_times_for_output;
 };
 
diff --git a/NumLib/TimeStepping/Algorithms/TimeStepAlgorithm.h b/NumLib/TimeStepping/Algorithms/TimeStepAlgorithm.h
@@ -49,11 +49,11 @@ class TimeStepAlgorithm
     /// the next step
     /// \param ts_current the current time step used to compute the size of the
     /// next step
-    /// \return A step acceptance flag and the computed step size.
-    virtual std::tuple<bool, double> next(const double solution_error,
-                                          int number_iterations,
-                                          NumLib::TimeStep& ts_previous,
-                                          NumLib::TimeStep& ts_current) = 0;
+    /// \return the computed step size.
+    virtual double next(const double solution_error,
+                        int number_iterations,
+                        NumLib::TimeStep& ts_previous,
+                        NumLib::TimeStep& ts_current) = 0;
 
     /// Get a flag to indicate whether this algorithm needs to compute
     /// solution error. The default return value is false.
diff --git a/ProcessLib/TimeLoop.cpp b/ProcessLib/TimeLoop.cpp
@@ -328,11 +328,11 @@ std::pair<NumLib::TimeIncrement, bool> TimeLoop::computeTimeStepping(
         ppd.timestep_current.setAccepted(
             ppd.nonlinear_solver_status.error_norms_met);
 
-        auto [previous_step_accepted, timestepper_dt] = timestep_algorithm.next(
+        auto const timestepper_dt = timestep_algorithm.next(
             solution_error, ppd.nonlinear_solver_status.number_iterations,
             ppd.timestep_previous, ppd.timestep_current);
 
-        if (!previous_step_accepted)
+        if (!ppd.timestep_current.isAccepted())
         {
             // Not all processes have accepted steps.
             all_process_steps_accepted = false;
diff --git a/Tests/NumLib/TestFixedTimeStepping.cpp b/Tests/NumLib/TestFixedTimeStepping.cpp
@@ -104,7 +104,7 @@ TEST_F(NumLibFixedTimeStepping, next)
             NumLib::Time(0), NumLib::Time(t_initial), 0);
         for (auto const& expected_time_point : expected_time_points)
         {
-            auto [is_next, step_size] =
+            auto const step_size =
                 fixed_time_stepping.next(0.0 /* solution_error */,
                                          0 /* number_of_iterations */,
                                          ts_dummy,
@@ -166,7 +166,7 @@ TEST_F(NumLibFixedTimeStepping, next_StaticTest)
     NumLib::TimeStep ts_current(NumLib::Time(0), NumLib::Time(t_initial), 0);
     for (auto const& expected_time_point : expected_time_points)
     {
-        auto [is_next, step_size] =
+        auto const step_size =
             fixed_time_stepping.next(0.0 /* solution_error */,
                                      0 /* number_of_iterations */,
                                      ts_dummy,
diff --git a/Tests/NumLib/TestTimeSteppingEvolutionaryPIDcontroller.cpp b/Tests/NumLib/TestTimeSteppingEvolutionaryPIDcontroller.cpp
@@ -54,10 +54,10 @@ TEST(NumLibTimeStepping, testEvolutionaryPIDcontroller)
     int const number_iterations = 0;
     // 1st step
 
-    auto [step_accepted, timestepper_dt] = PIDStepper->next(
+    auto const timestepper_dt = PIDStepper->next(
         solution_error, number_iterations, previous_timestep, current_timestep);
 
-    ASSERT_TRUE(step_accepted);
+    ASSERT_TRUE(current_timestep.isAccepted());
     NumLib::updateTimeSteps(timestepper_dt, previous_timestep,
                             current_timestep);
 
@@ -73,9 +73,9 @@ TEST(NumLibTimeStepping, testEvolutionaryPIDcontroller)
 
     // e_n_minus1 is filled.
     solution_error = 1.0e-4;
-    auto [step_accepted1, timestepper_dt1] = PIDStepper->next(
-        solution_error, number_iterations, previous_timestep, current_timestep);
-    ASSERT_TRUE(step_accepted1);
+    PIDStepper->next(solution_error, number_iterations, previous_timestep,
+                     current_timestep);
+    ASSERT_TRUE(current_timestep.isAccepted());
     NumLib::updateTimeSteps(timestepper_dt, previous_timestep,
                             current_timestep);
     /// ts = PIDStepper->getTimeStep();
@@ -89,9 +89,9 @@ TEST(NumLibTimeStepping, testEvolutionaryPIDcontroller)
 
     // e_n_minus2 is filled.
     solution_error = 0.5e-3;
-    auto [step_accepted2, timestepper_dt2] = PIDStepper->next(
+    auto timestepper_dt2 = PIDStepper->next(
         solution_error, number_iterations, previous_timestep, current_timestep);
-    ASSERT_TRUE(step_accepted2);
+    ASSERT_TRUE(current_timestep.isAccepted());
     NumLib::updateTimeSteps(timestepper_dt2, previous_timestep,
                             current_timestep);
     /// ts = PIDStepper->getTimeStep();
@@ -104,9 +104,9 @@ TEST(NumLibTimeStepping, testEvolutionaryPIDcontroller)
     // If error > solution_error, step is rejected and new step size is
     // estimated.
     solution_error = 0.01;
-    auto [step_accepted3, timestepper_dt3] = PIDStepper->next(
-        solution_error, number_iterations, previous_timestep, current_timestep);
-    ASSERT_TRUE(!step_accepted3);
+    PIDStepper->next(solution_error, number_iterations, previous_timestep,
+                     current_timestep);
+    ASSERT_TRUE(!current_timestep.isAccepted());
     /// ts = PIDStepper->getTimeStep();
     h_new = current_timestep.dt();
     // No change in current_timestep.timeStepNumber
@@ -120,9 +120,9 @@ TEST(NumLibTimeStepping, testEvolutionaryPIDcontroller)
 
     // With e_n, e_n_minus1, e_n_minus2
     solution_error = 0.4e-3;
-    auto [step_accepted4, timestepper_dt4] = PIDStepper->next(
+    auto timestepper_dt4 = PIDStepper->next(
         solution_error, number_iterations, previous_timestep, current_timestep);
-    ASSERT_TRUE(step_accepted4);
+    ASSERT_TRUE(current_timestep.isAccepted());
     NumLib::updateTimeSteps(timestepper_dt4, previous_timestep,
                             current_timestep);
     /// ts = PIDStepper->getTimeStep();
diff --git a/Tests/NumLib/TestTimeSteppingIterationNumber.cpp b/Tests/NumLib/TestTimeSteppingIterationNumber.cpp
diff --git a/Tests/NumLib/TimeSteppingTestingTools.h b/Tests/NumLib/TimeSteppingTestingTools.h

Original file line number	Diff line number	Diff line change
`@@ -64,10 +64,10 @@ class EvolutionaryPIDcontroller final : public TimeStepAlgorithm`
`64`	`64`	`{`
`65`	`65`	`}`
`66`	`66`
`67`		`- std::tuple<bool, double> next(double solution_error,`
`68`		`- int number_iterations,`
`69`		`- NumLib::TimeStep& timestep_previous,`
`70`		`- NumLib::TimeStep& timestep_current) override;`
	`67`	`+ double next(double solution_error,`
	`68`	`+ int number_iterations,`
	`69`	`+ NumLib::TimeStep& timestep_previous,`
	`70`	`+ NumLib::TimeStep& timestep_current) override;`
`71`	`71`
`72`	`72`	`bool isSolutionErrorComputationNeeded() const override { return true; }`
`73`	`73`
Original file line number	Diff line number	Diff line change
`@@ -213,15 +213,16 @@ FixedTimeStepping::FixedTimeStepping(double t0, double t_end, double dt)`
`213`	`213`	`}`
`214`	`214`	`}`
`215`	`215`
`216`		`-std::tuple<bool, double> FixedTimeStepping::next(`
`217`		`- double const /solution_error/, int const /number_iterations/,`
`218`		`- NumLib::TimeStep& /ts_previous/, NumLib::TimeStep& ts_current)`
	`216`	`+double FixedTimeStepping::next(double const /solution_error/,`
	`217`	`+ int const /number_iterations/,`
	`218`	`+ NumLib::TimeStep& /ts_previous/,`
	`219`	`+ NumLib::TimeStep& ts_current)`
`219`	`220`	`{`
`220`	`221`	`// check if last time step`
`221`	`222`	`if (ts_current.timeStepNumber() == _dt_vector.size() \|\|`
`222`	`223`	`ts_current.current() >= end())`
`223`	`224`	`{`
`224`		`- return std::make_tuple(true, 0.0);`
	`225`	`+ return 0.0;`
`225`	`226`	`}`
`226`	`227`
`227`	`228`	`double dt = _dt_vector[ts_current.timeStepNumber()];`
`@@ -230,7 +231,7 @@ std::tuple<bool, double> FixedTimeStepping::next(`
`230`	`231`	`dt = end()() - ts_current.current()();`
`231`	`232`	`}`
`232`	`233`
`233`		`- return std::make_tuple(true, dt);`
	`234`	`+ return dt;`
`234`	`235`	`}`
`235`	`236`
`236`	`237`	`bool FixedTimeStepping::areRepeatDtPairsValid(`