Sub_cycling sync time not properly adjusting with negative times

[vincentlaboure]

Bug Description

Until #14865 is fixed, a workaround is to run a pseudo-transient from start_time = some_negative_time to end_time = 0 and to restart the actual transient from that solution.

However, when such a pseudo-transient is performed with a main app driving a sub-app and sub_cycling is used, the sub-app may become out of sync with the main app. This is because the sub-app picks its own time step with sub_cycling and is supposed to make sure that the chosen dt does not put it after the target_time.

Unfortunately, the current check is:

// Adjust to a target time if set
  if (_target_time > 0 && _time + dt_cur + _timestep_tolerance >= _target_time)
  {
    dt_cur = _target_time - _time;
    _at_sync_point = true;

    diag << "Limiting dt for target time: " << std::setw(9) << std::setprecision(6)
         << std::setfill('0') << std::showpoint << std::left << _next_interval_output_time
         << " dt: " << std::setw(9) << std::setprecision(6) << std::setfill('0') << std::showpoint
         << std::left << dt_cur << std::endl;
  }

The check _target_time > 0 is not pertinent with negative times. I am not 100% sure why there is that condition in this if-statement but it seems to me that it is because we have target_time set by default to -1:

_target_time(declareRecoverableData<Real>("target_time", -1)),

If instead, I apply the following patch:

diff --git a/framework/src/executioners/Transient.C b/framework/src/executioners/Transient.C
index c96fa7c..c4a36d0 100644
--- a/framework/src/executioners/Transient.C
+++ b/framework/src/executioners/Transient.C
@@ -155,7 +155,7 @@ Transient::Transient(const InputParameters & parameters)
     _time_interval(declareRecoverableData<bool>("time_interval", false)),
     _start_time(getParam<Real>("start_time")),
     _timestep_tolerance(getParam<Real>("timestep_tolerance")),
-    _target_time(declareRecoverableData<Real>("target_time", -1)),
+    _target_time(declareRecoverableData<Real>("target_time", -std::numeric_limits<Real>::max())),
     _use_multiapp_dt(getParam<bool>("use_multiapp_dt")),
     _solution_change_norm(declareRecoverableData<Real>("solution_change_norm", 0.0)),
     _sln_diff(_nl.addVector("sln_diff", false, PARALLEL)),
@@ -531,7 +531,7 @@ Transient::computeConstrainedDT()
   }
 
   // Adjust to a target time if set
-  if (_target_time > 0 && _time + dt_cur + _timestep_tolerance >= _target_time)
+  if (_target_time > -std::numeric_limits<Real>::max() + _timestep_tolerance && _time + dt_cur + _timestep_tolerance >= _target_time)
   {
     dt_cur = _target_time - _time;
     _at_sync_point = true;

Everything behaves as expected (in the same way as it does with positive times).

Steps to Reproduce

Currently, I do not have any simple example reproducing the error that I can share.

Impact

This is problematic because the sub-apps become out-of-sync.

[vincentlaboure]

Before I open a PR with the proposed changes, could someone let me know if this is an appropriate fix?

[YaqiWang]

Looks good to me. At least this fix is a super set of current capabilities. I will just go ahead to create the PR.

[vincentlaboure]

The main difficulty is to create a simple test that would reproduce the original problem. But I'll see if I can come up with one.

[YaqiWang]

-1 and 0 are hardcoded anyway ;-) It will be great to have a test indeed.