python-bindings icon indicating copy to clipboard operation
python-bindings copied to clipboard

Easily switch off preCICE

Open uekerman opened this issue 3 years ago • 8 comments

Oftentimes, you want to run a coupled code in single-physics mode, so without preCICE. For codes like OpenFOAM or FEniCS, we would handle this in the adapter. As the Python bindings are also used in codes without adapters (e.g. Nutils) and the codes don't need compiling (Python), it would be really great if there was a simple "switch off" mechanism.

Say

import precice
precice.turn_off()
...
interface = precice.Interface("Solid", "../precice-config.xml", 0, 1)
...
vertex_ids = interface.set_mesh_vertices(mesh_id, vertices)

Then all preCICE calls should be NOPs. This prevents us from polluting compact codes with a lot of

if(precice_used):
   interface = precice.Interface("Solid", "../precice-config.xml", 0, 1)

Is there a standard Python way how to do this? If not, could we handle it in the Python bindings?

I don't think we need this feature for other bindings as there we have longer codes and/or adapters and we compile. Do you agree?

uekerman avatar Apr 22 '21 11:04 uekerman

Let me first state the obvious downsides of such a feature:

  • The code becomes misleading as it states things it doesn't do
  • The code becomes difficult to maintain for future generations
  • This feature is not present in other bindings

Wouldn't it be easier to refactor the code by breaking it up into general, precice-on and precice-off chunks? Then you use precice_used to select between these chunks in the background. This can also be done with a class hierarchy.

fsimonis avatar Apr 22 '21 11:04 fsimonis

I see the points. I really meant it for very short codes, such as https://github.com/precice/tutorials/blob/master/flow-over-heated-plate/solid-nutils/solid.py Here, a class hierarchy or if-else code blocks would make things more complicated and less readable.

uekerman avatar Apr 22 '21 12:04 uekerman

This prevents us from polluting compact codes

I can provide another example of such polluting code here. In this code I have a single python script to do the single-physics as well as coupled simulation. This is a relatively small code and adding if-else blocks does not look too bad to me. ~~I would vote against implementing this feature in the bindings as this would make the bindings unnecessarily complicated.~~ changed my opinion on this, really like the idea Benjamin Rodenberg has suggested below, it would be indeed helpful.

IshaanDesai avatar Apr 22 '21 12:04 IshaanDesai

We could have a precice_noop and just do

import precice_noop as precice

this would keep the python bindings clean and it still handles your use case.

Not sure whether I would provide this under precice/python-bindings or just as an independent package. Most users probably don't need it and this would keep everything independent.

BenjaminRodenberg avatar Apr 22 '21 14:04 BenjaminRodenberg

I just found this issue now. I would mostly agree with with @fsimonis and if necessary, I would prefer the approach of @BenjaminRodenberg with precice_noop.

At the moment I would have the following, additional critical remarks:

  • How often is "oftentimes"? The only example so far that has a "noop" feature in the code is the example of Ishaan. Here, the author could have solved that with a preCICE-dummy class as proposed by @BenjaminRodenberg.
  • Why is this a feature for the bindings? Would this not fit into preCICE itself? If you want that feature for all solvers, would it not be easier to have a noop option in preCICE. In this case one does not have to implement this in any adapter nor in the bindings.
  • Having a noop feature for all bindings and adapters, instead of only in preCICE, will potentially lead to a lot of additional code and thus additional places for bugs.
  • Why should adapter fulfill several purposes: Supplying an interface to preCICE and mocking an interface to preCICE. Would it not be better to have these two features separated?
  • If this feature is not suitable for preCICE, it should maybe stated that such a feature is available in all bindings and that it is expected by all adapters. Otherwise this would affect the consistency of user experience.

ajaust avatar May 19 '21 13:05 ajaust

Another code example why this feature is needed

IshaanDesai avatar Jul 06 '21 09:07 IshaanDesai

Another code example why this feature is needed

Can you elaborate or point to a more specific place? Looking at the code I don't directly see the reason (I only looked very quickly and carelessly, though).

BenjaminRodenberg avatar Jul 06 '21 11:07 BenjaminRodenberg

Can you elaborate or point to a more specific place? Looking at the code I don't directly see the reason (I only looked very quickly and carelessly, though).

Sure, if you search if coupling in the code, you will find five instances where the program flow is disturbed because the intention is to have one single code for single-physics and coupled-physics. The reason for having a single code is mainly software maintainability, i.e. whatever physics is tested in single-physics can be directly used in the coupled case. The code currently is very basic and involves serial-explicit coupling, I can predict that the workflow will get even more complicated when implicit coupling is implemented.

IshaanDesai avatar Jul 06 '21 11:07 IshaanDesai