-
-
Notifications
You must be signed in to change notification settings - Fork 20
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
Fixed subcycling in implicit modal dynamic simulations #105
Conversation
What would you like me to review, exactly? Because I still get a mesh failure in OpenFOAM (but I assume this is what you meant with "partial fix"). Using a timestep size of 3e-3 for the perpendicular flap, I get 163 steps. Applying a "warp by vector" (with displacement) filter on the CalculiX results (converted from FRD), the beam keeps getting longer, until it breaks down to an arbitrary shape.
I remember trying a similar investigation for the OpenFOAM adapter, where I used the
Does this mean that this fixes #9? Because for me, it does not look like it.
This worries me more. Maybe now CalculiX writes only in the beginning and not at the end? Maybe you can compare result files and see if they correspond at the same time. I have not looked at the code yet, but I don't think I have enough knowledge of it to review it. Edit: Looked at the code, I guess the main idea is to have separate read/write checkpoint functions for the modal analysis case, duplication which sounds like a source of future issues. But I understand that it may be needed here. Why not apply the same fixes for all simulations? |
Actually, not that much now, mostly aesthetics (does it make sense to store the watchpoint in that data structure? Would a separate structure be cleaner ? Etc)
Did you run a modal simulation? (
I don't quite understand the approach. What would I gain compared to logging the times where the
First steps are matching but an extra step is added.
An opinion of @KyleDavisSA would indeed be welcome. |
You are right, I forgot to enable the modal simulation mode. In that case, it looks like it solves the issue! 😄
Probably none. Indeed, different approaches are easier in this case. |
@@ -216,6 +222,24 @@ void Precice_ReadIterationCheckpoint(SimulationData *sim, double *v); | |||
*/ | |||
void Precice_WriteIterationCheckpoint(SimulationData *sim, double *v); | |||
|
|||
/** | |||
* @brief Reads iteration checkpoint (in dyna_precice) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need a comment for the motivation for the duplication here (and in the writing method). What is special in modal simulations?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Saved data is different. The saved data in modal simulations doesnt exist in regular ones.
But an if(isModal) inside would work too. Except that the number of arguments changes too, so I'm not sure
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great motivation, let's add it to a comment!
// Reload DOFs in eigenmodes space & counters | ||
memcpy(dofs, sim->eigenDOFs, sizeof(double) * nev); | ||
memcpy(derivatives, sim->eigenDOFsDerivatives, sizeof(double) * nev); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't it make more sense to add these in the main read/write checkpoint methods and only enable them if this is a modal simulation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That would probably mess up the arguments.unless i move the calculix data into the SimData, but that would be a bit messy and a break of separation of concerns in my opinion
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you feel that this would be more complicated, we can also leave it like this and look at it again in the future.
I've got a running draft of a change that solves the output issue. Since we can't override previously written output, I started storing intermediate results, then at the end of a time window that converged ( Since CalculiX is pretty much a blackbox in terms for understanding the output code, I designed a flexible buffer to handle changes if needed, as I didn't know exactly what had to be stored. The code is still a bit messy, and I'd like some conceptual approval on it before investing time on polishing the details. This buffer code should be reusable for the non-modal dynamic code later :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With the updated state:
- I get 500 output files instead of 163
- The modal case works, the non-modal remains the same
Regarding the big picture, I think the need and mechanics of the OutputBuffer need to be better documented. Since CalculiX does not expect to go back in time, I can understand that there is such a need for an OutputBuffer specifically for the adapter.
Of course, if there is a simpler way, it would be nice to save the additional complexity.
//if (iout == 2) { | ||
/* Adatper: TODO, integrate configurable output frequency */ | ||
if (1) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cleanup
I see how its internals should be documented, but where would you document the need? Inside the dyna_precice.c code ? That's actually why I left the commented |
I would just write a few lines at the beginning of the |
I did some refactoring & documentation. I still need to maje changes related to Calculix's configurable input frequency, but the Buffer code seems ready to me. |
I tested this PR for the perpendicular-flap case with various sub-cycling. If I remember correctly, it was failing when one time step was not a multiple of the other. I had no problems this time around. Am I remembering this correctly. Also, where can I find an example for the modal dynamic case? |
This should produce a correct output with all step sizes, but with visualization artifact (at least in ParaView with ccx2paraview as converter), as every outputted thing is seen as a unit step. Modal case is the perpendicular flap with |
Partial fix to #9 (which was first noticed for non-modal dynamic simulations)
Tried on the perpendicular flap modified to have smaller steps than the time window size. (I tried 0.5 * window size and 0.2. Non-exact dividers like 0.4 might be worth trying too).
I'm still confused by the output frequency: in the current state, it outputs only once per time window instead of once per step, and there is no extra writes from removed iterations. I'm very confused by the role of
iinc
andjprint
, and I can't tell if the write happens after the first step of an iteration or at the end of it. Investigation still ongoing.Result-wise, it's quite convincing and seems to match correctly the result with bigger steps.
I also noticed that with subcycling I get one less VTK step outputted than with full steps.