Check for (in)compressible solver type with Pressure dimensions

MakisH commented 6 years ago

Currently, we check the solver type based on the registered IOobjects. This is a very indirect way to check for compressible vs incompressible vs basic solvers, but it is maybe a not-so-bad way to check for the available models (classes we can access).

https://github.com/precice/openfoam-adapter/blob/c0f28814c4d8da073a73cd076dc4e1dfd5da7e74/CHT/CHT.C#L109-L196

However, this is a complicated way and maybe it would be easier to just check for the dimensions of Pressure to distinguish between compressible and incompresible solver. After this check, we can check if it is a basic solver (e.g. laplacianFoam) or an of an unknown type.

For compresible solvers, the dimensions of Pressure are [1 -1 -2 0 0 0 0], while for incompressible [0 2 -2 0 0 0 0].

JSeuffert commented 5 years ago

In the FSI-module we need something similar again to calculate the forces for all solver types. Can we determine the solver type somewhere outside the modules to avoid this duplication?

davidscn commented 4 years ago

@MakisH Should we move this in the adapter class instead?

After this check, we can check if it is a basic solver (e.g. laplacianFoam) or an of an unknown type.

This sounds like you know how to do that. Do you have anything in mind?

MakisH commented 4 years ago

Should we move this in the adapter class instead?

On the one hand, it would be nice to not duplicate the same code for every module. On the other hand, if we added more exotic modules, a shared checker could be difficult to maintain. Since right now a shared checker would be perfect and since we would only have problems with very exotic solvers, let's try to separate this from the modules.

I currently don't like that a very large part of the code is inside the adapter class. However, it looks like the best place to go at the moment.

After this check, we can check if it is a basic solver (e.g. laplacianFoam) or an of an unknown type.

This sounds like you know how to do that. Do you have anything in mind?

Looking back, I am not sure if the distinction between "basic" and "unknown" makes much sense in a wider set of solvers. In most cases, "unknown" would just mean "something is terribly wrong, could not guess the solver type".

With this in mind:

We should probably just assume "basic" in that case, throw a warning, and let OpenFOAM crash later on.
To silence the warning, the user could just explicitly define the type of the solver.
Even further, we should probably rename "basic" to "other".
In the CHT module, an "other"/"basic" solver just looks up the respective parameters in the preciceDict. This is a pattern we could reuse in other modules.

davidscn commented 4 years ago

However, it looks like the best place to go at the moment.

Alright.

Looking back, I am not sure if the distinction between "basic" and "unknown" makes much sense in a wider set of solvers.

Be aware, that we have e.g. for a "basic" solver type specific modules e.g. in the HeatFlux module and for "unknown" solver types not.

We should probably just assume "basic" in that case, through a warning, and let OpenFOAM crash later on.

Referring to the issue topic: Solely using the pressure dimension, we can not distinguish between an incomressible solver and a basic solver. Both have perhaps (e.g. potentialFoam) the same dimension.

To silence the warning, the user could just explicitly define the type of the solver.

Is this feature already implemented or on the ToDo list? I have not seen any specification in the preciceDict.

Even further, we should probably rename "basic" to "other".

Still not sure about the difference in the modules.

MakisH commented 4 years ago

Looking back, I am not sure if the distinction between "basic" and "unknown" makes much sense in a wider set of solvers.

Be aware, that we have e.g. for a "basic" solver type specific modules e.g. in the HeatFlux module and for "unknown" solver types not.

This is what I meant with my last point. We keep this functionality, we just name the solver type "other" instead of "basic" and expect reading the parameters from preciceDict.

We should probably just assume "basic" in that case, throw a warning, and let OpenFOAM crash later on.

Referring to the issue topic: Solely using the pressure dimension, we can not distinguish between an incomressible solver and a basic solver. Both have perhaps (e.g. potentialFoam) the same dimension.

This is fine, it will eventually crash (early) if a strange solver is used and is not correctly identified. Most of the cases will either be compressible or incompressible. If we want to catch the "other" solver type as well, maybe we should add another check. But potentialFoam sounds like a solver we don't need to target or worry about.

To silence the warning, the user could just explicitly define the type of the solver.

Is this feature already implemented or on the ToDo list? I have not seen any specification in the preciceDict.

Yes, see the wiki. We also already use it in the (latest state of) heated-plate tutorial for laplacianFoam.

Even further, we should probably rename "basic" to "other".

Still not sure about the difference in the modules.

There should not be any difference right now. Just renaming.

precice / openfoam-adapter

Check for (in)compressible solver type with Pressure dimensions #31