Use copy(deepcopy=true) for checkpointing

[BenjaminRodenberg]

There seems to be a bug when using checkpointing. I stumbled across this with subcycling:

• Perpendicular flap case with FEniCS and OpenFOAM as given in precice/tutorials@c20ed5b. Use subcycling in solid.py by setting fenics_dt = precice_dt / 10. Here, OpenFOAM runs into an exception after some time. The vtk files also start to obviously look faulty after some time.
• Perpendicular flap case with FEniCS and Fake fluid as given in precice/tutorials@c20ed5b. Use subcycling like above. Simulation runs through, but vtk results are incorrect.

Looking at the value of u_n in any of these cases shows that we do not correctly load the checkpoint we originally stored. I assume there is some unintended pointer-magic happening. I used the following debugging statements in solid.py to sample the solution at the tip of the flap:

if precice.requires_writing_checkpoint():  # write checkpoint
    print(f"Store checkpoint: {(u_n(0,H), v_n(0,H), a_n(0,H)),t,n}")
...
if precice.requires_reading_checkpoint():  # roll back to checkpoint
    print(f"Read checkpoint: {(u_cp(0,H), v_cp(0,H), a_cp(0,H)),t_cp,n_cp}")
    print(f"Overwrites: {(u_n(0,H), v_n(0,H), a_n(0,H)),t,n}")
    print(f"Ignores: {u_np1(0,H)}")
    ...
else:
    print(f"u_n: {(u_n(0,H), v_n(0,H), a_n(0,H)),t,n}")
...

The fixes applied here seem to avoid the error.

There are still some todos:

• Test this to make sure we don't get a similar error again.
• Understand performance implications. I have the impression that creating a deep-copy slows down the process. Would be nice to find a more efficient way, but the primary goal is to fix the bug.

[NiklasVin]

Before I start working on the TODOs of this PR, I wanted to understand what was going on here since I didn't experience any issues in the partitioned heat tutorial.
To this end, I ran some tutorials with subcycling and did some research about the copy function.

Tests (without the suggested fix)

1. Partitioned Heat equation
   I modified the heat.py file such that only one participant does subcycling.
   It seems like in this tutorial, the checkpointing doesn't affect the solution (or at least not so severely)
2. Elastic Tube 3D
   Similar to the perpendicular flap case, I set fenics_dt=precice_dt/5. The simulation crashed as well
3. Perpendicular Flap OpenFOAM-OpenFOAM
   Again, the solid participant does subcycling and this is what you'd get shortly before the program crash:
   
4. Perpendicular Flap Fluid Fake-FEniCS
   The picture speaks for itself:
   
5. Perpendicular Flap Fluid Fake-OpenFOAM
   After some time, the simulation explodes as well, but before that the flap does not become so wiggly as the one from 4. It is just bending to the right and grows.

What's wrong with copy() ?

Though in the current dolfin documentation about Function.copy(), the dof are deep copied, but as we use legacy FEniCS this is how the copy function is implemented (when I started looking at the documentation, I haven't considered that the dolfin version FEniCS uses is outdated; that's why I am pointing that out).
So, I think it is necessary to use a copy(deepcopy=true) with the current state of the adapter.

[NiklasVin]

Since the odd behavior isn't solely restricted to the OpenFOAM-FEniCS perpendicular flap tutorial but also to cases where the FEniCS adapter isn't used at all, my guess would be that there is a bug elsewhere as well.

What do you think?

[IshaanDesai]

Since the odd behavior isn't solely restricted to the OpenFOAM-FEniCS perpendicular flap tutorial but also to cases where the FEniCS adapter isn't used at all, my guess would be that there is a bug elsewhere as well.

The OpenFOAM-OpenFOAM failure definitely makes this more confusing. Otherwise I could imagine that something goes wrong in the Python bindings, and hence all Python-based participants produce failures.

[uekerman]

I could imagine that you have two independent issues here. Maybe ignoring OpenFOAM for the moment is the more sane strategy.

[BenjaminRodenberg]

@NiklasVin I assume you already did this: Can you also do the experiment from above including the changes from precice/tutorials#554 but without deepcopy=true? I would assume that you do not see any problems in this case because you are avoiding the problem on the side of the user.

[NiklasVin]

@NiklasVin I assume you already did this: Can you also do the experiment from above including the changes from precice/tutorials#554 but without deepcopy=true? I would assume that you do not see any problems in this case because you are avoiding the problem on the side of the user.

Exactly. I tested it without deepcopy=true and everything works fine.

[BenjaminRodenberg]

Based on the current state I would still suggest to merge this PR and use deepcopy by default. Main reason: It makes the usage of the FEniCS adapter more secure and we cannot run that easily in issues like the one fixed via precice/tutorials#554. So generally I would consider this as a usability improvement at cost of performance. If we learn that the performance cost is too high, we could still offer an option to set deepcopy=false for experienced users that implement safe update schemes like in precice/tutorials#554.

@IshaanDesai, @uekerman, @NiklasVin: Any comments? If not, I would like to merge this PR.

[IshaanDesai]

Makes sense, lets merge this 👍

[NiklasVin]

A short comment regarding the performance aspect: I tested the runtime of copying a dolfin function both with and without deep-copying. On average, the deep-copy variant is about 13% slower (only copying the function, not the whole execution of the simulation). So, the overall runtime shouldn't be affected too much, at least, it wasn't when comparing precice/tutorials#554 with the suggested changes here.
So, it makes sense to merge this.

[NiklasVin]

The changes look good to me. 👍
I am opening an issue regarding the crash of the OpenFOAM-OpenFOAM example from above.