tutorials icon indicating copy to clipboard operation
tutorials copied to clipboard

Partitioned heat conduction structure is non-standard and inconsistent

Open BenjaminRodenberg opened this issue 3 years ago • 10 comments

Background

In PR #223 there was already a discussion whether we should change the structure of our partitioned heat conduction tutorial or not (see https://github.com/precice/tutorials/pull/223#issuecomment-880537448). We decided to move this discussion to an independent issue, since it was off-topic for #223.

Current state

The current structure of this tutorial is inconsistent, since the OpenFOAM solver has independent solvers for Dirichlet and Neumann while the FEniCS and Nutils solvers don't:

Rough structure (from https://github.com/precice/tutorials/pull/223#issuecomment-882359957):

├── fenics
├── images
├── nutils
├── openfoam-dirichlet
├── openfoam-neumann
├── openfoam-solver
├── precice-config.xml
└── README.md

Possible solutions

Currently there are three main possibilities:

  1. keep it as it is. :+1:
  2. follow with all solvers the standard way, where you have an independent directory for every solver. We will replace fenics and nutils with fenics-dirichlet, fenics-neumann, nutils-dirichlet, nutils-neumann. This would be consistent with all/most of our other tutorials. :+1: :+1:
  3. try to only have a single OpenFOAM solver. We merge openfoam-dirichlet and openfoam-neumann into openfoam. As far as I understand we concluded in #223 that this is not possible. :-1:

Goal of this issue

We should agree in this issue on a structure that fits for cases like the partitioned heat equation, where there are not necessarily two independent solvers, but only a domain decomposition of a single solver happens. This structure should be as consistent with existing tutorials as possible and at the same time not introduce a large amount of code duplication (maintainment!).

BenjaminRodenberg avatar Jul 19 '21 11:07 BenjaminRodenberg

follow with all solvers the standard way, where you have an independent directory for every solver. We will replace fenics and nutils with fenics-dirichlet, fenics-neumann, nutils-dirichlet, nutils-neumann. This would be consistent with all/most of our other tutorials.

I would vote for this option. This opens the door for alternative contributions as well. Maybe some solver do not support all combinations of data reading and writing.

try to only have a single OpenFOAM solver. We merge openfoam-dirichlet and openfoam-neumann into openfoam. As far as I understand we concluded in Add partitioned-heat OpenFOAM participant with solver #223 that this is not possible.

If we explicitly want to run the case openfoam <-> openfoam, then this is not possible since the mesh is generated separately and stored in default directories. If we exclude the openfoam <-> openfoam case we could make it work using some custom run scripts where the relevant files are copied (i.e., activated) for the current simulation.

davidscn avatar Jul 19 '21 11:07 davidscn

If we exclude the openfoam <-> openfoam case we could make it work using some custom run scripts where the relevant files are copied (i.e., activated) for the current simulation.

This sounds to me like replacing one thing that is non-standard with two other things that are non-standard.

From my point of view option 2 is also the best one (if we want to change something).

BenjaminRodenberg avatar Jul 19 '21 11:07 BenjaminRodenberg

I just started adding :+1: and :-1: above. I understand that there are two :+1: for option 2 from @DavidSCN and me. There is one :-1: for option 3 from my side. And a :+1: on option 1 from my side - since this is no work. Feel free to edit this list (I think there is no need for a detailed breakdown where the votes come from).

BenjaminRodenberg avatar Jul 19 '21 11:07 BenjaminRodenberg

Among these options, I also support option 2 (fenics-dirichlet, fenics-neumann) for consistency and simplicity. I believe this is rather clear, we just didn't want to rename things in the same PR.

Regarding the naming, this is still inconsistent. If Dirichlet and Neumann are also the participant names, then we should name them dirichlet-fenics and neumann-fenics. Assuming that we always have e.g. Dirichlet being the left and Neumann the right.

What is not clear to me is where to store shared files (e.g. solver files). The distinction into openfoam-solver and openfoam-<case> goes already in a good direction, but I would prefer to group all these in a standardized subdirectory and not mix them with the cases. I believe that a <case>/tools/ would be intuitive enough.

MakisH avatar Jul 19 '21 11:07 MakisH

What is not clear to me is where to store shared files (e.g. solver files). The distinction into openfoam-solver and openfoam- goes already in a good direction, but I would prefer to group all these in a standardized subdirectory and not mix them with the cases. I believe that a /tools/ would be intuitive enough.

For me, tools is something which helps you with certain things such as cleaning or running, but which is not a significant part of the simulation itself. Hence, putting the solver into a tools directory for me counter intuitive and not the right place.

davidscn avatar Jul 19 '21 12:07 davidscn

What is not clear to me is where to store shared files (e.g. solver files). The distinction into openfoam-solver and openfoam- goes already in a good direction, but I would prefer to group all these in a standardized subdirectory and not mix them with the cases. I believe that a /tools/ would be intuitive enough.

For me, tools is something which helps you with certain things such as cleaning or running, but which is not a significant part of the simulation itself. Hence, putting the solver into a tools directory for me counter intuitive and not the right place.

Would a directory with a different name, such as solvers, be good enough?

MakisH avatar Jul 19 '21 12:07 MakisH

solvers is indeed very intuitive. However, I guess that this case here is rather an exception and I am not sure if we really put anything else there than this OpenFOAM solver (also in other cases). Or do we add fenics and nutils there as well? This would probably be inconsistent with other tutorial cases.

davidscn avatar Jul 19 '21 12:07 davidscn

As we are trying to extend our current structure to not have special cases, I am trying to imagine more similar cases. Such a case could also be the partitioned-pipe, if we added more cases.

This also originates from the wide range of "adapter" definitions. If solver == adapter and the solver is only tailored a particular scenario (e.g. partitioned HT), we have two cases:

  • (A) can the same adapter/solver act as either partiticipant of this case? --> Should be at the <case> directory level.
  • (B) is the adapter/solver specific to a participant? --> Should be at the <case>/<participant>-<solver> directory level.

We currently have "fused" solvers (A) e.g. in fenics and nutils. Transforming them to (B) would introduce some duplication, but avoid the whole discussion about where to put the solver. We could also transform the openfoam-solver to (B) and then we can store it inside <participant>-<solver> and stick to our general convention, without changing anything.

MakisH avatar Jul 19 '21 12:07 MakisH

I prefer duplication here over software engineering. Splitting /duplicating the nutils code into two variants is very easy and could also make the code easier to understand. Putting some of the code into a solvers would make things complicated to read. I expect additional lines of code for the software engineering > saved lines code by avoiding the duplication (for nutils).

Regarding the naming, this is still inconsistent. If Dirichlet and Neumann are also the participant names, then we should name them dirichlet-fenics and neumann-fenics. Assuming that we always have e.g. Dirichlet being the left and Neumann the right.

Yes, good point. Your last remark should be indeed a hard definition of this tutorial.

uekerman avatar Jul 20 '21 10:07 uekerman

Just to clarify: A solvers or tools directory would only be needed for OpenFOAM. Unless we also want to duplicate that one and distribute it in the <case>/<participant>-openfoam directories.

MakisH avatar Jul 20 '21 11:07 MakisH

This issue is quite old, but to me it reads like we actually concluded on option 2 in the end. Meaning: The action item is to split nutils into nutils-dirichlet and nutils-neumann (same for fenics). Main argument in favor: Simplifies the code because it only needs to deal with either Dirichlet or Neumann in each folder. Please :+1:, if this is summarizing the discussion from above or comment, if I'm missing something.

We don't have to start working on this now, but I think it would be good, if we could clearly move from the discussion phase into the doing phase here.

BenjaminRodenberg avatar Jan 18 '24 13:01 BenjaminRodenberg

I read again the whole thread. My summary:

  • [ ] Rename {openfoam-dirichlet, openfoam-neumann} to {dirichlet-openfoam, neumann-openfoam} and check for similar instances in other tutorials.
  • [ ] Split nutils into {dirichlet-nutils, neumann-nutils}
  • [ ] Split fenics into {dirichlet-fenics, neumann-fenics}

Even though not crystal clear from the discussion above, I would also add:

  • [ ] Move openfoam-solver to solvers/openfoam and check for similar instances in other tutorials (e.g., Quickstart)

I would indeed keep the FEniCS and Nutils as duplicate scripts, since this makes it easier for a user to extend/replace when preparing their own case.

Please upvote/downvote.

MakisH avatar Jan 19 '24 12:01 MakisH

Documented the additional rules and guidelines in https://github.com/precice/precice.github.io/pull/331

Overview of inconsistencies in https://github.com/precice/tutorials/issues/461

MakisH avatar Feb 07 '24 15:02 MakisH