Closed klevzoff closed 5 years ago
@cssherman, do you think you will have time to look at this one soon? If not, we can find someone else to address it. It shouldn't take too long. It is now a bottleneck for the flow solvers.
@klevzoff @joshua-white - In the event manager (and the old code), we are expecting each event to execute the full requested timestep in a self-contained fashion. Otherwise, when we are applying solvers using a split-operator approach the solvers could go out of sync. Also, we typically don't expect solvers to have the ability to re-wind the timestep. These are the two general ways we handled this in the old code:
1) Independent events: Say that two solvers (solver_a, solver_b) are being applied sequentially and that the current dt = 1s. During this cycle solver_a will successfully complete the timestep, and solver_b needs to cut the timestep. We would expect that solver_b sub-step until it achieves the full timestep.
2) Coupled solvers: In this example, we'd use the same two physics solvers, plus a third coupled solver wrapper (solver_c). In the event manager, we would call solver_c, which would be designed to mediate the timestep for solver_a and solver_b together.
With regards to cutting future timesteps, that capability is already in the event manager, but it doesn't look like any solvers are making use of it yet. During the event loop, the manager will ask each event/target for their maximum allowable timestep, and then set the next dt to the smallest request. The optional forceDt flag on events works by overriding the timestep request for its target and children. For example, if the events request these dt requests: [0.1, 0.5, 1e6, 2.0], the actual dt for the next step would be 0.1s.
To address your specific issue with needing to cut the timesteps, we could add an additional flag to EventBase called 'maximumEventDt'. If this flag is set, then the event's dt_request would be equal to min(maximumEventDt, target->GetTimestepRequest(time)). Using this method, we would expect the solvers to keep track of when they are having issues completing timesteps, and return reasonable dt requests when their GetTimestepRequest method is called. I was going to be working on streamlining part of the event manager this week, and can make the above changes pretty easily.
@cssherman Thanks! Understood, this makes perfect sense.
I made a simple fix to ensure SolverBase::Execute()
always gets to the target dt. GetTimestepRequest()
is not implemented in flow solvers yet, we'll have to target that in another PR.
Yes, thanks Chris. Now that you say it, this makes sense as a general design principle. Solvers always have to have some logic to get to the requested dt, which is easier than having the EventManager mediate these types of things.
Yeah. It should be the solver's responsibility to achieve the requested dt
or error out.
Is your feature request related to a problem? Please describe. There is not enough time step feedback from an
ExecutableGroup
. We are able to request next time step size, which is useful for adaptive time stepping. But if a solver was unable to converge on a given time step size and had to cut, this is not reported back to the event manager and the next step proceeds fromtime + plannedDt
rather thantime + actualDtTaken
.EventBase::execute()
does not have a slot for returningactualDtTaken
.Describe the solution you'd like If a target has to modify the time step during execution, we should return that information to the manager and take into account. For example, current target may be asked to execute again until the target time is reached.
Describe alternatives you've considered I don't know if there is another way to achieve desired behavior. Maybe instead of cutting the time step inside the solver, we should return some sort of failure indicator and have the event manager do the step cutting in a centralized manner?
Additional context Compositional flow solvers may have to cut the time step quite often due to strong nonlinearities and poor Newton convergence.
Or is it expected that the solver will perform time stepping on its own until it reaches target time?