-
-
Notifications
You must be signed in to change notification settings - Fork 8
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
Compatibility update for dolfinx 0.9.0 and preCICE v3 #28
Conversation
…d it seems like the coupling boundary is the main issue for the errors but it converges and looks good otherwise
Thank you for this update! I changed 0.8.0 in the description to 0.9.0 since you also mention 0.9.0 in the title of this PR. I will close #13 now assuming that the PR is outdated and superseeded by this one. I think any valuable changes in #13 should be moved over here if possible. Please also update the README and mention FEniCSx Version 0.9.0 as dependency. |
We should merge #26 such that the contribution of @efirvida shows up in the history. I see the danger that we move ideas from #26 into this PR and then #26 will never be merged into the history. I don't have a very good overview here but I see #28 as a maintainment PR for compatibility with FEniCSx 0.9.0 (also updating the partitioned heat equation tutorial) and #26 as a feature PR (adding the perpendicular flap). I see two options (depends on how crucial the ideas contributed by #26 for #28):
|
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.
I updated the tests and CI to work with this PR. Also added some minor comments. This is not a complete review.
I think it's already good news that the partitioned heat conduction test in the CI works 🎉
edit: at least it worked for a2c6c85. Not sure why I does not work anymore since the changes are unrelated.
* rename tutorials/fenicsx to tutorials/solver-fenicsx * only use a single requirements.txt
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.
I ran the tutorial and looks good. There are still some problems with errors and iteration count but let's deal with this at a later point in time.
I also fixed some tests.
Note that running the tutorial does not work out of the box since you need to setup the venv manually (remove fenicsxprecice
from requirements.txt
and install it with pip3 install ../../../.
). But I also do not think we need this since the tutorial will be moved somewhere else at a later point in time.
What worries me a bit: From my perspective bf5eda2 is necessary that the tests pass. At least locally I get errors if I do not apply this fix. However, the CI passes apparently for c40fb25. We should investigate this more closely in the future since this means that our tests do not fail as expected. |
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.
I went through the adapter code changes, and they look reasonable 👍 I did not test the tutorial case as others have already done so. Do we want to merge the partitioned-heat-conduction case files?
Definitely! The tutorial structure also needs an update. The diff is a bit hard to read but I checked it again and there are no files that have been added accidentally. I will merge this PR now. |
Updates the fenicsx-adapter such that it is compatible with dolfinx
0.8.00.9.0 and preCICE v3.The structure of the current state stays more or less the same but I am planning to add ideas and features from #26.
The changes are also resolving #17.
The changes also include the issues addressed in #13. Unfortunately, I saw this PR too late and implemented more or less exactly the same
Todos