Open MakisH opened 6 years ago
Hi Makis,
In my opinion the ideal openfoam adapter should be as portable as possible and easy to use. Therefore, the first solutions does not really work, because the user has to select the right branch.
The fourth option is the most elegant solution, but has a major flaw in my opinion. If the user wishes to support multiple openfoam versions, this approach does not work anymore.
Therefore, I think either the second or third option would be best.
The fourth option is the most elegant solution, but has a major flaw in my opinion. If the user wishes to support multiple openfoam versions, this approach does not work anymore.
I am not sure I understood this part. Why will it not work?
If I understand correctly, Configuring the make with Cmake will build the adapter according to the system configuration. So it will build the adapter using the openfoam version which is loaded at that moment.
However, if the user then wishes to switch to another version of openfoam, the adapter will not work, am I correct?
But building one adapter for multiple versions of OpenFOAM would not work eitherways. Different executables need to be produced and each of them will be written in the $FOAM_USER_LIBBIN
, which is different for every OpenFOAM installation.
If the user wishes to switch to another version of OpenFOAM, she would need to build the adapter also for this version, which is easy. The portability here mainly means having one source package that can adapt to the requested OpenFOAM version at the time it is being compiled.
You're right! I forgot the fact that the adapter needs to build a seperate dynamic library in the $FOAM_USER_LIBBIN
for every build.
In that case, the fourth solution would be best indeed. The user would need to build the adapter once for every openfoam installation.
Of course, the additional configuration is still an open question for now.
An interesting (but short) discussion on cfd-online: Foam Version as a compile-time macro. There, it is suggested that one uses Git branches instead of pre-processor statements. I would, as well, agree.
Mentioning this issue to @shkodm, he came up with the idea of using C++ templates, where the type would be the OpenFOAM version (which could be set once using macros). We did not check, but this could hide the non-used implementations from the compiler.
An abstract idea, at the moment, but thinking in design patterns in the code and not in building techniques is also a very nice idea.
Another quick suggestion from @fsimonis: use C++ traits.
@MakisH One could try to detect the version using the detection idiom.
Useful environment variables defined by WMake at build time:
-DOPENFOAM_PLUS=1712
-DOPENFOAM=1806
-DOPENFOAM=1812
No such variable is defined for OpenFOAM 5.x / 6.
The following condition seems to successfully distinguish between .com
and .org
, used to patch #27:
#if defined OPENFOAM_PLUS || (defined OPENFOAM && (OPENFOAM > 1000))
...
#endif
I expect that .org
could also define and start using OPENFOAM
one day, I just hope that the versioning systems of the two variants will remain the same for a few years.
Useful environment variables defined by WMake at build time:
- OpenFOAM v1712:
-DOPENFOAM_PLUS=1712
- OpenFOAM v1806:
-DOPENFOAM=1806
- OpenFOAM v1812:
-DOPENFOAM=1812
No such variable is defined for OpenFOAM 5.x / 6.
There are a possibility to detect the openfoam version and vendor. In etc/bashrc both openfoam variants are setting the version number by the variable WM_PROJECT_VERSION. Also use a different for develop branch. openfoam.com starts the version with an "v". openfoam.org only sets a major number of foam version.
There are only a little change in the wmake file with lines as
## detect openfoam vendor
# OpenFOAM_VENDOR == 0: unknown
# OpenFOAM_VENDOR == 1: .com
# OpenFOAM_VENDOR == 2: .org
if [[ ${WM_PROJECT_VERSION:0:1} == "v" ]] ; then
[ -n "$OpenFOAM_VENDOR" ] || OpenFOAM_VENDOR=1;
[ -n "$OpenFOAM_VERSION" ] || OpenFOAM_VERSION="${WM_PROJECT_VERSION:1}"
else
[ -n "$OpenFOAM_VENDOR" ] || OpenFOAM_VENDOR=2;
[ -n "$OpenFOAM_VERSION" ] || OpenFOAM_VERSION="${WM_PROJECT_VERSION}"
fi
ADAPTER_PREP_FLAGS="${ADAPTER_PREP_FLAGS}-DOpenFOAM_VENDOR=${OpenFOAM_VENDOR}-DOpenFOAM_VERSION=${OpenFOAM_VERSION}"
An additional header OpenFOAMSETTINGS.H with the lines
#ifndef OpenFOAMSETTINGS_H
// OpenFOAM_VENDOR values are
#define OpenFOAM_Vendor_Unknown = 0
#define OpenFOAM_Vendor_dotCOM = 1
#define OpenFOAM_Vendor_dotORG = 2
#endif
#define OpenFOAMSETTINGS_H
helps for correct detections.
To use only the version number and the hobe .org will always a version number < 1000 is risky in my eye's and non clear solution.
@MakisH An opinion on option 1, I can imagine this must be difficult for you guys to maintain, because in the LuxDEM team we want to have modified OF adapter available for different versions as well. But due to the difficulty in maintainability we are not able to do this.
But this is of course because we use volume coupling, once volume coupling is a default feature in the adapters it might get easier to maintain different version as you do now (option 1).
If there are other users who modify the OF adapters for there applications, option 1 heavily restricts you to a particular version (from difficulty in maintenance perspective).
@Alphaoo1 in the meantime, we have defined a release strategy: https://github.com/precice/openfoam-adapter/issues/52
Supporting the .org
with the same code as with the .com
(or just multiple .org
versions with the same code) is becoming more and more difficult. The only possible exit I see now is a complete redesign of the adapter to be able to separate the "adapter-specific" from the "openfoam-specific" files. But since the .com
variant keeps a stable API already for so long, I don't see any reason for opening this box now.
Just for the record:
An opinion on option 1, I can imagine this must be difficult for you guys to maintain, because in the LuxDEM team we want to have modified OF adapter available for different versions as well. But due to the difficulty in maintainability we are not able to do this.
Git should make maintenance possible, if you rebase on every release and do not diverge for too long. Not trivial or even easy, but definitely possible. And yes, definitely, integrating features back into the mainline is the best way forward for all of us!
If there are other users who modify the OF adapters for there applications, option 1 heavily restricts you to a particular version (from difficulty in maintenance perspective).
That's why we now make this version preference a bit more concrete. If the user does not already have a preference, they can use this to make their decision. If you really need one of the version-specific branches, then a three-way comparison (e.g. with Meld) would help.
@Alphaoo1 do you have published code for your volume coupling?
@engenegr You can find the PR here.
Thanks a lot
A suggestion by @FG-TUM to create patches: https://github.com/coccinelle/coccinelle (there are apparently also similar tools)
Often users are bound to specific OpenFOAM variants (e.g. openfoam.com, openfoam.org, foam-extend) and versions (not always the latest, sometimes very old). However, often small or not-so-small differences will give errors during compilation. Therefore, we need a system to be able to support different versions at compile time.
Implementing such a system would help a lot dealing with issues such as #21, #8, #9, #26. A list of currently supported versions can be found in our wiki.
Fortunately, all the three main variants set the environment variables
WM_PROJECT
(OpenFOAM
for .com and .org,foam
for foam-extend) andWM_PROJECT_VERSION
(4.1, 5.0, 6 (no dot), ...
anddev
for .org,v1712, v1806, ...
for .com,3.2, 4.0, ...
for foam-extend) (see also #29). Versioning systems and other assumptions have changed much lately.We currently use the WMake build system, developped together with OpenFOAM. This is a quite primitive system, but it is intrinsic to OpenFOAM. On top of it, I have added the Allwmake bash script to automate the procedure even more (also a common way in OpenFOAM-derived projects).
A few ideas on how to go on:
(ugly) Support different versions in different branches. The current state of the branch works with the latest version, Git tags/other branches for older versions.
(ugly) Use pre-processor commands to check the version and activate different parts of the code.
(ugly) Have source files like
Adapter_of6_dev.C
andAdapter_of4_5.C
and let our script pick the correct ones.Use a proper configuration system (e.g. CMake)
I would very much value your feedback on this.