View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0001232 | OpenFOAM | Bug | public | 2014-03-21 14:49 | 2016-05-04 20:35 |
Reporter | Assigned To | henry | |||
Priority | normal | Severity | feature | Reproducibility | always |
Status | resolved | Resolution | fixed | ||
Platform | Linux | OS | Other | OS Version | (please specify) |
Fixed in Version | dev | ||||
Summary | 0001232: WM_CC | ||||
Description | Say I want to use system's Intel compiler now, and I choose "system" for "foamCompiler", but this does not change WM_CC setting, which is set in "WM_ARCH" section. WM_CC here should be icc and WM_CXX should be icpc, not gcc and g++ | ||||
Tags | No tags attached. | ||||
|
I am not sure this is a bug. If you want to use ICC, you should use the rules for the Intel compiler. This is achieved by specifying "Icc" as compiler. Note that this is required also because optimization options for Intel compilers are different from gcc (-O3 would likely produce slower code in Icc, where the recommended option is -O2). You can find more details in the rules, and in the documentation for Icc. |
|
@Alberto, What you are talking about is a totally different thing. I am talking about the WM_CC in the settings.sh file, at least, it needs to be overwritten once Intel compiler is chosen. It may not affect OF compilation very much, but it will surely affect some 3rd party software. |
|
I see your point, but the "system" option seems to assume the system compiler is gcc (which is probably the case on most Linux systems). |
|
That's why I want to give them a heads up. :) E.g., on our cluster, I found if `CC` is not set correctly, mpi library may not work. |
|
WM_CC and WM_CXX can be set to whatever you want in your prefs.sh file after setting WM_COMPILER=Icc To automate this in some way with sensible user settings and overrides will add a lot of complexity to the already complex settings.sh file. If you think it is worth the complexity please provide a patch for your proposal for consideration. |
|
@Henry: Would it be acceptable to split "settings.sh" (and "settings.csh") into something with a structure a bit more similar to wmake's "rules" structure? I'm thinking about something along the lines of the following directory (folder) structure: - etc - config - architecture - Linux.sh - SunOS.sh - compiler - general - Gcc.sh - Icc.sh - Clang.sh - custom - Gcc.sh - Gcc45.sh - Gcc46.sh - ... - system.sh - mpi - SYSTEMOPENMPI.sh - OPENMPI.sh - ... * This would essentially break up the current "case" blocks into a file tree. * The "compiler/custom" folder is for the custom version settings for a particular compiler, namely "foamCompiler=OpenFOAM | ThirdParty". * The "system.sh" could then rely on "general/Gcc.sh", "general/Icc.sh", "general/Clang.sh" and other similar scripts for when using the system's compiler with a particular structure. * The "architecture/*.sh" files would not define the variables such as "export WM_CC". * And then have the "WM_CC" et al variables defined within the respective compiler shell script, such as "general/Gcc.sh"? Where this script would be sourced from "Gcc.sh", "Gcc45.sh" and others that are related? The upside is that "settings.sh" would then be considerably trimmed down and it could be a bit more easily be overridden by the user in local settings. The downside is the number of files that would increase considerably within "etc/config", but at least they would be a bit more manageable individually. And while we're at it, possibly to also split the "sh" and "csh" definitions into two separate directory (folder) structures? This would also make it a little bit easier for independent repositories to have "git clone ... etc/xyz && source etc/xyz/rc" of other shell environments that were similarly structured, such as fish and zsh, yet not maintained by the OpenFOAM Foundation ;) If this is acceptable, it should be fairly easy (although time consuming, but nice as a mellowing-out coding session) to build this structure and to test all of the possibilities and we (I with/or anyone else in the community) can provide this new structure! |
|
Do you think that this would provide sufficient benefit? It would create a very large number of small files. Also the separation is not clean, for example the OPENMPI.sh would still contain a selection based on the OS type. I take your point about "csh" given that this shell is pretty much deprecated in GNU/Linux and zsh is probably used more. However, your proposed directory is not necessary for this to be convenient, it only requires the current structure to be separated between shell types which would probably be a good thing. |
|
Also if the directory structures are separated between shells we could avoid the need for the .sh, .csh etc. extensions. |
|
commit 715d1bf2c08a961a111b187fe09425e92cdc394d Author: Henry Weller <http://cfd.direct> Date: Wed Feb 10 10:22:25 2016 +0000 OpenFOAM-dev/etc: separated scripts for bash and csh into separate directories etc/config.sh and etc/config.csh This structure is more convenient to add support for other shells, e.g. zsh, fish etc. Resolves feature request to simplify support for other shells in http://www.openfoam.org/mantisbt/view.php?id=1232 |
|
Sorry for taking a while to get back to you on this. The latest update makes it a lot easier to maintain, looks sharper and nicer! You stated the other day: > Also the separation is not clean, for example the OPENMPI.sh would still contain a selection based on the OS type. I'm a bit confused on this issue, given that the case option for "WM_MPLIB=OPENMPI" is currently identical for any OS type. > Do you think that this would provide sufficient benefit? It would create a very large number of small files. Also the separation is not clean, for example the OPENMPI.sh would still contain a selection based on the OS type. I'm still divided on this. Yes, there are dependencies on the OS type, but that's hard to decouple, unless we would increase the branching even further, which would become unmanageable :( But on the other hand, having such a lengthy "settings" file makes it look a bit too cumbersome. I've got to think more on this. Hopefully I'll have a breakthrough in how to organize this neatly till the weekend. But just one big question is still pending in my head: Wasn't WM_CC et al originally conceived as a measure for circumventing issues with certain compilers? Because I remember the first time I saw it in "ThirdParty-*/Allwmake", where it was being used for enforcing that GCC was used for building Scotch. Scotch might build nowadays with ICC, but I vaguely remember that this wasn't always the case. The other smaller/related question is this: Wouldn't it make sense to use WM_CC et al in the files at "wmake/rules"? Because these are mostly hard-coded to use gcc/g++ and there are situations where we need a specific version, e.g.: gcc-4.5/g++-4.5 |
|
You are right currently the OpenMPI settings are the same for all OSs but this is not so for other MPI implementations. The original problem we had with old icpc versions is that we could get OpenFOAM to compile but not ParaView, I can't remember about scotch. It is likely that all the ThirdParty packages will now compile with recent icpc releases. It would be possible to use WM_CC in wmake/rules if we can guarantee that all ThirdParty libraries will compile with all compiler types and versions which I think is unwise. I think we should maintain the freedom to set WM_CC independently. |
|
paraview (4,878 bytes)
#----------------------------------*-sh-*-------------------------------------- # ========= | # \\ / F ield | OpenFOAM: The Open Source CFD Toolbox # \\ / O peration | # \\ / A nd | Copyright (C) 2011-2016 OpenFOAM Foundation # \\/ M anipulation | #------------------------------------------------------------------------------ # License # This file is part of OpenFOAM. # # OpenFOAM is free software: you can redistribute it and/or modify it # under the terms of the GNU General Public License as published by # the Free Software Foundation, either version 3 of the License, or # (at your option) any later version. # # OpenFOAM is distributed in the hope that it will be useful, but WITHOUT # ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or # FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License # for more details. # # You should have received a copy of the GNU General Public License # along with OpenFOAM. If not, see <http://www.gnu.org/licenses/>. # # File # etc/config.sh/paraview # # Description # Setup file for paraview-[3-5].x # Sourced from OpenFOAM-<VERSION>/etc/bashrc or from foamPV alias # # Note # The env. variables 'ParaView_DIR' and 'ParaView_MAJOR' # are required for building plugins #------------------------------------------------------------------------------ # clean the PATH cleaned=$($WM_PROJECT_DIR/bin/foamCleanPath "$PATH" \ "$WM_THIRD_PARTY_DIR/platforms/$WM_ARCH$WM_COMPILER/cmake- \ $WM_THIRD_PARTY_DIR/platforms/$WM_ARCH$WM_COMPILER/paraview-" \ ) \ && PATH="$cleaned" # determine the cmake to be used unset CMAKE_HOME for cmake in cmake-3.2.1 cmake-2.8.12.1 cmake-2.8.8 cmake-2.8.4 cmake-2.8.3 \ cmake-2.8.1 do cmake=$WM_THIRD_PARTY_DIR/platforms/$WM_ARCH$WM_COMPILER/$cmake if [ -r $cmake ] then export CMAKE_HOME=$cmake export PATH=$CMAKE_HOME/bin:$PATH break fi done #- ParaView version, automatically determine major version #export ParaView_VERSION=3.12.0 #export ParaView_VERSION=4.0.1 #export ParaView_VERSION=4.1.0 #export ParaView_VERSION=4.3.1 #export ParaView_VERSION=4.4.0 export ParaView_VERSION=5.0.0 export ParaView_MAJOR=detect # Evaluate command-line parameters for ParaView _foamParaviewEval() { while [ $# -gt 0 ] do case "$1" in ParaView*=*) # name=value -> export name=value eval "export $1" ;; esac shift done } # Evaluate command-line parameters _foamParaviewEval $@ # set MAJOR version to correspond to VERSION # ParaView_MAJOR is "<digits>.<digits>" from ParaView_VERSION case "$ParaView_VERSION" in "$ParaView_MAJOR".* ) # version and major appear to correspond ;; [0-9]*) # extract major from the version ParaView_MAJOR=$(echo $ParaView_VERSION | \ sed -e 's/^\([0-9][0-9]*\.[0-9][0-9]*\).*$/\1/') ;; esac export ParaView_VERSION ParaView_MAJOR paraviewInstDir=$WM_THIRD_PARTY_DIR/ParaView-$ParaView_VERSION paraviewArchName=ParaView-$ParaView_VERSION export ParaView_DIR=$WM_THIRD_PARTY_DIR/platforms/$WM_ARCH$WM_COMPILER/$paraviewArchName # set paths if binaries or source are present if [ -r $ParaView_DIR -o -r $paraviewInstDir ] then export ParaView_INCLUDE_DIR=$ParaView_DIR/include/paraview-$ParaView_MAJOR if [ ! -d $ParaView_INCLUDE_DIR -a -d $ParaView_DIR/include/paraview-3.0 ] then export ParaView_INCLUDE_DIR=$ParaView_DIR/include/paraview-3.0 fi ParaView_LIB_DIR=$ParaView_DIR/lib/paraview-$ParaView_MAJOR if [ ! -d $ParaView_LIB_DIR -a -d $ParaView_DIR/lib/paraview-3.0 ] then ParaView_LIB_DIR=$ParaView_DIR/lib/paraview-3.0 fi export PATH=$ParaView_DIR/bin:$PATH export LD_LIBRARY_PATH=$ParaView_LIB_DIR:$LD_LIBRARY_PATH export PV_PLUGIN_PATH=$FOAM_LIBBIN/paraview-$ParaView_MAJOR if [ "$FOAM_VERBOSE" -a "$PS1" ] then echo "Using paraview" echo " ParaView_DIR : $ParaView_DIR" echo " ParaView_LIB_DIR : $ParaView_LIB_DIR" echo " ParaView_INCLUDE_DIR : $ParaView_INCLUDE_DIR" echo " PV_PLUGIN_PATH : $PV_PLUGIN_PATH" fi # add in python libraries if required paraviewPython=$ParaView_DIR/Utilities/VTKPythonWrapping if [ -r $paraviewPython ] then if [ "$PYTHONPATH" ] then export PYTHONPATH=$PYTHONPATH:$paraviewPython:$ParaView_LIB_DIR else export PYTHONPATH=$paraviewPython:$ParaView_LIB_DIR fi fi else unset PV_PLUGIN_PATH fi unset _foamParaviewEval unset cleaned cmake paraviewInstDir paraviewPython #------------------------------------------------------------------------------ |
|
I've attached the file "paraview", for updating a little bit the file "etc/config.sh/paraview". It is only a formatting change, by relying on "$()" instead of "``" and then breaking the long lines into 80 columns, which is allowed with "$()". The reason why "$()" is better than "``" was mentioned in an old bug report, but I can't find it with Google :( Regarding the compiler options and WM_CC: I'm still thinking about changing the optional script name "compiler" to "compiler$WM_COMPILER", in order to allow to more easily have an example for "compilerICC" to override the WM_CC and WM_CXX variables... but I haven't 'converged' on a solution yet, given that these custom "compiler" scripts are currently only available for custom-built compilers. |
|
commit 1ad975a69fc9f2acd3005ee98de67e2964b56d64 |
|
|
|
Took me almost a month, but I've finally managed to at least get a breakthrough on several details we should try and aim for. Along with this, I'll be adding a proposal in issue #1215, which depends on this proposal. I've attached the package "bash_proposal_v1.tar.gz", which is meant to be unpacked inside the "OpenFOAM-dev" folder. It's indexed to a recent commit e0451c75ec64383. This proposal contains the following changes, currently all only for bash, so that it can be more easily reviewed: 1. "foamCompiler" was changed to a more permanent "WM_COMPILER_TYPE" environment variable, so that it can be used by 3rd party installation scripts, such as "makeGcc", "makeLLVM" and so on. More on this will be provided in issue #1215. 2. The script functions such as "_foamSource()" and "_foamAddPath()" were moved to a new file "etc/config.sh/functions". It has the ability to set or unset, depending on whether "WM_BASH_FUNCTIONS" is defined or not. This allows for these functions to be reused by other scripts, such as "makeGcc". 3. The script "etc/config.sh/CGAL" relies on whether a local environment variable "SOURCE_CGAL_VERSIONS_ONLY" is defined or not, so that it will load only the version settings if it's defined. This is to make it easier to call this script from "makeCGAL". Although it still feels a bit of a clunky hack, but I didn't manage to deduce any other way we could do this :( I didn't add indentation within the if-block, to make it easier to read the changes. In addition, the local variable "common_path" is used to shorten the length of the lines and use slightly less repeated code. 4. Added another new script "etc/config.sh/compiler", which has only the version numbers for the compilers taken out from the "settings" file. It currently depends on "WM_COMPILER_TYPE" for setting the variables, the same way it did with "foamCompiler". This script is now always sourced from the "settings" file, for the following reasons: - "makeGCC" and "makeLLVM" can now take advantage of this script file. - The example "compiler" script (detailed next) can rely on this script file and then override parameters on-demand, as well as allowing for system compilers to have dedicated settings, such as setting "WM_CC". This is similar to how the example environment script for "paraview" works. 5. To the script "etc/config.sh/example/compiler" were added a few more examples: - It now starts with a block where it first loads the default "compiler" script. - Has a "WM_COMPILER=Gcc48u" case example for when we try to use GCC 4.8 in Ubuntu 15.10. This is just to give the idea that in a particular system, we might have several system-wide compiler versions. For example, in Ubuntu 15.10, there is GCC 4.7, 4.8 and 5.2, which could be used for testing performances or compatibility with some other 3rd party library. - Has the "WM_COMPILER=Icc" case example, related to the original bug report, where "WM_CC=icc" and "WM_CXX=icpc", so that the user then simply copies this file to their own local preferences folder. 6. Small bug fix in "etc/config.sh/mpi", where unsetting "minBufferSize" was missing at the end of the script. 7. Small change in "etc/config.sh/paraview", where "CMAKE_ROOT" is set along with "CMAKE_HOME". This is due to a rare issue that occurs on people's systems where they have a custom system-wide CMake version installed and which is used by having "CMAKE_ROOT" set on that environment. This can mess up OpenFOAM's custom ParaView builds, given that conflicting CMake versions can lead to not building ParaView at all. - This is something that has been on my to-do list to report for quite a while now, first spotted here: http://www.cfd-online.com/Forums/openfoam-installation/135658-openfoam-2-3-0-installation-rhel-5-a.html#post496272 - back in 2014. - For more details about "CMAKE_ROOT": https://cmake.org/Wiki/CMake_Useful_Variables 8. The scripts "_foamAddPath _foamAddLib _foamAddMan" were not being unset at the end of "settings". They are now unset at the end of "bashrc", through a call to the new double-use "functions" script. @Henry: If these changes are acceptable, I can take care of also providing the same structural changes for the csh version; although these are not as critical for csh, since csh is not used in the build scripts... but you'll know this better than me, namely how many people/clients actually use csh on the same level as we use bash/dash/sh. |
|
Thanks Bruno, I am happy with these changes and will test them during the week before pushing. I doubt many people are still using csh; my guess is zsh and fish are now used more commonly and for the python obsessed xonsh is probably the most interesting. Maybe support for all of these specialist shells should be maintained outside the main OpenFOAM code-base. |
|
> and for the python obsessed xonsh is probably the most interesting. Wow! I didn't know "xonsh" even existed! Many thanks for mentioning it! Although it makes perfect sense that it exists... this could potentially unleash the ultimate-open-source-science-human-readable-workbench... or simply end up just it being another one for the list of attempts at such a thing... but still, it looks very promising! |
|
I have applied your changes to bashrc, config.sh and updated other references and uses of foamCompiler to WM_COMPILER_TYPE for consistency: commit 711ec0e39ddd3805129bc3e1e92131f65a6d9ebf commit 41035d1643cd9df40a1dded0fa16ac145dc42457 commit 8c1f5eae3962934adf45c978895f40099d4af8be |
|
|
|
I've attached "bash_proposal_v2.tar.gz", which provides the following files and respective changes: - "etc/config.sh/CGAL": - Indented the contents of the recently added if block. - Added comment about using system versions. - Library paths are now only added if the respective version is not "boost-system" and "cgal-system". - "src/renumber/Allwmake": - It now relies on the previous file to get the version for Boost (the same way as in "makeCGAL"). This is so that it will also build "SloanRenumber" if "boost_version" is set to "boost-system". - "applications/utilities/mesh/generation/Allwmake": - It now also relies on the script "config.sh/CGAL" to get the version for CGAL. If "cgal_version" is set to "cgal-system", it will now also build "foamy*Mesh" utilities and respective libraries. This is also related to issue #1215. I didn't remember this before, because that initial report didn't specify that there were other locations where the build would not work due to setting Boost version to "system". |
|
Thanks, I have applied these changes: commit 64d256f79e95f84385cc0e05b8fbb41ed6104f5d |
Date Modified | Username | Field | Change |
---|---|---|---|
2014-03-21 14:49 |
|
New Issue | |
2014-03-25 05:09 | albertop | Note Added: 0002974 | |
2014-03-25 13:59 |
|
Note Added: 0002975 | |
2014-03-25 14:00 |
|
Note Edited: 0002975 | |
2014-03-25 14:04 | albertop | Note Added: 0002976 | |
2014-03-25 14:10 |
|
Note Added: 0002977 | |
2016-02-03 22:23 | henry | Note Added: 0005913 | |
2016-02-07 22:50 | wyldckat | Note Added: 0005924 | |
2016-02-08 14:36 | henry | Note Added: 0005925 | |
2016-02-08 14:37 | henry | Note Added: 0005926 | |
2016-02-10 10:26 | henry | Note Added: 0005930 | |
2016-02-10 11:18 | wyldckat | Note Added: 0005931 | |
2016-02-10 11:42 | henry | Note Added: 0005932 | |
2016-02-15 23:05 | wyldckat | File Added: paraview | |
2016-02-15 23:14 | wyldckat | Note Added: 0005947 | |
2016-02-16 08:59 | henry | Note Added: 0005949 | |
2016-02-21 20:24 | wyldckat | Relationship added | related to 0001215 |
2016-03-06 10:10 | wyldckat | File Added: bash_proposal_v1.tar.gz | |
2016-03-06 10:49 | wyldckat | Note Added: 0005996 | |
2016-03-06 11:00 | wyldckat | Assigned To | => henry |
2016-03-06 11:00 | wyldckat | Status | new => assigned |
2016-03-06 19:26 | henry | Note Added: 0006001 | |
2016-03-06 20:03 | wyldckat | Note Added: 0006003 | |
2016-03-09 09:09 | henry | Note Added: 0006026 | |
2016-03-20 22:34 | wyldckat | File Added: bash_proposal_v2.tar.gz | |
2016-03-20 22:45 | wyldckat | Note Added: 0006052 | |
2016-03-22 08:03 | henry | Note Added: 0006055 | |
2016-05-04 20:35 | henry | Status | assigned => resolved |
2016-05-04 20:35 | henry | Fixed in Version | => dev |
2016-05-04 20:35 | henry | Resolution | open => fixed |