precice / calculix-adapter

preCICE-adapter for the CSM code CalculiX
GNU General Public License v3.0
51 stars 20 forks source link

Checkpointing does not work for subcycling #9

Open derekrisseeuw opened 5 years ago

derekrisseeuw commented 5 years ago

For implicit coupling the writeCheckpoint is called by the function: Precice_WriteIterationCheckPoint

This function takes the values of the last timestep as checkpoint values. However, for implicit coupled simulations where the calculix is subcycling, the last calculix timestep is not the same as the last coupling timestep. Therefore, the wrong checkpoint is written.

uekerman commented 5 years ago

Thanks for pointing us to this. I guess that this has simply never been implemented.

derekrisseeuw commented 5 years ago

Thank you for your reaction Benjamin.

This problem can be easily overcome in the calculix input file by specifying: *DYNAMIC,DIRECT

Where the 'direct' keyword forces calculix to use the timestep specified (this should be the precice timestep).
nkr0 commented 4 years ago

Doesn't putting write/read checkpoints inside if *required prevent this? My understanding is that the solver has to be at one of the ends of a coupling step for these to return true.

If that is the case, CalculiX sub-cycles by storing states in vini and vold without touching simulationData->coupling_init_v, which is only written/read at the beginning/end of coupling steps. precice resets vold to the beginning of the coupling time step, when Precice_IsReadCheckpointRequired returns true, i.e., only when the coupling step is not converged at the end of a coupling step (not at the end of CalculiX sub-cycles). Similarly, Precice_IsWriteCheckpointRequired returns true only at the beginning of a coupling time step to store a checkpoint in simulationData->coupling_init_v.

https://github.com/precice/calculix-adapter/blob/b01641e40c11e067f66cc41fbcb0253d08290ff0/nonlingeo_precice.c#L1115-L1121 https://github.com/precice/calculix-adapter/blob/b01641e40c11e067f66cc41fbcb0253d08290ff0/nonlingeo_precice.c#L2664-L2678

Read/WriteCouplingData, AdjustSolverTimestep, and Advance executes every sub-cycle, to https://github.com/precice/calculix-adapter/blob/b01641e40c11e067f66cc41fbcb0253d08290ff0/nonlingeo_precice.c#L1097-L1099 https://github.com/precice/calculix-adapter/blob/b01641e40c11e067f66cc41fbcb0253d08290ff0/nonlingeo_precice.c#L2666 read/write simulationData->preciceInterfaces, https://github.com/precice/calculix-adapter/blob/b01641e40c11e067f66cc41fbcb0253d08290ff0/adapter/PreciceInterface.c#L162 https://github.com/precice/calculix-adapter/blob/b01641e40c11e067f66cc41fbcb0253d08290ff0/adapter/PreciceInterface.c#L229 set simulationData->dtheta (to keep calculix within the coupling step), https://github.com/precice/calculix-adapter/blob/b01641e40c11e067f66cc41fbcb0253d08290ff0/adapter/PreciceInterface.c#L84 and increments simulationData->precice_dt (to track progress within a coupling step), respectively. https://github.com/precice/calculix-adapter/blob/b01641e40c11e067f66cc41fbcb0253d08290ff0/adapter/PreciceInterface.c#L96

nkr0 commented 4 years ago

Furthermore, CalculiX's check for sub-cycle convergence is icutb==0. https://github.com/precice/calculix-adapter/blob/b01641e40c11e067f66cc41fbcb0253d08290ff0/nonlingeo_precice.c#L2683-L2725 This controls memcpy between vini and vold (vini is Calculix's sub-cycle "checkpoint") and precice checkpoints are also under this check, making sure precice checkpoints are only read/set when calculix sub-cycles are converged. https://github.com/precice/calculix-adapter/blob/b01641e40c11e067f66cc41fbcb0253d08290ff0/nonlingeo_precice.c#L1101-L1121 https://github.com/precice/calculix-adapter/blob/b01641e40c11e067f66cc41fbcb0253d08290ff0/nonlingeo_precice.c#L2628-L2681 You can see in the blob above that when precice reads a checkpoint, we increment icutb to unset CalculiX's converged state.

MakisH commented 2 years ago

I assume that the branch https://github.com/precice/calculix-adapter/tree/i9 refers to this issue. Should we still keep it around? Is this issue still relevant?

MakisH commented 2 years ago

This issue is still valid with the adapter 2.19.0. Easy to reproduce: get the perpendicular-flap tutorial and, in flap.inp, replace:

*DYNAMIC, ALPHA=0.0, DIRECT
- 1.E-2, 5.0
+ 3.E-3, 5.0

This makes the OpenFOAM side crash due to a mesh failure:

Screenshot from 2022-07-29 11-26-39

We definitely need to at least document this.

boris-martin commented 2 years ago

I've been toying around to fix this, but this opens new questions. Roughly speaking, in terms of data, it should be sufficient to have checkpoints "go back deep enough". I have prototypes who seem to go into the correct direction. However I'm not sure how to handle outputs: by default, every sub-step is recorded. If we subcycle, we might get steps rewritten may times. The only way I see it would be to add a condition like "if implicit coupling is used, output frequency must match exactly the time window size (or every N windows could work too)". If we do that, should we override manual frequency control ? Just warn ? This would probably mean that switching between explicit and implicit will mess the output frequency, which is a bit weird.

MakisH commented 2 years ago

Roughly speaking, in terms of data, it should be sufficient to have checkpoints "go back deep enough".

Important to ensure here, is that checkpoints should only be written on every coupling time window (and every time a coupling time window is repeated), not every solver time step. When reading a checkpoint, CalculiX should go back to the state of the last coupling time window.

However I'm not sure how to handle outputs: by default, every sub-step is recorded. If we subcycle, we might get steps rewritten may times.

This is something I am not completely sure how it works also in OpenFOAM at the moment. If I remember correctly, results get overwritten, and this is fine.

The only way I see it would be to add a condition like "if implicit coupling is used, output frequency must match exactly the time window size (or every N windows could work too)". If we do that, should we override manual frequency control ? Just warn ?

We could always give a warning (or error) till we fix the actual problem.

This would probably mean that switching between explicit and implicit will mess the output frequency, which is a bit weird.

I am not sure I understand this point.

boris-martin commented 2 years ago

Roughly speaking, in terms of data, it should be sufficient to have checkpoints "go back deep enough".

Important to ensure here, is that checkpoints should only be written on every coupling time window (and every time a coupling time window is repeated), not every solver time step. When reading a checkpoint, CalculiX should go back to the state of the last coupling time window.

Yes. From what I understood the issue lies mostly in the fact that not sufficient data is saved but since we check that writing checkpoints is required, this should be fine. We mostly need to upgrade to reset correctly the internal time, step number etc, as currently only the DOFs are saved.

However I'm not sure how to handle outputs: by default, every sub-step is recorded. If we subcycle, we might get steps rewritten may times.

This is something I am not completely sure how it works also in OpenFOAM at the moment. If I remember correctly, results get overwritten, and this is fine.

I'll have to check if CalculiX can overwrite. Since it's a unique file, I'm not so sure, it's not like you can write many times the file "output_step_37" many times.

This would probably mean that switching between explicit and implicit will mess the output frequency, which is a bit weird.

I am not sure I understand this point.

Imagine a time window size of 1.0 and an internal step of 0.4.