View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0002330 | OpenFOAM | Patch | public | 2016-11-11 10:25 | 2016-12-03 19:51 |
| Reporter | alexeym | Assigned To | henry | ||
| Priority | normal | Severity | feature | Reproducibility | always |
| Status | resolved | Resolution | fixed | ||
| Platform | GNU/Linux | OS | Fedora | OS Version | 24 |
| Product Version | dev | ||||
| Fixed in Version | dev | ||||
| Summary | 0002330: Be more gentle to system-wide installed Scotch and METIS | ||||
| Description | 1. Currently build logic for (PT)*Scotch and METIS decomposition methods is adapted for either Ubuntu or third-party installation locations. Basically additions are: - PLIBS in ptscotchDecomp LIB_LIBS, as on RHEL-based systems ptscotch is installed into MPI libraries folder. - check for .../lib*/libmetis.so to account for lib64 library folder. - options files check existence of *_ARCH_PATH locations before adding -I and -L flags. 2. scotchDecomp and metisDecomp use OpenFOAM types (label and scalar), so they have to be in accordance with idx_t/SCOTCH_Num and real_t defined by Scotch/METIS. Currently this is solved by custom compilation of the libraries. The patch adds OpenFOAM <-> Scotch/METIS conversion pieces in the beginning/end of decomposition methods; changes scalar to real_t in METIS; issues warning in case of different int lengths. | ||||
| Tags | No tags attached. | ||||
|
|
0004-System-wide-Scotch-and-METIS.patch (16,031 bytes)
From 15d32baeee49224a4cf8979f06cf35d728b401f7 Mon Sep 17 00:00:00 2001
From: Alexey Matveichev <github@matveichev.com>
Date: Fri, 11 Nov 2016 08:53:28 +0100
Subject: [PATCH 4/5] System-wide Scotch and METIS
---
src/parallel/decompose/metisDecomp/Allwmake | 11 ++-
src/parallel/decompose/metisDecomp/metisDecomp.C | 85 ++++++++++++++--------
src/parallel/decompose/ptscotchDecomp/Make/options | 21 ++++--
.../decompose/ptscotchDecomp/ptscotchDecomp.C | 49 ++++++++++---
src/parallel/decompose/scotchDecomp/Make/options | 17 ++++-
src/parallel/decompose/scotchDecomp/scotchDecomp.C | 42 ++++++++---
6 files changed, 163 insertions(+), 62 deletions(-)
diff --git a/src/parallel/decompose/metisDecomp/Allwmake b/src/parallel/decompose/metisDecomp/Allwmake
index 2f666ab..9408c3e 100755
--- a/src/parallel/decompose/metisDecomp/Allwmake
+++ b/src/parallel/decompose/metisDecomp/Allwmake
@@ -8,8 +8,15 @@ cd ${0%/*} || exit 1 # Run from this directory
if settings=`$WM_PROJECT_DIR/bin/foamEtcFile config.sh/metis`
then
. $settings
- echo "using METIS_ARCH_PATH=$METIS_ARCH_PATH"
- if [ -r $METIS_ARCH_PATH/lib/libmetis.so ]
+ # Fall-back to system-wide installed METIS
+ if [ -d "$METIS_ARCH_PATH" ]
+ then
+ echo "using METIS_ARCH_PATH=$METIS_ARCH_PATH"
+ else
+ echo "using system-wide installed METIS"
+ METIS_ARCH_PATH=/usr
+ fi
+ if [ -r $METIS_ARCH_PATH/lib*/libmetis.so ]
then
wmake $targetType
fi
diff --git a/src/parallel/decompose/metisDecomp/metisDecomp.C b/src/parallel/decompose/metisDecomp/metisDecomp.C
index 3ede45d..98d4629 100644
--- a/src/parallel/decompose/metisDecomp/metisDecomp.C
+++ b/src/parallel/decompose/metisDecomp/metisDecomp.C
@@ -54,26 +54,43 @@ Foam::label Foam::metisDecomp::decompose
List<label>& finalDecomp
)
{
+ if (sizeof(idx_t) < sizeof(label))
+ {
+ WarningInFunction
+ << "Size of idx_t is less than size of label, possible data loss."
+ << endl;
+ }
+ // METIS-typed lists
+ List<idx_t> tadjncy(adjncy.size(), Zero);
+ List<idx_t> txadj(xadj.size(), Zero);
+ forAll(adjncy, i)
+ {
+ tadjncy[i] = static_cast<idx_t>(adjncy[i]);
+ }
+ forAll(xadj, i)
+ {
+ txadj[i] = static_cast<idx_t>(xadj[i]);
+ }
// Method of decomposition
// recursive: multi-level recursive bisection (default)
// k-way: multi-level k-way
word method("recursive");
- label numCells = xadj.size()-1;
+ idx_t numCells = static_cast<idx_t>(xadj.size() - 1);
// Decomposition options
- List<label> options(METIS_NOPTIONS);
+ List<idx_t> options(METIS_NOPTIONS);
METIS_SetDefaultOptions(options.begin());
// Processor weights initialised with no size, only used if specified in
- // a file
- Field<scalar> processorWeights;
+ // a file. real_t is used to please system-wide installed METIS.
+ Field<real_t> processorWeights;
// Cell weights (so on the vertices of the dual)
- List<label> cellWeights;
+ List<idx_t> cellWeights;
// Face weights (so on the edges of the dual)
- List<label> faceWeights;
+ List<idx_t> faceWeights;
// Check for externally provided cellweights and if so initialise weights
@@ -157,55 +174,61 @@ Foam::label Foam::metisDecomp::decompose
}
}
- label ncon = 1;
- label nProcs = nProcessors_;
+ idx_t ncon = 1;
+ idx_t nProcs = static_cast<idx_t>(nProcessors_);
// Output: cell -> processor addressing
- finalDecomp.setSize(numCells);
+ finalDecomp.setSize(static_cast<label>(numCells));
+ List<idx_t> tfinalDecomp(static_cast<label>(numCells), 0);
// Output: number of cut edges
- label edgeCut = 0;
+ idx_t edgeCut = 0;
if (method == "recursive")
{
METIS_PartGraphRecursive
(
- &numCells, // num vertices in graph
- &ncon, // num balancing constraints
- const_cast<List<label>&>(xadj).begin(), // indexing into adjncy
- const_cast<List<label>&>(adjncy).begin(), // neighbour info
- cellWeights.begin(),// vertexweights
- nullptr, // vsize: total communication vol
- faceWeights.begin(),// edgeweights
- &nProcs, // nParts
+ &numCells, // num vertices in graph
+ &ncon, // num balancing constraints
+ txadj.begin(), // indexing into adjncy
+ tadjncy.begin(), // neighbour info
+ cellWeights.begin(), // vertexweights
+ nullptr, // vsize: total communication vol
+ faceWeights.begin(), // edgeweights
+ &nProcs, // nParts
processorWeights.begin(), // tpwgts
- nullptr, // ubvec: processor imbalance (default)
+ nullptr, // ubvec: processor imbalance (default)
options.begin(),
&edgeCut,
- finalDecomp.begin()
+ tfinalDecomp.begin()
);
}
else
{
METIS_PartGraphKway
(
- &numCells, // num vertices in graph
- &ncon, // num balancing constraints
- const_cast<List<label>&>(xadj).begin(), // indexing into adjncy
- const_cast<List<label>&>(adjncy).begin(), // neighbour info
- cellWeights.begin(),// vertexweights
- nullptr, // vsize: total communication vol
- faceWeights.begin(),// edgeweights
- &nProcs, // nParts
+ &numCells, // num vertices in graph
+ &ncon, // num balancing constraints
+ txadj.begin(), // indexing into adjncy
+ tadjncy.begin(), // neighbour info
+ cellWeights.begin(), // vertexweights
+ nullptr, // vsize: total communication vol
+ faceWeights.begin(), // edgeweights
+ &nProcs, // nParts
processorWeights.begin(), // tpwgts
- nullptr, // ubvec: processor imbalance (default)
+ nullptr, // ubvec: processor imbalance (default)
options.begin(),
&edgeCut,
- finalDecomp.begin()
+ tfinalDecomp.begin()
);
}
- return edgeCut;
+ forAll(tfinalDecomp, i)
+ {
+ finalDecomp[i] = static_cast<label>(tfinalDecomp[i]);
+ }
+
+ return static_cast<label>(edgeCut);
}
diff --git a/src/parallel/decompose/ptscotchDecomp/Make/options b/src/parallel/decompose/ptscotchDecomp/Make/options
index cb407ec..dc93165 100644
--- a/src/parallel/decompose/ptscotchDecomp/Make/options
+++ b/src/parallel/decompose/ptscotchDecomp/Make/options
@@ -2,16 +2,27 @@ sinclude $(GENERAL_RULES)/mplib$(WM_MPLIB)
sinclude $(RULES)/mplib$(WM_MPLIB)
EXE_INC = \
+ -Wno-old-style-cast \
$(PFLAGS) $(PINC) \
- -I$(SCOTCH_ARCH_PATH)/include/$(FOAM_MPI) \
- -I$(SCOTCH_ARCH_PATH)/include \
- -I/usr/include/scotch \
-I../decompositionMethods/lnInclude
LIB_LIBS = \
- -L$(SCOTCH_ARCH_PATH)/lib \
- -L$(FOAM_EXT_LIBBIN)/$(FOAM_MPI) \
+ $(PLIBS) \
-lptscotch \
-lptscotcherrexit \
-lscotch \
-lrt
+
+ifneq ("$(SCOTCH_ARCH_PATH)","")
+EXE_INC += -I$(SCOTCH_ARCH_PATH)/include/$(FOAM_MPI)
+EXE_INC += -I$(SCOTCH_ARCH_PATH)/include
+LIB_LIBS += -L$(SCOTCH_ARCH_PATH)/lib
+endif
+
+ifneq ("$(wildcard /usr/include/scotch)","")
+EXE_INC += -I/usr/include/scotch
+endif
+
+ifneq ("$(wildcard $(FOAM_EXT_LIBBIN)/$(FOAM_MPI))","")
+LIB_LIBS += -L$(FOAM_EXT_LIBBIN)/$(FOAM_MPI)
+endif
diff --git a/src/parallel/decompose/ptscotchDecomp/ptscotchDecomp.C b/src/parallel/decompose/ptscotchDecomp/ptscotchDecomp.C
index dcd2596..ae79f1e 100644
--- a/src/parallel/decompose/ptscotchDecomp/ptscotchDecomp.C
+++ b/src/parallel/decompose/ptscotchDecomp/ptscotchDecomp.C
@@ -23,6 +23,8 @@ License
\*---------------------------------------------------------------------------*/
+#include <type_traits>
+
#include "ptscotchDecomp.H"
#include "addToRunTimeSelectionTable.H"
#include "Time.H"
@@ -303,6 +305,26 @@ Foam::label Foam::ptscotchDecomp::decompose
Pout<< "ptscotchDecomp : entering with xadj:" << xadjSize << endl;
}
+ if (sizeof(label) > sizeof(SCOTCH_Num))
+ {
+ WarningInFunction
+ << "Size of SCOTCH_Num is less than size of label, "
+ "possible data loss." << endl;
+ }
+
+ List<SCOTCH_Num> tadjncy(adjncySize, 0);
+ List<SCOTCH_Num> txadj(xadjSize, 0);
+
+ for (label i = 0; i < adjncySize; i++)
+ {
+ tadjncy[i] = static_cast<SCOTCH_Num>(adjncy[i]);
+ }
+
+ for (label i = 0; i < xadjSize; i++)
+ {
+ txadj[i] = static_cast<SCOTCH_Num>(xadj[i]);
+ }
+
// Dump graph
if (decompositionDict_.found("scotchCoeffs"))
{
@@ -388,7 +410,7 @@ Foam::label Foam::ptscotchDecomp::decompose
// Graph
// ~~~~~
- List<label> velotab;
+ List<SCOTCH_Num> velotab;
// Check for externally provided cellweights and if so initialise weights
@@ -486,19 +508,19 @@ Foam::label Foam::ptscotchDecomp::decompose
0, // baseval, c-style numbering
xadjSize-1, // vertlocnbr, nCells
xadjSize-1, // vertlocmax
- const_cast<SCOTCH_Num*>(xadj),
+ txadj.data(),
// vertloctab, start index per cell into
// adjncy
- const_cast<SCOTCH_Num*>(xadj+1),// vendloctab, end index ,,
+ txadj.data()+1, // vendloctab, end index ,,
- const_cast<SCOTCH_Num*>(velotab.begin()),// veloloctab, vtx weights
- nullptr, // vlblloctab
+ velotab.begin(), // veloloctab, vtx weights
+ nullptr, // vlblloctab
adjncySize, // edgelocnbr, number of arcs
adjncySize, // edgelocsiz
- const_cast<SCOTCH_Num*>(adjncy), // edgeloctab
- nullptr, // edgegsttab
- nullptr // edlotab, edge weights
+ tadjncy.data(), // edgeloctab
+ nullptr, // edgegsttab
+ nullptr // edlotab, edge weights
),
"SCOTCH_dgraphBuild"
);
@@ -522,7 +544,7 @@ Foam::label Foam::ptscotchDecomp::decompose
SCOTCH_Arch archdat;
check(SCOTCH_archInit(&archdat), "SCOTCH_archInit");
- List<label> processorWeights;
+ List<SCOTCH_Num> processorWeights;
if (decompositionDict_.found("scotchCoeffs"))
{
const dictionary& scotchCoeffs =
@@ -577,17 +599,22 @@ Foam::label Foam::ptscotchDecomp::decompose
{
Pout<< "SCOTCH_dgraphMap" << endl;
}
+ List<SCOTCH_Num> tfinalDecomp(max(1, xadjSize-1), 0);
check
(
SCOTCH_dgraphMap
(
&grafdat,
&archdat,
- &stradat, // const SCOTCH_Strat *
- finalDecomp.begin() // parttab
+ &stradat, // const SCOTCH_Strat *
+ tfinalDecomp.begin() // parttab
),
"SCOTCH_graphMap"
);
+ forAll(tfinalDecomp, i)
+ {
+ finalDecomp[i] = static_cast<label>(tfinalDecomp[i]);
+ }
#ifdef FE_NOMASK_ENV
feenableexcept(oldExcepts);
diff --git a/src/parallel/decompose/scotchDecomp/Make/options b/src/parallel/decompose/scotchDecomp/Make/options
index d2cc770..5dd1627 100644
--- a/src/parallel/decompose/scotchDecomp/Make/options
+++ b/src/parallel/decompose/scotchDecomp/Make/options
@@ -7,13 +7,22 @@ sinclude $(RULES)/mplib$(WM_MPLIB)
EXE_INC = \
$(PFLAGS) $(PINC) \
- -I$(SCOTCH_ARCH_PATH)/include \
- -I/usr/include/scotch \
-I../decompositionMethods/lnInclude
LIB_LIBS = \
- -L$(SCOTCH_ARCH_PATH)/lib \
- -L$(FOAM_EXT_LIBBIN) \
-lscotch \
-lscotcherrexit \
-lrt
+
+ifneq ("$(SCOTCH_ARCH_PATH)","")
+EXE_INC += -I$(SCOTCH_ARCH_PATH)/include
+LIB_LIBS += -L$(SCOTCH_ARCH_PATH)/lib
+endif
+
+ifneq ("$(wildcard /usr/include/scotch)","")
+EXE_INC += -I/usr/include/scotch
+endif
+
+ifneq ("$(wildcard $(FOAM_EXT_LIBBIN))","")
+LIB_LIBS += -L$(FOAM_EXT_LIBBIN)
+endif
diff --git a/src/parallel/decompose/scotchDecomp/scotchDecomp.C b/src/parallel/decompose/scotchDecomp/scotchDecomp.C
index 987ef18..5f84373 100644
--- a/src/parallel/decompose/scotchDecomp/scotchDecomp.C
+++ b/src/parallel/decompose/scotchDecomp/scotchDecomp.C
@@ -206,6 +206,26 @@ Foam::label Foam::scotchDecomp::decomposeOneProc
List<label>& finalDecomp
)
{
+ if (sizeof(label) > sizeof(SCOTCH_Num))
+ {
+ WarningInFunction
+ << "Size of SCOTCH_Num is less than size of label, "
+ "possible data loss." << endl;
+ }
+ // Temporary lists to resolve SCOTCH_Num and label differences
+ List<SCOTCH_Num> sadjncy(adjncy.size());
+ List<SCOTCH_Num> sxadj(xadj.size());
+
+ forAll(adjncy, i)
+ {
+ sadjncy[i] = static_cast<SCOTCH_Num>(adjncy[i]);
+ }
+
+ forAll(xadj, i)
+ {
+ sxadj[i] = static_cast<SCOTCH_Num>(xadj[i]);
+ }
+
// Dump graph
if (decompositionDict_.found("scotchCoeffs"))
{
@@ -276,7 +296,7 @@ Foam::label Foam::scotchDecomp::decomposeOneProc
// Graph
// ~~~~~
- List<label> velotab;
+ List<SCOTCH_Num> velotab;
// Check for externally provided cellweights and if so initialise weights
@@ -334,13 +354,13 @@ Foam::label Foam::scotchDecomp::decomposeOneProc
(
&grafdat,
0, // baseval, c-style numbering
- xadj.size()-1, // vertnbr, nCells
- xadj.begin(), // verttab, start index per cell into adjncy
- &xadj[1], // vendtab, end index ,,
+ sxadj.size()-1, // vertnbr, nCells
+ sxadj.begin(), // verttab, start index per cell into adjncy
+ &sxadj[1], // vendtab, end index ,,
velotab.begin(), // velotab, vertex weights
nullptr, // vlbltab
- adjncy.size(), // edgenbr, number of arcs
- adjncy.begin(), // edgetab
+ sadjncy.size(), // edgenbr, number of arcs
+ sadjncy.begin(), // edgetab
nullptr // edlotab, edge weights
),
"SCOTCH_graphBuild"
@@ -355,7 +375,7 @@ Foam::label Foam::scotchDecomp::decomposeOneProc
SCOTCH_Arch archdat;
check(SCOTCH_archInit(&archdat), "SCOTCH_archInit");
- List<label> processorWeights;
+ List<SCOTCH_Num> processorWeights;
if (decompositionDict_.found("scotchCoeffs"))
{
const dictionary& scotchCoeffs =
@@ -438,6 +458,7 @@ Foam::label Foam::scotchDecomp::decomposeOneProc
finalDecomp.setSize(xadj.size()-1);
finalDecomp = 0;
+ List<SCOTCH_Num> tfinalDecomp(xadj.size()-1, 0);
check
(
SCOTCH_graphMap
@@ -445,17 +466,20 @@ Foam::label Foam::scotchDecomp::decomposeOneProc
&grafdat,
&archdat,
&stradat, // const SCOTCH_Strat *
- finalDecomp.begin() // parttab
+ tfinalDecomp.begin() // parttab
),
"SCOTCH_graphMap"
);
+ forAll(tfinalDecomp, i)
+ {
+ finalDecomp[i] = static_cast<label>(tfinalDecomp[i]);
+ }
#ifdef FE_NOMASK_ENV
feenableexcept(oldExcepts);
#endif
-
//finalDecomp.setSize(xadj.size()-1);
//check
//(
--
2.7.4
|
|
|
I am OK with (1) if it doesn't cause any problems on other systems. Bruno, is it OK for you? I think the type casting in (2) is a bad idea and it is better to make sure the builds are consistent. If real_t and scalar are actually the same type there should be no need for casting and if they are of different types casting should not be possible. |
|
|
I use only label <-> idx_t/SCOTCH_Num casts. There are 4 possibilities with sizes (label/idx_t): 1. 32/32 2. 32/64 3. 64/32 4. 64/64 1,2, and 4 are OK. In case 3 user receives warning. real_t is used in METIS for processor weights; so instead of list of scalars list of real_t is created, then it is read from metisCoeffs dictionary. OpenFOAM can easily read floats even if it is build with WM_DP, while METIS can not deal with doubles as processor weights if it is built with real_t as float. |
|
|
Using float type for the weights makes sense but to avoid storage and conversion overhead I think it is better if the same index type is used in OpenFOAM and Scotch. |
|
|
Storage and conversion overhead could be avoided using conditional compilation (METIS defines IDXTYPEWIDTH, which has the same meaning as WM_LABEL_SIZE; with Scotch everything is a little bit fancier, yet this could be done indirectly using SCOTCH_NUMMAX). But I guess, you will strongly object against this solution. |
|
|
If Scotch and OpenFOAM are built with different index types surely some storage and conversion overhead is required irrespective of what #defines and conditional compilation is involved. How would SCOTCH_NUMMAX avoid this overhead? |
|
|
Current version of the patch do conversion without any checks: before METIS/Scotch call I convert all label lists to respective idx_t/SCOTCH_Num lists. Using conditional compilation copying could be performed only if sizeof(label) != sizeof(idx_t). In case of METIS we can use IDXTYPEWIDTH and compare it with WM_LABEL_SIZE. In case of Scotch comparison is not so obvious, as library does not have direct analogy of WM_LABEL_SIZE. But Scotch defines SCOTCH_NUMMAX, which is written to scotch.h during compilation and for 32-bit indices its value (((int) (((unsigned int) 1 << ((sizeof (int) << 3) - 1)) - 1))) is lower than for 64-bit ones (((int64_t) (((uint64_t) 1 << ((sizeof (int64_t) << 3) - 1)) - 1))). This can be used in conditional compilation. |
|
|
Small addition. First part of the patch without second is rather useless. It will cause more confusion as it is rare when system-wide installed METIS and Scotch index type sizes are equal to label's. |
|
|
> This can be used in conditional compilation. Does this mean that Scotch recompiles itself if the index size is not the same as the required index size or that Scotch must be recompiled by the user? |
|
|
I have committed the change to processor weights: OpenFOAM-dev: commit 1fc85296725805cc0f47604cfe78f9b48d06b719 and removed the SP/DP setting from the compilation of metis: ThirdParty-dev: commit 706dde435278b8f477b6ff210a834c02d298a241 Still running tests on various platforms but it looks OK so far. Please report any problems. |
|
|
Regarding the changes to "Allwmake" and "Make/options", at first I thought there would be a couple of breaking points, but it seems that the majority of the situations are covered. Nonetheless: - It doesn't cover the situation where "/usr/include/MPI_NAME/scotch" is used, given that "MPI_NAME" may depend on the Linux Distribution and installed MPI toolbox... - It has come to my attention that Metis in some situations will create the library folder at "$METIS_ARCH_PATH/lib64" instead of "$METIS_ARCH_PATH/lib". Problem is that I haven't found out if it's Metis version dependent, or Linux Distribution dependent. Alexey's changes to "Allwmake" already account for this, but the "Make/options" file doesn't. With this in mind and given the complexity we're using to account for system/custom builds, I would like to suggest that we shift to consistent 1-to-1 convention, instead of always chaining together the two possibilities of having system and custom builds, which is susceptible to accidentally having cross-wires (system headers with custom library or vice-versa). The idea would be to have the environment variables always point to the correct paths "*_ARCH_PATH" from which we want Scotch and Metis to be used from and therefore have the system paths defined in the shell environment "*_ARCH_PATH" variables before "wmake" is called. Furthermore, assuming that "$FOAM_EXT_LIBBIN" has the libraries we are looking for is also a bit dangerous (e.g. the aforementioned cross-wires), I would really prefer that we explicitly relied on an environment that states the path we want to use for Metis or Scotch or PTScotch. Regarding to the 32-bit vs 64-bit integers, just a quick side note: I only noticed last week that I wasn't able to use the system's Metis in openSUSE 42.1 when building with 32-bit labels in OpenFOAM, because the system's Metis build is hard-coded to 64-bit integers and 32-bit floats. This to say that the latest commit at least covers the float/double issue. And although I don't really like the overhead of converting between integer formats (mostly due to the memory overhead), allowing for users and package managers to forcefully use the system's Metis/Scotch perhaps could be acceptable as a build option? |
|
|
I agree that the handling of ThirdParty and system scotch and metis installations needs to be rationalized, generalized and simplified and the current proposed patch introduces complexity without resolving all the issues. I don't have time to work on this but would be happy to receive patches which resolve the issues in an elegant manner. Given how easy and automated it is to build metis and scotch in the ThirdParty directory I think that the clutter, maintenance and run-time overhead of using an incompatible system installation of these packages is not warranted. However support for both double and float involved no overhead in either complexity or run-time. |
|
|
[RANT] ThirdParty tarball creates overhead of third party tarball (and certain third party software source tarballs). It could be automated and easy to install, yet it will never be easy and maintainable as my preferred distribution's package manager with a team of package maintainers. Surely from OpenFOAM's maintainer point of view it is much better if all users use the same distribution and compile third party software using ThirdParty tarball. And it seems everyone is quite happy with the way OpenFOAM manages third party software and would be glad to compile (gcc, OpenMPI, Scotch, METIS, boost, CGAL, QT, ParaView) it for every installed version. Partly this dream was partly achieved by Docker deployment. Still I do not quite get why Debian's /usr/include/scotch are in OpenFOAM's default -I flags and there is no RHEL's /usr/lib64 in -L flags. Somehow /usr/include easily got into CGAL's rules and broke build on certain systems. Why I have to edit config.sh/{scotch,metis,CGAL} every compilation instead of software try to look for my system-wide installed headers and libraries. [/RANT] Currently, it seems maintaining own patch set for system-wide installed Scotch/METIS is much simpler solution, so bug report can be closed. |
|
|
Building against system-wide installed Scotch/METIS is fine provided the builds of those are consistent with the build of OpenFOAM being attempted. If the system Scotch/METIS are built with 64bit indices then OpenFOAM would also have to be built with 64bit indices for consistency. If OpenFOAM is to be built with 32bit indices, which is more efficient, then the Scotch/METIS should also be available compiled with 32bit indices either system-wide or local; of course it would be better if both the 32bit and 64bit builds of Scotch/METIS are availible system-wide. The problem is that the sytem installations of Scotch/METIS are in different places on different systems and on many systems not available at all or out of date (which is the case on the systems I work with so I build in ThirdParty). It will take a bit of work to support all of the build options in a robust, flexible and convenient manner. I agree with Bruno that to provide generality with simplicity and robustness we should probably provide environment variables which can be explicitly set if the libraries are not in "standard" locations. |
|
|
@Alexey: From my perspective, many thanks for the rant! It does remind me that you've always been adamant in wanting to use the system's packages (seen in forum posts), instead of having to build custom ones. They are there in the package manager for a very good reason, although they may be outdated in some situations. I'm not sure if it's always like this, but "/usr/include" and "/usr/lib*" are usually set by default by the compiler stack, so these rarely need to be explicitly defined. As for some paths being hard-coded to Debian/Ubuntu scheme, it may be because it's the more popular Linux Distribution type and because the Deb packages are the ones more commonly published by the OpenFOAM Foundation... so there may be a bias coming from there. That and it was probably a matter of getting things up and running sooner rather than later. @Henry: I've been meaning to make the settings for third-party software uniform for quite sometime already and this report + other recent reports has helped me gain a better perspective of what's needed. I'll try to have something for this as soon as possible (a week or so), given that it's more relaxing than much of the coding work I usually need to do (and more relaxing than the other bugs that need fixing). But I do have a question: If the hack for supporting mixed integer precision for Scotch and Metis is minimally invasive (ifdef+include+endif), would that be acceptable? This way the 3/6 line-hack could be provisionally commented out if it gets in the way when debugging/maintaining. Furthermore, when the settings are consolidated into their respective files in "etc/config.*", it's easier to create an auxiliary script that sets these up based on the existing system settings, making it possible to automate the switch for system installations... although this does also remind me that foamInstallationTest and foamSystemCheck probably need additional cleanup/consolidation work too... |
|
|
I think supporting mixed integer precision for Scotch and Metis is fundamentally a bad idea; it brings with it unnecessary overhead in storage, CPU time and code complexity. If all the system libraries use 64bit indexing and it is imperative that OpenFOAM use those libraries then for consistency OpenFOAM should also be compiled with 64bit indexing. If the operational overhead of 64bit indexing is too great then both OpenFOAM and the other libraries should be compiled with 32bit indexing. I don't see a fundamental problem with system-wide installations of libraries compiled with either or both index types; they just need to be installed in different directories. > This way the 3/6 line-hack I am not sure which hack you are referring to; the code to map between 32bit and 64bit indexing is way more than 3/6 lines and involves storage and conversion overhead. Maybe we are talking at cross-purposes. |
|
|
> This way the 3/6 line-hack Sorry, I should have been clearer of what I had in mind: The idea is to place the lengthy hack inside a separate header file (or two) to be included on a need basis, i.e. similar to what's done with the solvers, but using #ifdef instead of a conventional if. This would minimize the clutter in the main class code, while supporting the requested feature. Either way, I see what you mean, namely that the various Linux Distributions should update their Deb/RPM packages to support both 32 and 64-bit integers... although... some of them already do that, but the problem is that its hard-coded to 32-bit architectures... Anyway, that will take some considerable time to propagate to all Linux Distributions (assuming they accept the request), but I guess this is to be treated in the same line as many other issues where the problem is with the third-party software, not with OpenFOAM. |
|
|
Sorry, I don't have all of the work done yet for the consistent environment, but I already have some preliminary/conceptual work done. I haven't started another task for this either, since it's still mostly connected to this report. Since what I have is still WIP, feel free to browse the branch "bug2330_consistent_env" on the fork at wyldckat at Github, specifically this commit: https://github.com/wyldckat/OpenFOAM-dev/commit/c72823e32b6c7d18e42c958dc535850d11a01e57 Already done: 1. SCOTCH 2. METIS Notes: Allwmake scripts were updated to only use these two if the "$*_ARCH_PATH" path exists. Therefore the only failure to build is delegated to wmake/make. It's a bit more bureaucratic in the settings, by using several environment variables instead of just 2, but this is to avoid any ambiguity in the defined paths (e.g. 'lib' vs 'lib32' vs 'lib64'). Still to do: 1. CGAL/Boost 2. Example scripts for each distribution, possibly placed inside a new sub-folder "etc/config.sh/presets" for making it easier to distinguish from the examples. 3. foamSetup script, where the first version will simply rely on the presets. 4. foamSetup to take over the work done by foamSystemCheck and foamInstallationTest. Beyond this initial work, foamSetup can be later extended to rely on the package manager of each system, or simply based on "cat /etc/*-release | grep". A few example concepts on how to use the script: foamSetup scotch=default metis=default foamSetup scotch=debian foamSetup scotch=custom metis=none The first version of this script 'foamSetup' will be rather simple, by relying on the 'presets' folder. |
|
|
Thought: we add locations of external libraries to LD_LIBRARY_PATH, locations of external binaries to PATH. Why don't we add the locations of include files to the CPATH variable (supported in gcc, clang and icc). This could simplify some of the setup problems. |
|
|
> This could simplify some of the setup problems. Mmm... it depends on how it would be used... It would simplify the rule files... but only to the point of solving the issues in OpenFOAM. But we would risk contaminating the remaining shell environment for the end-user, if this is sourced from within bashrc/cshrc. From a user perspective, LD_LIBRARY_PATH still leaves open the selection of which library version is to be used when running another application, at least when the libraries are versioned. Similarly, the PATH can still pick up other executables, if they have different names, i.e. if the user knows what is meant to be used... The problem with relying on CPATH is that if this is not sourced at the rule files or wmake or Allwmake level, this may conflict with other building environments that the end-user may be trying to do with other software, e.g. using the system GMP for a system-based application, while using a more recent GMP for OpenFOAM... simply because header files usually aren't versioned in their names... This is somewhat consistent with how "ThirdParty-*/Allwmake" works, by using a local definition of CFLAGS et al. |
|
|
Sorry, maybe it is special feature of Mantis but I was not able to find unsubscribe link. Is there any? |
|
|
I'm sorry Alexey, I should have started a new report instead. I'll reference this report once I've got things ready. I'm closing this bug report as "resolved", as per comment #7164. The note is that only part of the original report+patch was applied. |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2016-11-11 10:25 | alexeym | New Issue | |
| 2016-11-11 10:25 | alexeym | File Added: 0004-System-wide-Scotch-and-METIS.patch | |
| 2016-11-11 10:35 | henry | Note Added: 0007141 | |
| 2016-11-11 15:18 | alexeym | Note Added: 0007150 | |
| 2016-11-11 16:27 | henry | Note Added: 0007154 | |
| 2016-11-11 17:21 | alexeym | Note Added: 0007156 | |
| 2016-11-11 17:43 | henry | Note Added: 0007158 | |
| 2016-11-11 18:08 | alexeym | Note Added: 0007159 | |
| 2016-11-11 18:21 | alexeym | Note Added: 0007160 | |
| 2016-11-11 18:25 | henry | Note Added: 0007161 | |
| 2016-11-12 09:12 | henry | Note Added: 0007164 | |
| 2016-11-13 13:24 | wyldckat | Note Added: 0007179 | |
| 2016-11-13 14:41 | henry | Note Added: 0007180 | |
| 2016-11-13 21:25 | alexeym | Note Added: 0007195 | |
| 2016-11-13 21:45 | henry | Note Added: 0007196 | |
| 2016-11-13 22:05 | wyldckat | Note Added: 0007198 | |
| 2016-11-13 22:42 | henry | Note Added: 0007199 | |
| 2016-11-13 23:26 | wyldckat | Note Added: 0007200 | |
| 2016-11-20 23:42 | wyldckat | Note Added: 0007251 | |
| 2016-12-02 10:55 | MattijsJ | Note Added: 0007378 | |
| 2016-12-02 11:40 | wyldckat | Note Added: 0007379 | |
| 2016-12-02 21:56 | alexeym | Note Added: 0007381 | |
| 2016-12-03 19:51 | wyldckat | Assigned To | => henry |
| 2016-12-03 19:51 | wyldckat | Status | new => resolved |
| 2016-12-03 19:51 | wyldckat | Resolution | open => fixed |
| 2016-12-03 19:51 | wyldckat | Fixed in Version | => dev |
| 2016-12-03 19:51 | wyldckat | Note Added: 0007388 |
