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

Port preCICE example to v3 #168

Merged
merged 6 commits into from
Feb 21, 2024
Merged

Conversation

davidscn
Copy link
Contributor

@davidscn davidscn commented Feb 6, 2024

preCICE version 3 comes with a couple of breaking changes. This PR applies all changes necessary. Our porting guide still needs some clean-up, but should help with such things in the near future.

With the current changes, everything works technically and I would expect the same results as obtained with the v2.5.0 (CI ref data). However,

  • some code comments still need an update
  • for exemplary purposes, I would like re-arrange the order of some API calls. It shouldn't make a difference with this trivial example case, but might lead to misunderstandings for realistic examples. See also the detailed description in our doxygen docs
  • Not sure about the formatting changes here. I only have clang-format 15 as of now (assuming the clang-format is just the same as the main deal.II project)

@davidscn davidscn mentioned this pull request Feb 6, 2024
@marcfehling
Copy link
Member

I'm sure users who would like to use precice and deal.II will at some point stumble over your code gallery program. So please adjust it in all means you find helpful for understanding the code. I second your suggestions on updating comments and changing the order of API calls.

Regarding indentation: That choice is entirely up to you. I like the fact that you would like to adhere to a certain indentation style. In deal.II we currently use clang-format 16 (see here), but it's fine if you use the tools you have available.

Also, feel free to take over/cherry-pick the commit from #167 to have a coherent patch.

@davidscn davidscn marked this pull request as ready for review February 9, 2024 18:16
@davidscn davidscn changed the base branch from precice to master February 9, 2024 18:17
@davidscn
Copy link
Contributor Author

davidscn commented Feb 9, 2024

Thanks for the reply. Changes went now a bit bigger than anticipated, as the handling of time and time-related data becomes more explicit. I changed the fancy_boundary_condition to actually provide data related to t=0.1 and made the data of t=0 optional (it's only used in case initialize="true" is used in the preCICE configuration file, which is not the case by default). With this, the association of time and the boundary condition is actually correct and also the relevant data for the Laplace participant, as it uses a backward Euler scheme, i.e., coupling data is required at the end of each time window.

Due to the changes time management, I had to update the reference/CI data as well. I verified, that all technical changes lead to the same results obtained with v2.5.0.

Ready for review.

@marcfehling
Copy link
Member

Thank you for the updates!

@marcfehling marcfehling merged commit b3078bb into dealii:master Feb 21, 2024
@davidscn davidscn deleted the port-precice branch February 21, 2024 07:03
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.

2 participants