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

save state after every coupling iteration in explicit coupling #23

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

nkr0
Copy link
Collaborator

@nkr0 nkr0 commented Nov 27, 2019

fix #8

A small hack to make DisplacementDeltas work for explicit coupling as well. My question however is, is DisplacementDeltas change in displacement within a sub-cycle or change in displacement relative to the displacement at the beginning of the coupling step. I hope I'm clear here. The changes here assume the former.

@nkr0 nkr0 changed the base branch from develop to master November 27, 2019 17:16
@nkr0 nkr0 requested a review from MakisH November 27, 2019 17:18
@nkr0 nkr0 changed the base branch from master to develop November 27, 2019 17:25
@MakisH
Copy link
Member

MakisH commented Nov 27, 2019

I am not sure I understand the solution. Why store a checkpoint in explicit coupling? Maybe you can extract only the relevant bit and call it always, independent of the checkpointing?

Copy link
Member

@MakisH MakisH left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contributions! :D I hope these comments help express a bit better in the fix what I think you actually want to do.

@@ -1114,10 +1114,10 @@ void nonlingeo_precice(double **cop, ITG *nk, ITG **konp, ITG **ipkonp, char **l

memcpy(&vini[0],&vold[0],sizeof(double)*mt**nk);

if( Precice_IsWriteCheckpointRequired() )
if( Precice_IsWriteCheckpointRequired() || simulationData.coupling_explicit)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would then almost always be true, right? I understnad that you always want checkpoints to be stored (to trigger some internal update).

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For explicit coupling, yes.

@@ -47,6 +47,9 @@ void Precice_Setup( char * configFilename, char * participantName, SimulationDat
// Initialize coupling data
Precice_InitializeData( sim );

// find if coupling is implicit or explicit. Implicit coupling will return true at the very beginning and explicit will always return false
sim->coupling_explicit = !Precice_IsWriteCheckpointRequired();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you need such a boolean, it would be better to make is coupling_implicit. Usually we assume that implicit coupling is the special case that requires special treatment. You could then still write if (!coupling_implicit).

But I think you don't need this check in the first place, it should be enough to call what you want always.

Copy link
Collaborator Author

@nkr0 nkr0 Nov 28, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure, !coupling_implicit or coupling_explicit is no problem. But a check is needed. Because the call I want to happen always is only in the case of explicit coupling. For implicit coupling it should follow Precice_IsWriteCheckpointRequired() (only at the ends of coupling iterations).

@nkr0
Copy link
Collaborator Author

nkr0 commented Nov 28, 2019

I am not sure I understand the solution. Why store a checkpoint in explicit coupling? Maybe you can extract only the relevant bit and call it always, independent of the checkpointing?

There is more information in issue #8. It's not exactly a "checkpoint". We just want the memcpy line from the write function,

void Precice_WriteIterationCheckpoint( SimulationData * sim, double * v )
{
printf( "Adapter writing checkpoint...\n" );
fflush( stdout );
// Save time
sim->coupling_init_theta = *( sim->theta );
// Save step size
sim->coupling_init_dtheta = *( sim->dtheta );
// Save solution vector v
memcpy( sim->coupling_init_v, v, sizeof( double ) * sim->mt * sim->nk );
}

so that the reference point w.r.t. which we compute DisplacementDeltas, sim->coupling_init_v (v_init) gets updated in every iteration for only explicit cases.
for( j = 0 ; j < dim ; j++ ) displacementDeltas[dim * i + j] = v[nodeIdx * mt + j + 1] - v_init[nodeIdx * mt + j + 1];

So yes, we could extract the memcpy line and write it. But the only differences between calling writecheckpoint and calling memcpy are the 2 lines of saving theta and dtheta. It really doesn't make any difference.

But, all of this is only if my understanding of the definition of DisplacementDeltas is correct. What is the reference point for DisplacementDeltas? Is it the Displacement from the last sub-cycle? Like it is implemented now. Or, is it the Displacement from the beginning of the coupling iteration? Like how it is for implicit cases, where the reference point is set at the beginning of a new coupling iteration, controlled by the IsWriteCheckpointRequired call. Explicit coupling cases also synchronise at regular coupling intervals right? Either fixed or controlled by one of the participants.

@KyleDavisSA
Copy link
Contributor

A question from #8

Here the previous solution is added to coordinates and the comment says it is for restarts. Although I'm not really sure how correct that is. Displacement is the solution from a loaded condition. But changing the coordinates really alters the solid. It can stay in the displaced state without any load acting on it.

That is correct, but I guess the description of a restart is vague. The "restart" is when the simulation must be stopped and restarted from the same point in time. An example would be if the fluid mesh is too distorted. The simulation can be stopped, the fluid mesh re-meshed, and the simulation restarted from the last time step. This then does involve restarting CalculiX from the previous loaded condition.

so that the reference point w.r.t. which we compute DisplacementDeltas, sim->coupling_init_v (v_init) gets updated in every iteration for only explicit cases.

Before void Precice_WriteIterationCheckpoint( SimulationData * sim, double * v ), the variable double * v is updated with the displacement at the end of the previous iteration vold

memcpy(&vini[0],&vold[0],sizeof(double)*mt**nk);
if( Precice_IsWriteCheckpointRequired() )
{
Precice_WriteIterationCheckpoint( &simulationData, vini );
Precice_FulfilledWriteCheckpoint();
}

This is just to clarify that DisplacementDeltas should be the displacement at the current time - displacement at the end of the previous timestep.

@nkr0
Copy link
Collaborator Author

nkr0 commented Nov 29, 2019

The part about restart from #8 is only something I misunderstood back then. Nothing to do with this PR or issue #8 itself.

This is just to clarify that DisplacementDeltas should be the displacement at the current time - displacement at the end of the previous timestep.

That is correct, and that will be the case once the PR is merged. First we copy vold into vini. And then vini is saved as displacement at the previous time step.

memcpy(&vini[0],&vold[0],sizeof(double)*mt**nk);
if( Precice_IsWriteCheckpointRequired() || simulationData.coupling_explicit)
{
Precice_WriteIterationCheckpoint( &simulationData, vini );

After the solution for the current time step is computed, v is copied into vold and we call WriteCouplingData.
memcpy(&vold[0],&v[0],sizeof(double)*mt**nk);
Precice_WriteCouplingData( &simulationData );

Inside WriteCouplingData we compute DisplacementDelta,
case DISPLACEMENTDELTAS:
getNodeDisplacementDeltas( interfaces[i]->nodeIDs, interfaces[i]->numNodes, interfaces[i]->dim, sim->vold, sim->coupling_init_v, sim->mt, interfaces[i]->nodeVectorData );
precicec_writeBlockVectorData( interfaces[i]->displacementDeltasDataID, interfaces[i]->numNodes, interfaces[i]->preciceNodeIDs, interfaces[i]->nodeVectorData );

where v_init is the old solution we stored at the beginning of the iteration and v is a direct pointer access to the current solution CalculiX had just finished calculating.
void getNodeDisplacementDeltas( ITG * nodes, ITG numNodes, int dim, double * v, double * v_init, int mt, double * displacementDeltas )
{
// CalculiX variable mt = 4 : temperature + 3 displacements (depends on the type of analysis)
// where 0 index corresponds to temp; 1, 2, 3 indices correspond to the displacements, respectively
ITG i, j;
for( i = 0 ; i < numNodes ; i++ )
{
int nodeIdx = nodes[i] - 1; //The node Id starts with 1, not with 0, therefore, decrement is necessary
for( j = 0 ; j < dim ; j++ ) displacementDeltas[dim * i + j] = v[nodeIdx * mt + j + 1] - v_init[nodeIdx * mt + j + 1];
}
}

@nkr0 nkr0 changed the title save state after every iteration in explicit coupling save state after every coupling iteration in explicit coupling Nov 29, 2019
@uekerman
Copy link
Member

I am sorry for the very late reply here.

I appreciate the contribution a lot, but I am not 100% happy with the current solution. It was already a bit hacky before (current master) and now it seems to get too complicated.

Up front, to clarify again: DisplacementDeltas should be relative to the displacements from the last coupling timestep / coupling window. This definition is (and needs to be) independent of whether explicit or implicit coupling is used.

Explicitly storing whether a implicit or explicit coupling scheme is used (through a variable such as coupling_explicit) is something that is not intended by the preCICE API. Whenever you feel you need to do this, there has to be a cleaner way (or the design assumptions of the preCICE API are wrong). I agree that fully understanding this is an hard entry barrier and thus, a weak spot of preCICE.

I guess the cleaner solution here is to use two separate variables:

  • One to checkpoint the solution, needs to be displacements of the full domain (or maybe also velocity values etc depending on the time stepping method used)
    • read and write this variable in write/readIterationCheckpoint
  • One to store the referential displacement values for DisplacementDeltas. This could be just stored in the "preCICE data structure", so only values at the coupling interface.
    • write this variable in isTimestepComplete, use it when writing DisplacementDeltas to preCICE.

Understandable?

@nkr0
Copy link
Collaborator Author

nkr0 commented Dec 17, 2019

Yes, understood. However, check the bit below,

if( Precice_IsWriteCheckpointRequired() ) // implicit coupling
{
Precice_WriteIterationCheckpoint( &simulationData, vini );
Precice_FulfilledWriteCheckpoint();
}
if( Precice_IsCouplingTimestepComplete() && !simulationData.coupling_implicit ) // explicit coupling
Precice_WriteIterationCheckpoint( &simulationData, vini );

in implicit coupling, when does isTimestepComplete return true? Is it only at the end of a converged coupling time-step, or every time the coupling iteration reaches the end of the coupling time-step (even when readIterationCheckpoint is going to restart the iteration)? If it is the former, yeah we can use isTimestepComplete for DisplacementDeltas, and isWriteCheckpointRequired for everything else. But, in the latter case we cannot and will need an explicit/implicit switch to in addition to isTimestepComplete. This doubt lead me to create separate cases for implicit and explicit.

I vaguely remember checking this and I think it is the latter.

@uekerman
Copy link
Member

in implicit coupling, when does isTimestepComplete return true? Is it only at the end of a converged coupling time-step, or every time the coupling iteration reaches the end of the coupling time-step (even when readIterationCheckpoint is going to restart the iteration)?

It only returns true if the timestep is completely completed, so converged (for implicit coupling) and last sub timestep. So the former.

If it is really the latter it would be a bug in preCICE.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DisplacementDeltas not implemented for explicit coupling?
4 participants