Iteration number based time stepping #2318

endJunction · 2019-01-14T09:09:59Z

The main algorithm was implemented quite some time already, but there was no driver for it. Here it goes.

I'd suggest commit-wise review.

bilke · 2019-01-14T21:12:23Z

ProcessLib/RichardsFlow/Tests.cmake

+    EXECUTABLE_ARGS RichardsFlow_2d_small_iteration_adaptive_dt.prj
+    TESTER vtkdiff
+    REQUIREMENTS NOT OGS_USE_MPI
+    # No vtkdiff comparison here, because of the different file names for


Then I guess you have to remove TESTER vtkdiff-line.

wenqing

Looks good!

wenqing · 2019-01-15T09:06:49Z

NumLib/TimeStepping/Algorithms/IterationNumberBasedTimeStepping.cpp

-        dt = _min_ts;
-    else if (dt > _max_ts)
-        dt = _max_ts;
+    if (dt < _min_dt)


dt = std::max(dt, _min_dt);

wenqing · 2019-01-15T09:08:03Z

NumLib/TimeStepping/Algorithms/IterationNumberBasedTimeStepping.cpp

+    {
+        dt = _min_dt;
+    }
+    else if (dt > _max_dt)


dt = std::min(dt, _max_dt);

TomFischer · 2019-01-15T11:49:23Z

NumLib/TimeStepping/Algorithms/IterationNumberBasedTimeStepping.cpp

+      _min_dt(min_dt),
+      _max_dt(max_dt),
+      _initial_dt(initial_dt),
+      _max_iter(_iter_times_vector.empty() ? 0 : _iter_times_vector.back())


~~Sorry, I'm not so familiar with the time stepping. Could you give a simple and short example what data is stored in _iter_times_vector?~~ Docu is below.

TomFischer · 2019-01-15T11:58:02Z

NumLib/TimeStepping/CreateTimeStepper.cpp

+        "\n\tSingleStep,"
+        "\n\tFixedTimeStepping,"
+        "\n\tEvolutionaryPIDcontroller,",
+        "\n\t IterationNumberBasedTimeStepping\n",


Indentation of IterationNumberBasedTimeStepping is inconsistent with other time stepping schemas.

TomFischer

Looks good. ⏩

jbathmann · 2019-01-17T11:23:11Z

I did some tests with the setup attached to this post on this and found some points, which were not exactly clear to me:

The algorithm seems to fail if nonlinear_solvers/nonlinear_solver/max_iter == max(number_iterations), because last element of list does not seem to be taken into account
The algorithm seems to fail, if max_iterations (same as above) > max(number_iterations), because multiplier is set to 1
I needed some time to figure out, what number_iterations and multiplier tag mean. Maybe some comments on this in the .md files would be really nice. Or did I miss on finding the documentation?
-The fixed output times from the project file are not hit. This might be the reason the lack of output files for the fixed times?

Hope this helps?
testsetup.zip

Obligatory clang-format too. Choosing int over size_t for number of iterations -- nobody is going to wait that long anyway.

It is obviously adaptive.

It contains a boolean for met error norms and a number of iterations of the solver made. The number of iterations is needed for a timestepping scheme.

Same as the PID test.

Aside of the I/F changes, the expected result also changed. The first zero in nr_iterations is added, for the simulated first time step.

The iteration number is now set through the next() call.

Remove duplicate docu from the inherited functions.

Check vector sizes.

Otherwise the timestepper will repeat the second iteration of the first time step with the initial_dt and simulation will be aborted. In the current implementation the minimum dt will be taken for the second iteration.

Change the way a multiplier is computed, s.t. for a set of iterations and multipliers as: i = 5 8 10 m = 2 1 0.5 following multiplier is taken m(<5) = 2 m(5-7) = 2 m(8-9) = 1 m(>=10) = 0.5 This is easier to comprehend in the project files.

endJunction · 2019-01-21T13:21:16Z

As @jbathmann mentioned above there are some issues with the repeating of the time steps for certain combinations of number_of_iterations vector and the max_iter in the non-linear solver; the simulations stops, although it could have succeeded. Increasing the max_iter seems to resolve this kind of problems.

Real fix need a major rewrite of the time loop.

Please review from the "[doc] Add IterationNumberBasedTS prj tags docu." commit 'til the end, especially the changes in the ProcessLib/UncoupledProcessesTimeLoop.cpp

First removal force the convergence checks to be executed. Last two changes handle the t being (almost) equal to the end time.

wenqing

Looks good in these three commits.

wenqing · 2019-01-21T16:41:19Z

ProcessLib/UncoupledProcessesTimeLoop.cpp

@@ -431,7 +425,8 @@ double UncoupledProcessesTimeLoop::computeTimeStepping(
        }
        else
        {
-            if (t < _end_time)
+            if (t < _end_time || std::abs(t - _end_time) <
+                                     std::numeric_limits<double>::epsilon())


Just found a std function: std::islessequal.

Thanks, I didn't know that function. But from cppreference it seems that the only difference to operator<=(x, y) is that the floating point exceptions are not raised (and we do like exceptions).

wenqing · 2019-01-21T16:42:08Z

ProcessLib/UncoupledProcessesTimeLoop.cpp

@@ -451,7 +446,8 @@ double UncoupledProcessesTimeLoop::computeTimeStepping(
        }
        else
        {
-            if (t < _end_time)
+            if (t < _end_time || std::abs(t - _end_time) <
+                                     std::numeric_limits<double>::epsilon())


It can be replaced with std::islessequal.

TomFischer

⏩

ogsbot · 2020-06-19T12:57:09Z

OpenGeoSys development has been moved to GitLab.

See this pull request on GitLab.

bilke reviewed Jan 14, 2019

View reviewed changes

wenqing approved these changes Jan 15, 2019

View reviewed changes

TomFischer reviewed Jan 15, 2019

View reviewed changes

TomFischer approved these changes Jan 15, 2019

View reviewed changes

bilke force-pushed the master branch from 2a347a0 to a016b45 Compare January 17, 2019 07:09

endJunction added the WIP 🏗️ label Jan 18, 2019

endJunction and others added 22 commits January 20, 2019 22:14

[NL] Simplify creation of timestepper.

7f9003c

[NL] Small refactorings, renames, braces, types.

18403ae

Obligatory clang-format too. Choosing int over size_t for number of iterations -- nobody is going to wait that long anyway.

[NL] Rename IterationNumberBasedTimeStepping.

3788375

It is obviously adaptive.

[NL] Implement createNumberBasedTimeStepping().

ac90a1e

Use int for number of iterations.

060267b

[NL] Solvers now return a NonlinearSolverStatus.

4deb00f

It contains a boolean for met error norms and a number of iterations of the solver made. The number of iterations is needed for a timestepping scheme.

[NL] Pass number of iterations to time step algo.

a79b0b7

[NL] Add isSolutionErrorComputationNeeded = true

5893269

[T] Rename RichardsFlow PID adaptive dt project.

d3ae112

[T] New iteration nr. based dt; RichardsFlow

b2cee42

Same as the PID test.

[T] Drop unneeded tester specification.

f0de2c5

[NL] Use std::min/max instead of if's.

c93c441

[T/NL] Updates for the changes timestepper I/F.

315951f

[T/NL] Change TimeSteppingIterationNumberBased2.

7813e4c

Aside of the I/F changes, the expected result also changed. The first zero in nr_iterations is added, for the simulated first time step.

[NL] Remove unnecessary setIterationNumber().

869c342

The iteration number is now set through the next() call.

[NL] TimeStepping; Cleanup comments and docu.

47d12ef

Remove duplicate docu from the inherited functions.

[doc] Add IterationNumberBasedTS prj tags docu.

bd4d284

[NL] INBTS; Add strict constructor checks.

f78423b

Check vector sizes.

[NL] INBTS; Extract findMultiplier().

81eed6e

[NL] INBTS; Special handling of first ts, first it

80fde82

Otherwise the timestepper will repeat the second iteration of the first time step with the initial_dt and simulation will be aborted. In the current implementation the minimum dt will be taken for the second iteration.

[NL] NS; Avoid increment of the 'iteration'.

8dfd301

[PL] Time loop. Add dt information on output.

4706681

endJunction removed the WIP 🏗️ label Jan 21, 2019

endJunction added 3 commits January 21, 2019 16:04

[NL] INBTS; Slightly simplify findMultiplier().

7e3e98d

[PL] TS: Change handling when reaching end time.

735f2f4

First removal force the convergence checks to be executed. Last two changes handle the t being (almost) equal to the end time.

[PL] TS; Remove an abort for repeated ts size.

d59bc2a

wenqing reviewed Jan 21, 2019

View reviewed changes

TomFischer approved these changes Jan 22, 2019

View reviewed changes

endJunction merged commit 83cb86a into ufz:master Jan 22, 2019

endJunction deleted the TimeStepping branch January 22, 2019 11:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Iteration number based time stepping #2318

Iteration number based time stepping #2318

endJunction commented Jan 14, 2019

bilke Jan 14, 2019

wenqing left a comment

wenqing Jan 15, 2019

wenqing Jan 15, 2019

TomFischer Jan 15, 2019 •

edited

Loading

TomFischer Jan 15, 2019

TomFischer left a comment

jbathmann commented Jan 17, 2019 •

edited

Loading

endJunction commented Jan 21, 2019

wenqing left a comment

wenqing Jan 21, 2019 •

edited

Loading

endJunction Jan 21, 2019

wenqing Jan 22, 2019

wenqing Jan 21, 2019 •

edited

Loading

TomFischer left a comment

ogsbot commented Jun 19, 2020

Iteration number based time stepping #2318

Iteration number based time stepping #2318

Conversation

endJunction commented Jan 14, 2019

bilke Jan 14, 2019

Choose a reason for hiding this comment

wenqing left a comment

Choose a reason for hiding this comment

wenqing Jan 15, 2019

Choose a reason for hiding this comment

wenqing Jan 15, 2019

Choose a reason for hiding this comment

TomFischer Jan 15, 2019 • edited Loading

Choose a reason for hiding this comment

TomFischer Jan 15, 2019

Choose a reason for hiding this comment

TomFischer left a comment

Choose a reason for hiding this comment

jbathmann commented Jan 17, 2019 • edited Loading

endJunction commented Jan 21, 2019

wenqing left a comment

Choose a reason for hiding this comment

wenqing Jan 21, 2019 • edited Loading

Choose a reason for hiding this comment

endJunction Jan 21, 2019

Choose a reason for hiding this comment

wenqing Jan 22, 2019

Choose a reason for hiding this comment

wenqing Jan 21, 2019 • edited Loading

Choose a reason for hiding this comment

TomFischer left a comment

Choose a reason for hiding this comment

ogsbot commented Jun 19, 2020

TomFischer Jan 15, 2019 •

edited

Loading

jbathmann commented Jan 17, 2019 •

edited

Loading

wenqing Jan 21, 2019 •

edited

Loading

wenqing Jan 21, 2019 •

edited

Loading