Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Evaluate time-varying boundary conditions and motion at the end of the time step for implicit temporal schemes #2353

Open
3 tasks done
j-signorelli opened this issue Sep 6, 2024 · 6 comments
Labels

Comments

@j-signorelli
Copy link
Contributor

There seems to potentially be a bug with doing an unsteady dual-timestepping simulation without a provided initial condition (restart file):

When no restart file is provided, the first inner iterations have Time_Iter=0 and Cur_Time=0 (the state before these inner iterations). After completion of this time iteration, a file flow_00000.vtu and all others are outputted. These files are not the initial condition as specified by the configuration file -- they seem to actually be the solution after the first time step. This can be reproduced easily in the config file I have attached to this issue (an updated version of TestCases/plunging_naca0012 for SU2 v8). The flow_00000.vtu file has non-uniform flowfields, which doesn't make sense.

For simulations where a restart file like restart_00000.dat is provided, the Time_Iter=1 and Cur_Time=dt (the state after these inner iterations), and the subsequently outputted files have the correct time iteration appended to it.

I think that for non-restart unsteady simulations, the IC should be outputted as solely just the initialization before any inner iterations are completed, Time_Iter++ and Cur_Time+=dt, and THEN inner iterations performed.

plunging_NACA0012.cfg.txt

Bug report checklist
Please make sure that you have followed the checklist below, many common problems can be solved by:

Desktop (please complete the following information):

  • OS: Linux (Ubuntu 22.04)
  • C++ compiler and version: GNU
  • MPI implementation and version: MPICH
  • SU2 Version: v8.0.1
@j-signorelli j-signorelli changed the title Non-Restart Unsteady Control Parameters + IC Outputs Not Correct? Non-Restart Unsteady Time, TimeIter, and IC Outputs Not Correct? Sep 6, 2024
j-signorelli added a commit to j-signorelli/su2-adapter that referenced this issue Sep 10, 2024
@pcarruscag pcarruscag added question and removed bug labels Dec 8, 2024
@pcarruscag
Copy link
Member

The solution is always at the end of time steps, to restart at time iter 2 you need 0 and 1

@j-signorelli
Copy link
Contributor Author

Hi @pcarruscag, thank you for your response but I am referring to an issue I found when not restarting and just using the freestream as the initial condition - because the solution is always at the end of timesteps, after inner iterations are completed on the 0th iteration / 0th timestep, SU2 outputs a flow_00000.vtu file, but this is after inner iterations which it shouldn't be doing on the IC. When I look at the flowfield, it is indeed not just the freestream everywhere but appears to be a timestep ahead because inner iterations are performed on the IC.

I feel that before SU2 performs any inner iterations, it should output flow_00000.vtu and then increment time/timestep, and THEN start solving as normal.

Please let me know if I am misunderstanding anything, I found this issue when wanting to use the freestream as an initial condition without a restart file.

@pcarruscag
Copy link
Member

I do understand the confusion but the iteration indexing is 0-based, so at iteration "i", ""i+1" * dt has elapsed.
Changing this would be quite painful unfortunately and it would break restarts from existing solutions that folks may have.
If you want to write out the initial conditions (at iteration -1) you can give it a try using the Python wrapper by calling the Output() function before any iteration is performed. Have a look at the examples in TestCases/py_wrapper and let me know if you find some issues.

@j-signorelli
Copy link
Contributor Author

j-signorelli commented Dec 12, 2024

That makes sense, thank you for the explanation but I think there are a lot of places in the code that presume that the time to be solved for is dt*TimeStep instead of dt*TimeStep+dt -- isn't this incorrect since dual-timestepping is implicit and thus the state should be at t+1? I remember when attempting #2190 seeing a LOT of places that assume this which was ultimately why I stopped working on it. Just some examples:

# Apply a custom load and then solve the time step.
time = time_iter * driver.GetUnsteadyTimeStep()
for i_vertex in range(n_vertex_ctrl):
i_point = driver.GetMarkerNode(ctrl_id, i_vertex)
if driver.GetNodeDomain(i_point):
driver.SetMarkerCustomFEALoad(ctrl_id, i_vertex, (-0.002 + 0.002 * math.cos(2 * math.pi * time / 0.02), 0))

/* Determine the time for which the motion data must be determined. */
const su2double deltaT = config->GetDelta_UnstTimeND();
su2double tNew = TimeIter*deltaT;

/*--- Forward time for the direct problem ---*/
time_new = static_cast<su2double>(iter) * deltaT;
if (harmonic_balance) {
/*--- For harmonic balance, begin movement from the zero position ---*/
time_old = 0.0;
} else {
time_old = time_new;
if (iter != 0) time_old = (static_cast<su2double>(iter) - 1.0) * deltaT;

/*--- Forward time for the direct problem ---*/
time_new = static_cast<su2double>(iter) * deltaT;
time_old = time_new;
if (iter != 0) time_old = (static_cast<su2double>(iter) - 1.0) * deltaT;

/*--- Compute delta time based on physical time step ---*/
time_new = iter*deltaT;
if (iter == 0) time_old = time_new;
else time_old = (iter-1)*deltaT;

The last 3 in particular seem to avoid this assumption by checking if the iteration is 0. To reiterate I may be completely misunderstanding here (if the intention for everything is to be IMEX), but want to confirm this with you. It may be worthwhile to add clarifications regarding this to the documentation.

@pcarruscag
Copy link
Member

That is right, for time-implicit approaches we should have everything at the end of the timestep.

@j-signorelli
Copy link
Contributor Author

Should I open a new issue then for this documenting it? Or should we re-open this one and change the title?

@pcarruscag pcarruscag reopened this Dec 13, 2024
@pcarruscag pcarruscag changed the title Non-Restart Unsteady Time, TimeIter, and IC Outputs Not Correct? Evaluate time-varying boundary conditions and motion at the end of the time step for implicit temporal schemes Dec 13, 2024
@pcarruscag pcarruscag added bug and removed question labels Dec 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants