-
Notifications
You must be signed in to change notification settings - Fork 683
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
{math}[gompi/2022b] SCOTCH v7.0.3 #17459
{math}[gompi/2022b] SCOTCH v7.0.3 #17459
Conversation
SCOTCH library supports CMake since 7.0.3. Under my opinion it will be better to use generic CMakeMake easyblock. |
@jfgrimm compilation without -DBUILD_SHARED_LIBS=ON produces static libraries on my system. May be that SCOTCH somewhat bypasses standard CMakeMake setup. -DINSTALL_METIS_HEADERS=OFF is essential to avoid compilation problems with other libraries. The same trick was in scotch easyblock (see lines 135-137 in the easyblock scotch.py). Therefore I prefer to follow this behavior and to use INSTALL_METIS_HEADERS=OFF. |
It seems that there is a problem with OpenFOAM build which requires non-threaded SCOTCH. Therefore I propose to switch back to version based on scotch.py easyblock. |
@mboisson I did some tests with OpenFOAM v2212 witch threaded scotch and I didn't found any problem yet. Nevertheless, I added threadedmpi = False as in your eb to keep compatibility with OF10. |
For reference, the threaded-scotch issue with OpenFOAM is discussed in this issue: #15907 |
I'm aware of #15907. I tried the motorBike tutorial without any problems both with threaded and non-threaded SCOTCH with OF 2212. |
Ok, then maybe the issue with threaded SCOTCH is simply resolved in version 7.x. Or maybe it simply does not happen with OpenFOAM v2xyz (only with OpenFOAM 10, 11, etc.) |
I will try to install OF10 and check it.... |
@mboisson Indeed, the installation of OF10 failed during sanity checking for SCOTCH 7.0.3 without threadedmpi=True With threadedmpi=False the installation was successful. Therefore, the thradedmpi=False is still necessary. Thanks for hint. |
I can't approve PRs in this repo (fortunately ? unfortunately ? I don't know) |
source_urls = ['https://gitlab.inria.fr/scotch/scotch/-/archive/v%(version)s/'] | ||
sources = ['%(namelower)s-v%(version)s.tar.gz'] | ||
checksums = ['5b5351f0ffd6fcae9ae7eafeccaa5a25602845b9ffd1afb104db932dd4d4f3c5'] | ||
threadedmpi = False |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, i tried following the dicussion, but i'm still hazy on this. You say we still need to disable this, but we have never had a SCOTCH easyconfig that set this before. As far as i can tell, this was always the default (actually, default in gompi, default on in iimpi), so should we just leave it out in this easyconfig as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed, we still need to disable this. It was always broken in many of the builds of OpenFOAM done by EB as far as I know.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OpenFOAM.org requires a non-threaded MPI version of SCOTCH. Whether the flag makes it into the default recipe or in a separate recipe (which is what we did in our stack) does not really matter.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Having threaded MPI version of SCOTCH is the source of the following long-standing issues: #7093
#15907
and is discussed upstream:
https://develop.openfoam.com/Development/openfoam/-/issues/2791
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed, we still need to disable this.
I thought we were already disabling it by default, but i misread the code;
https://github.com/easybuilders/easybuild-easyblocks/blob/develop/easybuild/easyblocks/s/scotch.py#L109
I missed the not
in not in
But the comment in the easyblock suggests this is some old crap that should be removed a looong time ago.
# For backwards compatability of v2.8.0 this is necessary but could be removed on a major version upgrade
mpi_family = self.toolchain.mpi_family()
if self.cfg['threadedmpi'] is None and mpi_family not in [toolchain.INTELMPI, toolchain.QLOGICMPI]:
cflags += " -DSCOTCH_PTHREAD"
version 2.8.0 of what i wonder.. it couldn't be SCOTCH since that would be too ancient. I traced it back to this PR:
easybuilders/easybuild-easyblocks#914
so it was referring to EasyBuild 2.8.0
So, i guess this part of the easyblock should have been removed a long time ago.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
@boegelbot please test @ generoso |
Test report by @Micket |
@Micket: Request for testing this PR well received on login1 PR test command '
Test results coming soon (I hope)... - notification for comment with ID 1642405326 processed Message to humans: this is just bookkeeping information for me, |
Test report by @boegelbot |
@boegelbot please test @ jsc-zen2 |
Going in, thanks @furstj! |
@Micket: Request for testing this PR well received on jsczen2l1.int.jsc-zen2.easybuild-test.cluster PR test command '
Test results coming soon (I hope)... - notification for comment with ID 1642417376 processed Message to humans: this is just bookkeeping information for me, |
Test report by @boegelbot |
(created using
eb --new-pr
)