precice / openfoam-adapter

OpenFOAM-preCICE adapter
https://precice.org/adapter-openfoam-overview.html
GNU General Public License v3.0
135 stars 80 forks source link

Re-arrange `if` logic for Pstream initialization #293

Closed davidscn closed 1 year ago

davidscn commented 1 year ago

I compiled a recent develop snapshot of OpenFOAM 2306 on a cluster machine in order to test it against #291 to save potential future efforts. Compiling works flawless.

However, the teardown of a coupling participant fails when executed in serial (parallel execution via MPI worked fine) with the following stacktrace:

[stack trace]
=============
#1  Foam::sigSegv::sigHandler(int) in ~/tmp/openfoam/platforms/linux64GccDPInt32Opt/lib/libOpenFOAM.so
#2  ? in /lib/x86_64-linux-gnu/libpthread.so.0
#3  Foam::UPstream::freeCommunicatorComponents(int) in ~/tmp/openfoam/platforms/linux64GccDPInt32Opt/lib/sys-openmpi/libPstream.so
#4  Foam::UPstream::shutdown(int) in ~/tmp/openfoam/platforms/linux64GccDPInt32Opt/lib/sys-openmpi/libPstream.so
#5  Foam::argList::~argList() in ~/tmp/openfoam/platforms/linux64GccDPInt32Opt/lib/libOpenFOAM.so
#6  ? in ~/tmp/openfoam/platforms/linux64GccDPInt32Opt/bin/laplacianFoam
#7  __libc_start_main in /lib/x86_64-linux-gnu/libc.so.6
#8  ? in ~/tmp/openfoam/platforms/linux64GccDPInt32Opt/bin/laplacianFoam
=============
Segmentation fault

Seems to be related to https://github.com/precice/precice/issues/128#issuecomment-402549649 and skipping the Pstream initialization does the job, i.e., the workaround is apparently not required any more. Would be great to get a confirmation from @olesenm on this.

olesenm commented 1 year ago

I have opened an OpenFOAM issue.

I think we need to fix it in OpenFOAM, not this patch. I will address the develop branch first and pick across to maintenance branches later.

olesenm commented 1 year ago

Could update like this (makes a bit more sense in terms of the OpenFOAM version numbering):

#if (defined OPENFOAM && (OPENFOAM >= 1712)) || (defined OPENFOAM_PLUS && (OPENFOAM_PLUS >= 1712))
davidscn commented 1 year ago

I can confirm that the issue was resolved with https://develop.openfoam.com/Development/openfoam/-/commit/3de090e602acfd470c3f8ec6f66799096352c5cf

I applied your suggestion, guessing that it was supposed to be (note the version numbers)

#if (defined OPENFOAM && (OPENFOAM >= 1806)) || (defined OPENFOAM_PLUS && (OPENFOAM_PLUS >= 1712))

Thanks a lot!

olesenm commented 1 year ago

I can confirm that the issue was resolved with https://develop.openfoam.com/Development/openfoam/-/commit/3de090e602acfd470c3f8ec6f66799096352c5cf

I applied your suggestion, guessing that it was supposed to be (note the version numbers)

#if (defined OPENFOAM && (OPENFOAM >= 1806)) || (defined OPENFOAM_PLUS && (OPENFOAM_PLUS >= 1712))

Thanks a lot!

Actually the repeated 1712 was intended. We had OPENFOAM_PLUS initially, which was later corrected to be a plain OPENFOAM. The define for checking the version number with (OPENFOAM >= 1712) will probably also work with the maintenance branches (does no harm either way) but makes for clearer documentation.

Alternative, drop mention of older versions - it could be that the adaptor doesn't even work for them anyhow.

olesenm commented 1 year ago

I can confirm that the issue was resolved with https://develop.openfoam.com/Development/openfoam/-/commit/3de090e602acfd470c3f8ec6f66799096352c5cf

This was actually very useful timing for your bug report. The fix will be in the 2306 release, and has been applied to various maintenance branches (1912 onwards).

davidscn commented 1 year ago

Actually the repeated 1712 was intended. We had OPENFOAM_PLUS initially, which was later corrected to be a plain OPENFOAM. The define for checking the version number with (OPENFOAM >= 1712) will probably also work with the maintenance branches (does no harm either way) but makes for clearer documentation.

Alternative, drop mention of older versions - it could be that the adaptor doesn't even work for them anyhow.

I rectified it according to your suggestion, thanks again.

This was actually very useful timing for your bug report. The fix will be in the 2306 release, and has been applied to various maintenance branches (1912 onwards).

This fix was very useful for us either :)