Refactor TimeIntegration

[TTitscher]

TimeIntegrationBase currently has 29 member variables, mostly options. NewmarkBase has another 10 and NewmarkDirect 4. So we should at least remove one to make it 42 in total. Or better:

My suggestion is a split into reasonable sub classes, these may be influenced by the NewmarkDirect scheme, since this is the one I use most

• class PostProcessing
• class TimeStepping - maybe with derived classes TimeSteppingConstant and TimeSteppingAutomatic
• class NewtonRaphson - this algorithm is implemented multiple times already...
  • more general class RootFinding, potentially even with new methods like class SecantMethod or classBrentsMethod or whatever
• class LineSearch
• class TimeDiscretization with TimeDiscretizationNewmark that contains the beta/gamma stuff.

Since this is the reference time integration scheme, we should make it great again.

[phuschke]

I completely agree with your suggestions.

NewmarkDirect:

int mVisualizeResidualTimeStep; //unused
int mVerboseLevel = 1; //no setter and no option in CTOR. Its basically constexpr for us

NewmarkBase:

double mInternalEnergy; //unused
double mExternalEnergy; //unused
double mKineticEnergy; //unused
double mDampedEnergy; //unused
double mToleranceForce; // only used to copy its value to mToleranceResidual

TimeIntegrationBase: These variables should not (among others) be in the TimeIntegrationBaseClass as they are not necessary for many time integration methods. A TimeStepping class would be awesome for this problem.

bool mAutomaticTimeStepping; 
double mMaxTimeStep;
double mMinTimeStep;

We should probably start rethinking the greatest common divisor of all time integration methods and start with the base class and the PostProcessing class.

[Psirus]

There is still a lot to improve, and some "temporary fixes" have been introduced in the refactor. I'll leave this open for now. Opinions welcome.

[vhirtham]

Because we recently had this discussion because of the question, why the GetTimeControl is not const:

We should decide, whether we want to wrap all the functionality of the timeControl class in the timeIntegrationBase or if you should get a non const reference to it to do the necessary adjustments.

Arguments for wrapping it:

• As user it feels more intuitive to look for a timestep setter in the time integration scheme than to get a time control class and set their timestep

Arguments against it:

• You generate a lot of unnecessary code in the timeIntegrationBase

• If we wrap every funtionality, there is also the question, why do I need a separate class in the first place

Third option is to have both to some extent. Just wrap basic functionality like timestep, min, max and automatic timestepping. Every other fancy stuff needs to be done on the object itself. I think this is probably the best way. Since 99% of the cases are covered by equidistant and default automatic time stepping, it is okay to wrap those functionality for the standard user. ;) With a non const getter to the time control class you still have full flexibility to do whatever you need to do. In this case the class design itself should prohibit actions that could mess up the data. For example, no copy or move assignment and no possibility to set the current time directly.

Well looking forward to hear your opinions.

[TTitscher]

Forth option: TimeIntegration just uses the TimeControl object, without modifying it. User creates a TimeControl object, modifies it as he wishes and copies/moves/references it to TimeIntegration which itself provides no way of modifying this object. (The same pattern could be applied to PostProcessing)

Apart from that, another question: The adaptive time stepping is tightly coupled to the number of iterations and some converged flag. This may be OK for implicit schemes. For explicit schemes or IMPL-EX there is always one iteration. Can you think of a more general apporach? Another std::function that somehow returns bool? How to capture its arguments?

[vhirtham]

Fourth option is also okay for me. It is basically the same like the "no wrapped functions" approach with the same critics mentioned above applying to it. You just removed the ownership with all the consequences (no need for a getter, pass reference on construction).

Concerning your adaptive time stepping question: Problem is, that I don't know much about the requirements for explicit schemes or other stuff like impl-ex. Maybe we should rename the current defaultAutomaticTimestepping function to defaultAutomaticTimesteppingImplicit and write another default explicit version. The current function is just the version I found in Newmark, which of course is an implicit method. If you know a general solution that works for implicit and explicit schemes, feel free to replace the current version.

I should mention, that at the moment the time control is only used by Newmark. I think it makes sense to approach this problem if we start implementing the time control into the other time integration schemes.