Make CFL condition the upper limit for time steps in explicit transport solver (#170) · Issues · dorie / dorie

Make CFL condition the upper limit for time steps in explicit transport solver

Updated Issue

Following #170 (comment 39668), the CFL condition only serves as upper limit for time steps in an explicit time step scheme. It should not set the actual time step, as originally proposed.

@sospinar:

The idea of the CFL condition is to ensure stable conditions for the explicit solve, not to solve in fewer time steps, hence, I consider that suggesting it to the time step controller is the correct approach here.

@lriedel:

Alright, I perceived the CFL condition as the only criterion for selecting a time step, following the current implementation. I'm sorry I did not notice that when we ported the solver to the new time step controller 😕

So you propose to increase the time step after every successful step with the timestepIncreaseFactor and only use the CFL criterion as upper limit? Works for me. As you state, this gives more freedom to the user in terms of time step selection. I'll adjust !179 (merged) accordingly.

Original Issue

When setting the next time step with an explicit time step scheme, the transport solver may choose a time step which advances further than the Richards model has progressed.

Summary

The TimeStepController in the transport model always stores the final simulation end time. It can thus only ensure that the time step does not propagate past this value. However, in a coupled simulation, the transport model must not "overtake" the Richards model. This is first ensured by "suggesting" the Richards time step to the transport model. But with an explicit time step scheme, we let the CFL condition decide on the time step. In particular, it may choose a larger one than the current one, which is generally fine. But as the TimeStepController does not store the current Richards model time, at cannot re-adjust the time step to this time.

Steps to reproduce

Produce a simulation where the CFL condition leads to a higher time step than the initial time step of the Richards model. See input files below.

What is the current bug behaviour?

Transport model with explicit time step scheme can choose a time step that overtakes the Richards model, leading to an error thrown by the grid function container.

What is the expected correct behaviour?

Transport model time steps are always limited by the current time of the Richards model.

Relevant logs, screenshots, files...?

Anything that helps reproducing the bug

Expand to see logs

root@abc565c71a3a:/mnt# dorie run config.ini 
[15:01:00.333 I]  Starting DORiE
[15:01:00.333 I]  Reading configuration file: config.ini
[15:01:00.337 I]  Creating a rectangular grid in 2D
[15:01:00.339 I] [RIC]  Loading parameter data file: richards_param.yml
[15:01:00.343 I] [RIC]  Loading boundary condition data file: gravity_flow.yml
[15:01:00.351 I] [RIC]  Setup complete
[15:01:00.354 I] [SOL]  Loading parameter data file: transport_param.yml
[15:01:00.358 I] [SOL]  Loading boundary condition data file: solute.yml
[15:01:00.361 D] [SOL]    Filling gaps in boundary condition specs with default boundary condition
[15:01:00.365 D] [SOL]  Creating initial condition of type: analytic
[15:01:00.365 D] [SOL]  Creating a VTK writer with subsampling level: 0
[15:01:00.367 D] [SOL]  Setting up grid operators and solvers
[15:01:00.367 D] [SOL]    Total number of DOF: 400
[15:01:00.367 I] [SOL]  Setup complete
[15:01:00.367 I] [CPL]  Setup complete
[15:01:00.408 I] [RIC]  Time Step 0: 0.00e+00 + 1.00e+03 -> 1.00e+03
[15:01:00.429 I] [SOL]  Time Step 0:
[15:01:00.429 W] [SOL]  Time step '3.6862e+06' incompatible to chosen limits. Adjusting automatically
[15:01:00.429 D] [SOL]    Time 0.00e+00 + 1.00e+02 -> 1.00e+02
[15:01:00.457 I] [SOL]  Time Step 1:
[15:01:00.458 W] [SOL]  Time step '3.6862e+06' incompatible to chosen limits. Adjusting automatically
[15:01:00.458 D] [SOL]    Time 1.00e+02 + 1.00e+05 -> 1.00e+05
[15:01:00.460 D] [SOL]    Solver not converged
[15:01:00.460 E] [SOL]    Unexpected error in solver: Dune::InvalidStateException [check_in_range_time:/opt/dune/dorie/dune/dorie/common/grid_function_container.hh:473]: time above the range (0,1000) of the grid function container!
[15:01:00.461 C]  Aborting DORiE after exception: Dune::Exception [step:/opt/dune/dorie/dune/dorie/model/transport/transport.cc:220]: Critical error in linear solver
Error while running DORiE executable: /opt/dune/dorie/build-cmake/dune/dorie/transport_d2_r0_t0

Reproducing input

Do you have input files reproducing the problem? Insert them here:

Input data
Simulation Case	Solute pulse into sand in gravity flow
PFG config file
Grid mapping file
GMSH grid file
Boundary Condition file	gravity_flow.yml solute.yml
Parameterization file	richards_param.yml transport_param.yml
Run config file	config.ini

Ideas how to fix this?

Make the Transport solver store its global simulation end time separately. Add a new method with which the ModelRichardsTransportCoupling can propagate the Richards model time to the transport TimeStepController.

## Updated Issue
Following https://ts-gitlab.iup.uni-heidelberg.de/dorie/dorie/issues/170#note_39668, the CFL condition only serves as upper limit for time steps in an explicit time step scheme. It should *not* set the actual time step, as originally proposed.

@sospinar:
> The idea of the CFL condition is to ensure stable conditions for the explicit solve, not to solve in fewer time steps, hence, I consider that suggesting it to the time step controller is the correct approach here.

@lriedel:
>  Alright, I perceived the CFL condition as the only criterion for selecting a time step, following the current implementation. I'm sorry I did not notice that when we ported the solver to the new time step controller :confused:
> 
> So you propose to increase the time step after every successful step with the `timestepIncreaseFactor` and only use the CFL criterion as upper limit? Works for me. As you state, this gives more freedom to the user in terms of time step selection. I'll adjust !179 accordingly.

## Original Issue
When [setting the next time step](https://ts-gitlab.iup.uni-heidelberg.de/dorie/dorie/blob/master/dune/dorie/model/transport/transport.cc#L167) with an explicit time step scheme, the transport solver may choose a time step which advances further than the Richards model has progressed.

### Summary
The `TimeStepController` in the transport model always stores the final simulation end time. It can thus only ensure that the time step does not propagate past this value. However, in a coupled simulation, the transport model must not "overtake" the Richards model. This is first ensured by "suggesting" the Richards time step to the transport model. But with an explicit time step scheme, we let the CFL condition decide on the time step. In particular, it may choose a larger one than the current one, which is generally fine. But as the `TimeStepController` does not store the current Richards model time, at cannot re-adjust the time step to this time.

### Steps to reproduce
Produce a simulation where the CFL condition leads to a higher time step than the initial time step of the Richards model. See input files below.

### What is the current _bug_ behaviour?
Transport model with explicit time step scheme can choose a time step that overtakes the Richards model, leading to an error thrown by the grid function container.

### What is the expected _correct_ behaviour?
Transport model time steps are always limited by the current time of the Richards model.

### Relevant logs, screenshots, files...?
_Anything that helps reproducing the bug_

<details>
<summary>Expand to see logs</summary>
<pre>
root@abc565c71a3a:/mnt# dorie run config.ini 
[15:01:00.333 I]  Starting DORiE
[15:01:00.333 I]  Reading configuration file: config.ini
[15:01:00.337 I]  Creating a rectangular grid in 2D
[15:01:00.339 I] [RIC]  Loading parameter data file: richards_param.yml
[15:01:00.343 I] [RIC]  Loading boundary condition data file: gravity_flow.yml
[15:01:00.351 I] [RIC]  Setup complete
[15:01:00.354 I] [SOL]  Loading parameter data file: transport_param.yml
[15:01:00.358 I] [SOL]  Loading boundary condition data file: solute.yml
[15:01:00.361 D] [SOL]    Filling gaps in boundary condition specs with default boundary condition
[15:01:00.365 D] [SOL]  Creating initial condition of type: analytic
[15:01:00.365 D] [SOL]  Creating a VTK writer with subsampling level: 0
[15:01:00.367 D] [SOL]  Setting up grid operators and solvers
[15:01:00.367 D] [SOL]    Total number of DOF: 400
[15:01:00.367 I] [SOL]  Setup complete
[15:01:00.367 I] [CPL]  Setup complete
[15:01:00.408 I] [RIC]  Time Step 0: 0.00e+00 + 1.00e+03 -> 1.00e+03
[15:01:00.429 I] [SOL]  Time Step 0:
[15:01:00.429 W] [SOL]  Time step '3.6862e+06' incompatible to chosen limits. Adjusting automatically
[15:01:00.429 D] [SOL]    Time 0.00e+00 + 1.00e+02 -> 1.00e+02
[15:01:00.457 I] [SOL]  Time Step 1:
[15:01:00.458 W] [SOL]  Time step '3.6862e+06' incompatible to chosen limits. Adjusting automatically
[15:01:00.458 D] [SOL]    Time 1.00e+02 + 1.00e+05 -> 1.00e+05
[15:01:00.460 D] [SOL]    Solver not converged
[15:01:00.460 E] [SOL]    Unexpected error in solver: Dune::InvalidStateException [check_in_range_time:/opt/dune/dorie/dune/dorie/common/grid_function_container.hh:473]: time above the range (0,1000) of the grid function container!
[15:01:00.461 C]  Aborting DORiE after exception: Dune::Exception [step:/opt/dune/dorie/dune/dorie/model/transport/transport.cc:220]: Critical error in linear solver
Error while running DORiE executable: /opt/dune/dorie/build-cmake/dune/dorie/transport_d2_r0_t0
</pre>
</details>

#### Reproducing input
_Do you have input files reproducing the problem? Insert them here:_

### Ideas how to fix this?
Make the Transport solver store its global simulation end time separately. Add a new method with which the `ModelRichardsTransportCoupling` can propagate the Richards model time to the transport `TimeStepController`.

Edited Dec 20, 2019 by Lukas Riedel

Discussion
Designs

The one place for your designs

To upload designs, you'll need to enable LFS. More information