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

Easily switch off preCICE #99

Open
uekerman opened this issue Apr 22, 2021 · 8 comments
Open

Easily switch off preCICE #99

uekerman opened this issue Apr 22, 2021 · 8 comments

Comments

@uekerman
Copy link
Member

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?

@fsimonis
Copy link
Member

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.

@uekerman
Copy link
Member Author

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.

@IshaanDesai
Copy link
Member

IshaanDesai commented Apr 22, 2021

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.

@BenjaminRodenberg
Copy link
Member

BenjaminRodenberg commented Apr 22, 2021

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.

@ajaust
Copy link
Collaborator

ajaust commented May 19, 2021

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.

@IshaanDesai
Copy link
Member

Another code example why this feature is needed

@BenjaminRodenberg
Copy link
Member

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).

@IshaanDesai
Copy link
Member

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.

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

No branches or pull requests

5 participants