View Issue Details

IDProjectCategoryView StatusLast Update
0001232OpenFOAMBugpublic2016-05-04 20:35
Reporteruser313Assigned Tohenry  
PrioritynormalSeverityfeatureReproducibilityalways
Status resolvedResolutionfixed 
PlatformLinuxOSOtherOS Version(please specify)
Fixed in Versiondev 
Summary0001232: WM_CC
DescriptionSay 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++
TagsNo tags attached.

Relationships

related to 0001215 resolvedhenry conformalVoronoiMesh/foamyHexMesh build ignores ThirdParty boost 

Activities

albertop

2014-03-25 05:09

reporter   ~0002974

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.

user313

2014-03-25 13:59

  ~0002975

Last edited: 2014-03-25 14:00

@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.

albertop

2014-03-25 14:04

reporter   ~0002976

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).

user313

2014-03-25 14:10

  ~0002977

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.

henry

2016-02-03 22:23

manager   ~0005913

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.

wyldckat

2016-02-07 22:50

updater   ~0005924

@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!

henry

2016-02-08 14:36

manager   ~0005925

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.

henry

2016-02-08 14:37

manager   ~0005926

Also if the directory structures are separated between shells we could avoid the need for the .sh, .csh etc. extensions.

henry

2016-02-10 10:26

manager   ~0005930

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

wyldckat

2016-02-10 11:18

updater   ~0005931

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

henry

2016-02-10 11:42

manager   ~0005932

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.

wyldckat

2016-02-15 23:05

updater  

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

#------------------------------------------------------------------------------
paraview (4,878 bytes)   

wyldckat

2016-02-15 23:14

updater   ~0005947

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.

henry

2016-02-16 08:59

manager   ~0005949

commit 1ad975a69fc9f2acd3005ee98de67e2964b56d64

wyldckat

2016-03-06 10:10

updater  

wyldckat

2016-03-06 10:49

updater   ~0005996

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.

henry

2016-03-06 19:26

manager   ~0006001

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.

wyldckat

2016-03-06 20:03

updater   ~0006003

> 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!

henry

2016-03-09 09:09

manager   ~0006026

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

wyldckat

2016-03-20 22:34

updater  

wyldckat

2016-03-20 22:45

updater   ~0006052

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".

henry

2016-03-22 08:03

manager   ~0006055

Thanks, I have applied these changes:

commit 64d256f79e95f84385cc0e05b8fbb41ed6104f5d

Issue History

Date Modified Username Field Change
2014-03-21 14:49 user313 New Issue
2014-03-25 05:09 albertop Note Added: 0002974
2014-03-25 13:59 user313 Note Added: 0002975
2014-03-25 14:00 user313 Note Edited: 0002975
2014-03-25 14:04 albertop Note Added: 0002976
2014-03-25 14:10 user313 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