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

Mistake in the interpretation of DisplacementDelta #106

Closed
nkr0 opened this issue Nov 28, 2019 · 12 comments
Closed

Mistake in the interpretation of DisplacementDelta #106

nkr0 opened this issue Nov 28, 2019 · 12 comments
Assignees
Labels
bug Unexpected problems (crashes, numerical issues, etc)

Comments

@nkr0
Copy link

nkr0 commented Nov 28, 2019

If the definition of DisplacementDelta is V - V_checkpoint, the adapter's interpretation is not correct. Adapter is interpreting the delta as V - V_sub_cycle.

pointDisplacementFluidPatch[i][0] += buffer[bufferIndex++];

@nkr0 nkr0 changed the title Mistake in the interpretation of DisplacementDeltas Mistake in the interpretation of DisplacementDelta Nov 28, 2019
@MakisH MakisH added the bug Unexpected problems (crashes, numerical issues, etc) label Dec 13, 2019
@davidscn davidscn self-assigned this Mar 6, 2020
@davidscn
Copy link
Member

davidscn commented Mar 7, 2020

Ok, I like to recap what's meant here a little bit more in detail:
In case DisplacementDelta is used for coupling, it is assumed that the solid solver writes relative changes of the displacement between two time steps: d_i - d_{i-1}, where d indicates here the absolute displacement at time i. These relative changes are later added to the displacement field in the Openfoam adapter.

If the time step size of the fluid solver is now smaller than the coupling time step (or time window in preCICE languagae), everything works as expected, since data is only transferred at the coupling time step and the solid solver displacement corresponds to this time window.

If the solid time step is smaller than the coupling time step we have a problem, since we only obtain the difference (delta) in the displacement for a smaller solid time step on the fluid side.
But there is no way on the fluid side, to compensate for this case, since new data is only obtained at the end of a coupling time step and not in between. The 'missing' deltas for all other subcycled time steps is not known and the absolute displacement after this time is on the fluid side unknown as well.

So, one needs to take care of this problem on the solid side, by adding all DisplacementDelta for each subcycled time step and pass the result to the fluid participant. Then, the definition as implemented in the adapter is valid.

@MakisH @nkr0 Do you agree?

@MakisH
Copy link
Member

MakisH commented Mar 7, 2020

I agree with your point about Fluid vs Solid: we need to adapt the size of the delta on the Solid side.

However, there is (I think) another problem on the Fluid side: in every subcycle (time step size < time window size), we are adding a delta to the displacement. This means that we will get completely different results with a different time step size. Could you maybe check this?

@davidscn
Copy link
Member

davidscn commented Mar 7, 2020

This should not be the case:
Data is added, when the adapter calls

void preciceAdapter::FSI::DisplacementDelta::read(double * buffer, const unsigned int dim)

as in every data field. According to the adapter design, this function must only be called after each time window. Otherwise, we would assign (reset) the obtained values in other fields (e.g. displacement absolut) after each subcycled time step as well, which makes no sense. So, this would be a bug in the design of the whole adapter. I will check it anyway..

@davidscn
Copy link
Member

davidscn commented Mar 7, 2020

This is a question for preCICE experts:
The values are read, if the following condition returns true:

    // Are new data available or is the participant subcycling?
    if (precice_.isReadDataAvailable())

But logically it should return false in case of subcycling, since there isn't new data. We should replace the debug message there, since data is not read in any case. Maybe this was confusing here.

@nkr0
Copy link
Author

nkr0 commented Mar 7, 2020

Reference for DisplacementDelta is the beginning of the coupling time step. So yes, if solid time step is smaller than the coupling time step, it has to be taken care of in the solid adapter. But, if the fluid time step is smaller than the coupling time step, it has to be taken care on the fluid side. For example. Let's assume that the solid went from 0 to 1 in a coupling step taking 2 linear increment steps. And the fluid wants to do that in 5 linear increment steps. Solid sends DisplacementDelta with reference to coupling displacement 0 as,
sub_cycle 1 => 0.5
sub_cycle 2 => 1
Let's say the fluid adapter gets this DisplacementDelta data from preCICE as,
sub_cycle 1 => 0.2
sub_cycle 2 => 0.4
sub_cycle 3 => 0.6
sub_cycle 4 => 0.8
sub_cycle 5 => 1
What we want the fluid adapter to do is, add these to the saved displacement field from the beginning of the coupling time step, 0, and get displacement fields for each sub-cycle as,
sub_cycle 1 => 0+0.2
sub_cycle 2 => 0+0.4
sub_cycle 3 => 0+0.6
sub_cycle 4 => 0+0.8
sub_cycle 5 => 0+1
However, what the adapter does now is, add the DisplacementDelta from preCICE to the current displacement field. Or at-least that is my understanding. This will result in displacements to be,
sub_cycle 1 => 0+0.2
sub_cycle 2 => 0.2+0.4
sub_cycle 3 => 0.6+0.6
sub_cycle 4 => 1.2+0.8
sub_cycle 5 => 2+1
This is the mistake this issue is referring to. It can't be addressed on the solid side. I'm not sure whether the displacement field is reset at the beginning of each sub_cycle. But that has no effect on this issue and is something else to make sure. Solution to the current issue is to change pointDisplacementFluidPatch[i][0] += buffer[bufferIndex++]; to pointDisplacementFluidPatch[i][0] = subcycle_checkpoint + buffer[bufferIndex++];

@nkr0
Copy link
Author

nkr0 commented Mar 7, 2020

more discussion on this issue can be seen here. precice/calculix-adapter#23

@davidscn
Copy link
Member

davidscn commented Mar 8, 2020

Ok I don't agree here, but maybe I don't understand you correctly.
Here is a citation from Benjamins thesis:

preCICE is technically able to handle subcycling,meaning that one solver A performs smaller local timesteps till another solver B finishes one bigger global timestep. In this case, Solver A uses, however, the value from solver B as constant boundary condition throughout all local timesteps, thus a order-zero extrapolation for parallel coupling schemes

and further:

isReadDataAvailable and isWriteDataAvailablecan prevent unnecessary data access in case of subcycling

which means: During subcycling, no participant accesses any data or let preCICE read any data.

To take your example again: Solid participants performs two subcycling time steps and reaches a displacement of zero to one and the fluid participant performs five subcycling time steps and receives the data. This will look like this in preCICE:
For the solid (starting)
sub_cycle 1
sub_cycle 2 => 1 (0.5 per time step) => write to preCICE
For the fluid:
Receive 1 from the solid:
sub_cycle 1 => 0+1
sub_cycle 2
sub_cycle 3
sub_cycle 4
sub_cycle 5 => write force value

This is also, what I observe in the adapter. AfaIk, time interpolation is a future project of preCICE.

I'm not sure whether the displacement field is reset at the beginning of each sub_cycle.

The fields are reloaded at the beginning of each implicit coupling time step, which is independent of the subcycling time steps.

@nkr0
Copy link
Author

nkr0 commented Mar 8, 2020

Ok, interpolate was the wrong choice of word. What I meant are the post-processed values from preCICE and the values I wrote are just to explain the issue. Still, the modified example that you've written looks more like an explicit coupling where 1 solver has to wait for the other to finish. From what I've seen in implicit schemes, fluid writes forces at the end of every sub_cycle and reads DisplacementDeltas at the beginning of every sub_cycle. And the opposite for the solid. And I'm not sure about the context of the quotes from @uekerman 's thesis. @uekerman ?

@MakisH
Copy link
Member

MakisH commented Mar 8, 2020

This is a question for preCICE experts:
The values are read, if the following condition returns true:

    // Are new data available or is the participant subcycling?
    if (precice_.isReadDataAvailable())

But logically it should return false in case of subcycling, since there isn't new data. We should replace the debug message there, since data is not read in any case. Maybe this was confusing here.

Indeed, I forgot about this.The same happens in the CalculiX adapter.

Then this is actually related to #3

@uekerman
Copy link
Member

  • DisplacementDeltas d_now - d_ref should be relative to the last window (aka coupling timestep), which in general is not the last timestep. So d_ref is the value from the last window. Every other choice will give problems with quasi-Newton acceleration as we need a fixed input->output mapping there.
  • isReadDataAvailable is indeed only true at the end of a window. In the future (once we have time interpolation), this method will maybe drop out. tbd. Don't use this method to update d_ref
  • Instead, you can/should update the reference displacement d_ref = d_now if writeIterationCheckpoint is required. So, exactly where you store your checkpoint.
  • No solver should need to know which timestep size the other solver uses, and also not what the window size is. Whenever you think you need to know this, there is a simpler solution.

Did I actually answer your questions?

@nkr0
Copy link
Author

nkr0 commented Mar 12, 2020

Yes, and that means this issue can be closed. Thanks.

@nkr0 nkr0 closed this as completed Mar 12, 2020
@MakisH
Copy link
Member

MakisH commented Mar 14, 2020

  • isReadDataAvailable is indeed only true at the end of a window. In the future (once we have time interpolation), this method will maybe drop out. tbd. Don't use this method to update d_ref
  • Instead, you can/should update the reference displacement d_ref = d_now if writeIterationCheckpoint is required. So, exactly where you store your checkpoint.

Just for completeness: this is what we currently do in the OpenFOAM adapter:

void preciceAdapter::Adapter::writeMeshCheckpoint()

This is called only when isWriteCheckpointRequired():

openfoam-adapter/Adapter.C

Lines 285 to 295 in 11db960

if (isWriteCheckpointRequired())
{
checkpointing_ = true;
// Setup the checkpointing (find and add fields to checkpoint)
setupCheckpointing();
// Write checkpoint (for the first iteration)
writeCheckpoint();
fulfilledWriteCheckpoint();
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Unexpected problems (crashes, numerical issues, etc)
Projects
None yet
Development

No branches or pull requests

4 participants