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

Provide initial data for perpendicular flap case #540

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

Conversation

BenjaminRodenberg
Copy link
Member

When @NiklasVin and I evaluated the convergence order of the FEniCS-based solid solver using fluid-fake the missing initialization actually cost us quite some time. I think it is generally a good idea to initialize the data when using higher-order time stepping. If we want to generally apply initialization for this case, all adapters have to support this feature since otherwise the tutorial case might break for certain combinations.

This is also related to my comment here and to precice/precice#2033.

Checklist:

  • I added a summary of any user-facing changes (compared to the last release) in the changelog-entries/<PRnumber>.md.
  • I will remember to squash-and-merge, providing a useful summary of the changes of this PR.

Copy link
Member

@uekerman uekerman left a comment

Choose a reason for hiding this comment

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

I am still a bit doubtful whether this is really the way to go. I think the problem you encountered was due to the not-at-rest initial conditions of your fake solvers.
IIRC, I did not need to initialize data for the convergence studies with Nutils as fluid solver.

perpendicular-flap/convergence-studies/.gitkeep Outdated Show resolved Hide resolved
<exchange data="Force" mesh="Solid-Mesh" from="Fluid" to="Solid" />
<exchange data="Displacement" mesh="Solid-Mesh" from="Solid" to="Fluid" />
<exchange data="Force" mesh="Solid-Mesh" from="Fluid" to="Solid" initialize="true" />
<exchange data="Displacement" mesh="Solid-Mesh" from="Solid" to="Fluid" initialize="true" />
Copy link
Member

Choose a reason for hiding this comment

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

Why initializing the displacement? The solid is initially at rest, no? Maybe the fake solid does not mimic this behavior?

Copy link
Member Author

Choose a reason for hiding this comment

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

The displacement is not relevant for the fake solid solver (it ignores data it reads). But I'm also suggesting to generally initialize here.

Generally, I think explicitly initializing with zero is much safer than implicitly initializing with zero like we do it currently. Therefore, I would prefer to initialize all exchanged data (potentially with zero). See precice/precice#2033

Copy link
Member

Choose a reason for hiding this comment

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

The problem with this that will bite us is that then the first mapped and communicated data is zero. I am afraid that users will be very confused and wonder why we waste the resources.

Copy link
Member Author

Choose a reason for hiding this comment

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

In the end this is a question of priorities: If we want the default case to offer maximum performance at the risk of being less flexible while maintaining accuracy then we should not initialize. If we want to allow for customization of the case to some degree without introducing errors we should initialize. I'm leaning towards prioritizing accuracy and flexibility over performance. I would also argue that potentially wasting resources once in the beginning of the simulation is not as bad as introducing an error at the beginning from which you cannot recover.

Copy link
Member

Choose a reason for hiding this comment

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

I don't agree on the flexibility argument. Our current solution is flexible, user can initialize data if they want.

In the end, the tutorial setup fixes whether non-zero data needs to be initialized or not. For the perpendicular flap, this is not the case. If a user wants to alter the case, they have to alter the configuration as well.

Besides performance, my main concern is that users will wonder why initial data is explicitly initialized if it is zero anyway.

Copy link
Member Author

Choose a reason for hiding this comment

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

Then let's maybe phrase what I mean with flexibility differently: Our current scenario requires you to do (and remember) more additional steps when you want to use non-zero data because the default case assumes zero data.

We can avoid having to do these additional steps, but this costs us performance and might look unnecessary to the users.

I think the key question is how we want to resolve precice/precice#2033.

@@ -46,8 +46,8 @@
<time-window-size value="0.01" />
<max-time value="5" />
<participants first="Fluid" second="Solid" />
<exchange data="Force" mesh="Solid-Mesh" from="Fluid" to="Solid" />
<exchange data="Displacement" mesh="Solid-Mesh" from="Solid" to="Fluid" />
<exchange data="Force" mesh="Solid-Mesh" from="Fluid" to="Solid" initialize="true" />
Copy link
Member

Choose a reason for hiding this comment

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

Initializing the force could be tricky. How would you do it for a non-fake fluid solver? I guess an example could help here, e.g. OpenFOAM.

Copy link
Member Author

Choose a reason for hiding this comment

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

OpenFOAM would still initialize with zero forces. That's also working out of the box with this configuration.

@@ -74,7 +74,7 @@ def neumann_boundary(x, on_boundary):
precice = Adapter(adapter_config_filename="precice-adapter-config-fsi-s.json")

# Initialize the coupling interface
precice.initialize(coupling_boundary, read_function_space=V, write_object=V, fixed_boundary=fixed_boundary)
precice.initialize(coupling_boundary, read_function_space=V, write_object=u_n, fixed_boundary=fixed_boundary)
Copy link
Member

Choose a reason for hiding this comment

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

Related to this PR?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. If you want to initialize the values in the FEniCS case you need to provide a function, not only the function space at initialization time.

@BenjaminRodenberg BenjaminRodenberg self-assigned this Oct 19, 2024
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.

3 participants