First prototype of the dune-precice module (dune-adapter) for preCICE #1

maxfirmbach · 2021-09-29T13:55:15Z

Hello preCICE-team and developers,

this is a prototype of a module inside the DUNE framework, representing an adapter for the preCICE-library.
For now this module is mainly used for FE calculations, enabling multi-physics simulations within DUNE.

Literature references can be found in the README inside the dune-precice folder.

uekerman · 2021-10-01T15:10:34Z

Thanks for the contribution, @maxfirmbach 🎉
Why did you put everything in the subfolder dune-precice? Feel free to completely overwrite the preliminary README that is already on the main branch.

MakisH · 2021-10-06T09:27:13Z

@maxfirmbach thank you very much for the contribution! Before reviewing it, do you maybe have a case that we can run it with? If you want, you can also open a separate PR in the tutorials.

ajaust · 2021-10-06T14:21:23Z

This looks like a great addition to the adapter repositories. I have worked on an adapter for DuMuX which shares quite some functionality with the DUNE adapter. I tried to give an extensive overview in #2 .

maxfirmbach · 2021-10-07T07:12:11Z

@MakisH I can create a tutorial case and will do a PR in the tutorials (might however take some time). To run a test case the DUNE core modules and some extension modules are needed and the first installation of the whole DUNE environment can be a bit cumbersome.

ajaust · 2021-10-07T09:00:19Z

Regarding the installation of DUNE. There is a Spack recipe for DUNE. Alternatively, there is an installation script for DuMuX. The installation script also installs DUNE dependencies. The dependency list in line 134 needs to be adapted for the DUNE adapter to contain common, functions and istl (functions is currently missing as DuMuX does not need it). If one is not interested in DuMuX, one can comment out the corresponding parts or simply remove the dumux directory after the installation.

maxfirmbach · 2021-10-12T08:30:07Z

I added a case, where Dune acts as solid-participant in the perpendicular-flap tutorial case. The corresponding folder inside the tutorial directory should also be ready.

davidscn

I tried to install the module but there was a problem (see below)

The directory structure is actually fine (dune-precice).

davidscn · 2021-10-20T11:37:01Z

README.md

+https://github.com/maxfirmbach/dune-elastodynamics
+
+The adapter can be build with:
+`<path-to-dune-common/bin/dunecontrol> --current all`


I installed dune from apt, where dunecontrol is in the global searchpath:

Suggested change

`<path-to-dune-common/bin/dunecontrol> --current all`

`dunecontrol --current all`

Maybe link here also to the module installation instructions provided by DUNE itself. Is the dunecontrol make install command missing?

I guess that are the instructions on the website: https://www.dune-project.org/doc/installation/

README.md

davidscn · 2021-10-20T15:25:07Z

dune-precice/src/dune-perpendicular-flap.cc

+
+  // setup looping
+  double t = 0.0;
+  double dt = 0.0001;


Why do you need to select such a small time-step size? The tutorial uses actually 1e-2?!

An explicit RKN-method is used in this case and for larger time steps I had difficulties with stability. I guess the tutorial cases use an implicit method.

Ah ok. This probably explains the deviation of the displacement amplitude as well (I can run a brief comparison tomorrow). You don't have any other time-stepping methods available, right? All in all, I can run the tutorial case andI will have a closer look at the code tomorrow.

There should also be a family of Newmark methods (might have to clean that up a bit) and adaptive explicit RKN methods. But great that the thing runs at least.

maxfirmbach · 2021-10-21T06:14:59Z

I added a implicit time stepping case dune-perpendicular-flap-implicit.cc using Newmark's method and a time step size of 0.01, which should be the same as the openfoam fluid case.

I also cleaned up the time stepping method in dune-elastodynamics so before trying the new case, get a fresh pull of that please. I did a simulation, which looked fine ... I hope it works for you too.

davidscn · 2021-10-21T08:33:22Z

I added a implicit time stepping case dune-perpendicular-flap-implicit.cc using Newmark's method and a time step size of 0.01, which should be the same as the openfoam fluid case.

I also cleaned up the time stepping method in dune-elastodynamics so before trying the new case, get a fresh pull of that please. I did a simulation, which looked fine ... I hope it works for you too.

Yes, this works as well and it is faster and more compliant to the other tutorials. However, there is still some deviation between our other tutorials and the dune code here, even with the implicit time-stepper (as you probably know)

Do you have any ideas about the reasoning? Method-wise, you use continuous finite-elements of polynomial degree 2 and an implicit Newmark time-stepping with gamma=0.5 and beta=0.25, right?

maxfirmbach · 2021-10-21T09:05:41Z

Yes, I use continuous finite-elements of polynomial degree 2 and an implicit Newmark time-stepping with gamma=0.5 and beta=0.25. To be honest, I'm not sure why the result is different ... I already had a close look at all parts and to my best knowledge the structural part is correct. Still I couldn't figure out till now where the deviation comes from ...

Maybe a short remark, there's a file called hooketensor.hh. Due to the fact that the simulation is 2D there can be a plane stress or plane strain, right now plane stress is active ... maybe that also makes a difference.

davidscn · 2021-10-21T10:59:28Z

Ah that's nice, we use plane-strain. Switching this for the dune adapter give actually the correct results:

so please change this in your elastodynamics module. I will wait for the feedback of @ajaust here for now.

maxfirmbach · 2021-10-21T11:25:37Z

That looks really nice! I can't just change it ... cause for beam bending problems I need plane stress ... but I will find a way to call the right one.

Thanks for the help so far!

ajaust

Thank you again for the adapter and sorry for the late response. I like your work, but nevertheless left some critical remarks. You do not have to fix everything now, but I think there are some things we have to think about and maybe create issues for them to track them. I can also help with some things if needed. I left some remarks in the code.

In short, these are some of the general remarks:

Make function parameters constant where possible. C++ core guidelines
Mark functions that do not change any members as const. C++ core guidelines
Trivial types do not have to be passed by reference. C++ core guidelines
Could we add some simple test (maybe with a solver dummy)? Ideally with some test that might be run via a GitHub action (I would help with that). We can also move this into an issue for a future version.
The module name dune-preciceis good. I would call the repository dune-precice instead of dune-adapter. This would follow the standard nomenclature used by DUNE projects. It would make sense to change the layout as well by moving everything from the directory dune-precice in the root of the repository. This would be follow structure of other DUNE modules.
dune-precice should not depend on dune-elastodynamics in my opinion as the adapter itself does not depend on it. Only the supplied example depends on dune-elastodynamics and dune-preciceThis means we should do something with the supplied example in src/.
I did not try to compile or work with the supplied example yet.
Document the dependency on a preCICE version. Does it work with preCICE v1 or v2? Does it work with both version?
We should maybe add more documentation and some test (solverdummies)?!

ajaust · 2021-10-22T15:00:46Z

README.md

@@ -1,2 +1,31 @@
 # dune-adapter


I think the whole repository should be called dune-precice to follow standard DUNE notation. At the same time the structure of the repository should be adapted accordingly. Namely, all the content in the folder dune-precice should be in the root of the repository.

ajaust · 2021-10-22T15:15:43Z

dune-precice/dune.module

+Version: 1.0
+Maintainer: [email protected]
+#depending on
+Depends: dune-common (>= 2.7) dune-istl (>= 2.7) dune-functions (>= 2.7) dune-elastodynamics


Currently, this repository depends on dune-elastodynamics which does not make sense to me. This increases the number of dependencies while I would vote for dune-precice being a baseline implementation that can be used in other DUNE modules with minimum dependencies. Why would I care installing dune-elastodynamics if I do want to use dune-precice for a completely different type of application?

Therefore I would suggest

Suggested change

Depends: dune-common (>= 2.7) dune-istl (>= 2.7) dune-functions (>= 2.7) dune-elastodynamics

Depends: dune-common (>= 2.7) dune-istl (>= 2.7) dune-functions (>= 2.7)

and adding a simple example (dummysolver?) that purely relies on dune-precice and its dependencies.

I can see three scenarios at the moment:

Relaxing the current dependency Make the current example a suggested dependency and prevent automatic building of the example.

Suggested change

Depends: dune-common (>= 2.7) dune-istl (>= 2.7) dune-functions (>= 2.7) dune-elastodynamics

Depends: dune-common (>= 2.7) dune-istl (>= 2.7) dune-functions (>= 2.7)

Suggests: dune-elastodynamics (>=1.0)

In this case I would also rename the directory. src/ is not very verbose. Something like examples/elastodynamic/ would be better.

Move all examples that require further dependencis into its one DUNE module (dune-precice-examples?).

The current example could live in dune-elastodynamics and one could add dune-precice as a suggested dependency there.

The dune-precice module is now only depending on the core modules. I moved the examples to dune-precice-howto as this naming convention is also used for tutorial modules of certain core modules.

Considering dune-precice-examples I was thinking about cutting the dependency to dune-elastodynamics completely in the future and replacing it by dune-pdelab. A linear-elasticity example should be rather easy to construct there.

dune-precice/CMakeLists.txt

dune-precice/dune/precice/couplinginterface.hh

ajaust · 2021-10-22T16:17:24Z

dune-precice/dune/precice/couplinginterface.hh

+    for(int i=0; i<dune_to_precice.N(); i++) {
+      for(int j=0; j<dim; j++) {
+        if(isBoundary[i][j]) { write_data[iteration_count] = dune_to_precice[i][j]; ++iteration_count; }
+      }
+    }  


Could we avoid raw loops here and instead used std::for_each, for example? See Slide 3 or the actual talk. 😄

Is isBoundary[i] not true for all j? I do not know/understand the indexing?! I would expect i to be the index of the degree of freedom and j refer to the coordinates or so.

davidscn · 2021-10-27T09:27:15Z

@maxfirmbach do you have any idea which parts you want to address or if you want to address parts at all? Do you need help with some comments? If you don't want to keep working on this we could also merge and add a TODO list.

maxfirmbach · 2021-10-27T09:33:14Z

I can address the changes to the code, but I work quite unregulary on the dune-adapter ... so it always takes some time. Considering the dependencies and the structure of the tutorial examples etc., I guess that's a part, which shouldn't be decided by me. If a merge + ToDo List is ok, I'm fine with that, I will work on the adapter from time to time.

davidscn · 2021-10-27T12:20:51Z

Alright that's totally fine, then let's keep it as it is. I just want to prevent that things become stale.

maxfirmbach · 2021-11-11T09:15:20Z

I restructured the adapter and the tutorial cases. Now there are two DUNE modules inside the dune-adapter, namely dune-precice containing the adapter (depending only on the core modules) and dune-precice-howto containing the tutorial case (and it's dependencies). For now I think that is a good structure and separation.

ajaust

Thank you very much for the work. I went through the main adapter part and did not check the dune-precice. I like the state of the adapter a lot. I have just some minor remarks.

dune-precice/dune-precice/couplinginterface.hh

ajaust · 2021-11-14T14:23:53Z

dune-precice/dune-precice/couplinginterface.hh

+
+#include <dune/functions/functionspacebases/interpolate.hh>
+
+#include <precice/SolverInterface.hpp>


Does it make sense to include it inside the Dune::preCICE namespace to keep the scope of preCICE more narrow?

I'm not sure, does it make a big difference?

I am not sure what would be the best here. Including it within the namespace would hide preCICE and its precice namespace a bit more from users. From my perspective this would be something good since we want people to use the adapter. However, I did not find any examples regarding good and bad practices. This is for sure a small thing we can also easily change later. I just wanted to mention it.

Maybe @fsimonis has an idea/suggestion regarding the best practices regarding includes inside/outside namespaces?!

I think we can just keep this way, it is also more clear from an outside view (at least in my opinion), because you have all headers before starting to do anything.

dune-precice/dune-precice/couplinginterface.hh

davidscn · 2021-12-06T08:09:34Z

I didn't follow the discussions, but I think this PR is ready to merge, @maxfirmbach any concerns? Is the tutorial ready as well?

ajaust · 2021-12-06T08:14:04Z

I agree that it is ready. If there are problems, we can still discuss and change/fix things later.

maxfirmbach · 2021-12-07T07:10:18Z

From my side there are no concerns, at least for now. The tutorial should also be ready.

davidscn · 2021-12-07T07:31:39Z

Thank you for your contribution!

maxfirmbach added 3 commits September 29, 2021 15:47

inital push to develop

164605f

update README

f9a51ac

update README

dc5e509

move description to outer Readme

8ee8c56

ajaust mentioned this pull request Oct 6, 2021

Differences and similarities between DUNE adapter and dumux-precice #2

Open

maxfirmbach mentioned this pull request Oct 7, 2021

Create a perpendicular-flap tutorial case for Dune precice/tutorials#239

Merged

Add perpendicular-flap tutorial case

85928fc

davidscn self-requested a review October 20, 2021 11:28

davidscn requested changes Oct 20, 2021

View reviewed changes

maxfirmbach added 3 commits October 20, 2021 14:45

Add dune installation to readme

ca038df

set required dune version to 2.7

8d7cb53

Adjust dune-elastodynamics version

e756635

davidscn reviewed Oct 20, 2021

View reviewed changes

added implicit timestepping case

5a0ce3d

ajaust requested changes Oct 22, 2021

View reviewed changes

Restructure adapter and tutorials

38a5289

maxfirmbach added 2 commits November 11, 2021 15:09

First change requestions

25662ff

Second change requestions

e0931ad

ajaust reviewed Nov 14, 2021

View reviewed changes

Third change requestions

662f83b

davidscn approved these changes Dec 7, 2021

View reviewed changes

davidscn merged commit 5f2364d into precice:main Dec 7, 2021

	`<path-to-dune-common/bin/dunecontrol> --current all`
	`dunecontrol --current all`

	Depends: dune-common (>= 2.7) dune-istl (>= 2.7) dune-functions (>= 2.7) dune-elastodynamics
	Depends: dune-common (>= 2.7) dune-istl (>= 2.7) dune-functions (>= 2.7)

	Depends: dune-common (>= 2.7) dune-istl (>= 2.7) dune-functions (>= 2.7) dune-elastodynamics
	Depends: dune-common (>= 2.7) dune-istl (>= 2.7) dune-functions (>= 2.7)
	Suggests: dune-elastodynamics (>=1.0)


		#include <dune/functions/functionspacebases/interpolate.hh>

		#include <precice/SolverInterface.hpp>

First prototype of the dune-precice module (dune-adapter) for preCICE #1

First prototype of the dune-precice module (dune-adapter) for preCICE #1

Conversation

maxfirmbach commented Sep 29, 2021

uekerman commented Oct 1, 2021

MakisH commented Oct 6, 2021

ajaust commented Oct 6, 2021

maxfirmbach commented Oct 7, 2021

ajaust commented Oct 7, 2021

maxfirmbach commented Oct 12, 2021

davidscn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maxfirmbach Oct 20, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maxfirmbach commented Oct 21, 2021

davidscn commented Oct 21, 2021

maxfirmbach commented Oct 21, 2021 • edited Loading

davidscn commented Oct 21, 2021

maxfirmbach commented Oct 21, 2021 • edited Loading

ajaust left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davidscn commented Oct 27, 2021

maxfirmbach commented Oct 27, 2021 • edited Loading

davidscn commented Oct 27, 2021

maxfirmbach commented Nov 11, 2021

ajaust left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davidscn commented Dec 6, 2021

ajaust commented Dec 6, 2021

maxfirmbach commented Dec 7, 2021

davidscn commented Dec 7, 2021

maxfirmbach Oct 20, 2021 •

edited

Loading

maxfirmbach commented Oct 21, 2021 •

edited

Loading

maxfirmbach commented Oct 21, 2021 •

edited

Loading

maxfirmbach commented Oct 27, 2021 •

edited

Loading