Skip to content

support OpenFOAM package(s) - #3528

Merged
tgamblin merged 8 commits into
spack:developfrom
olesenm:openfoam
Mar 30, 2017
Merged

support OpenFOAM package(s)#3528
tgamblin merged 8 commits into
spack:developfrom
olesenm:openfoam

Conversation

@olesenm

@olesenm olesenm commented Mar 23, 2017

Copy link
Copy Markdown
Contributor

Other updates (paraview, scotch, mxml) have now been done elsewhere

@davydden davydden left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO this PR needs to split into at least 5: flex, mxml, openfoam, paraview, qt. It would be much easier to review and non-controversial changes like mxml will be quick to merge.

homepage = "https://github.com/westes/flex"
url = "https://github.com/westes/flex/releases/download/v2.6.1/flex-2.6.1.tar.gz"

version('2.6.3', 'a5f65570cd9107ec8a8ec88f17b31bb1')

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do you remove this one? If thats due to openfoam, then add constrain there in depends_on().

@olesenm olesenm Mar 24, 2017

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like a git (user) issue with rolling back or merging - I'll get it sorted.
I had had problems with spack resolving 2.6.1 and complaining when 2.6.3 was further up.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the error, which is why I hacked the flex spec. The error occurs when building something that depends_on("flex")

==> Error: Invalid spec: 'flex@2.6.3%gcc@4.8 arch=linux-opensuse42-x86_64 ^bison@3.0.4%gcc@4.8 arch=linux-opensuse42-x86_64 ^bzip2^gettext@0.19.8.1%gcc@4.8+bzip2+curses+git~libunistring+libxml2+tar+xz arch=linux-opensuse42-x86_64 ^help2man@1.47.4%gcc@4.8 arch=linux-opensuse42-x86_64 ^libsigsegv^libxml2^m4@1.4.18%gcc@4.8+sigsegv arch=linux-opensuse42-x86_64 ^mawk^ncurses^pkg-config@0.9.0:^tar^xz^zlib@1.2.11%gcc@4.8+pic+shared arch=linux-opensuse42-x86_64'.
Package flex requires version :2.6.1, but spec asked for 2.6.3

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error occurs when building something that depends_on("flex")

please open a separate issue for this problem and also state what that something is, so that others can reproduce. Regardless, the change you have should be reverted, as it's not a solution, but only a workaround.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can probably be debugged using the spack spec command; no need to spend time building things.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing 2.6.3 is consistent with the previous removal of 2.6.2. Obviously, we cannot continue this forever. Both should be added back, and then packages that don't work the new version should be set to use @:2.6.1. Someone needs to grep for packages that use flex, and change/test them accordingly.

Also... versions should be in reverse numerical order.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you read westes/flex#113, you'll see that 2.6.2 had serious flaws that broke backwards compatibility. This was fixed in 2.6.3, so we just commented out 2.6.2 with a note. Before we realized that this was a flex problem and not a "everything that depends on flex" problem, we must have added depends_on('flex@:2.6.1') to at least one package. This can be removed now. The real problem is a bug in the concretization algorithm though.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had the same thoughts. The problem is/was scotch. I tried building scotch with flex 2.6.3, but it didn't look any better.


version('4.4.0', 'fa1569857dd680ebb4d7ff89c2227378')
version('5.0.0', '4598f0b421460c8bbc635c9a1c3bdbee')
version('5.0.1', 'fdf206113369746e2276b95b257d2c9b')

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please arrange versions in ascending order

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now as separate pull request: #3537

# Notes.
# - Resolution of flex, zlib needs more attention
# - Paraview/VTK build - needs adjustments in ~/.spack/packages.yaml
class Openfoam(Package):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this package should be renamed openfoam-plus. The latest release of OpenFOAM+ is 1612, while the latest release of OpenFOAM is 4.1. The homepages are different (openfoam.com vs openfoam.org) and the tarballs are different.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Understood, but I'm not sure how long the '+' part of plus will actually be around (both OpenFOAM and OpenFOAM+ are in use at OpenCFD). The main intention of the '+' was to distinguish from the 3.0 .org version, which dropped many features that were given to them for release. For clarity and to remain neutral, either we keep openfoam as a package name and supply +com and +org variants (eventually adding in +extend in the process), or we spit things entirely and have openfoam-com, openfoam-org, foam-extend as packages, each providing 'openfoam' as a virtual (similar to mpi treatment). This would also allow other combinations without any further problems. Additional packages like swak4Foam that build with several different versions could require 'openfoam' and then be able to build various variants. This sounds to me to be the cleanest solution.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we spit things entirely and have openfoam-com, openfoam-org, foam-extend as packages, each providing 'openfoam' as a virtual (similar to mpi treatment).

i like this idea, let's see what others think.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we spit things entirely and have openfoam-com, openfoam-org, foam-extend as packages, each providing 'openfoam' as a virtual (similar to mpi treatment).

another angle: if a lot of things are the same in terms of build procedure (variants/dependencies/options/etc), then the common functionality can be factored out in an auxiliary class like what is done for IntelInstaller in intel/package.py and reused in intel-mkl, intel-parallel-studio, etc. We would avoid code duplication, but also make sure that there is no confusion in versions of each package (version()) and tarbal sources.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have reworked various flavours so that the methods looks quite a bit more similar, but I don't think there is much refactoring possible. We can discuss more when I push individual merge requests (later).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just waiting for the paraview pull request to get merged and then I can fire off an update for openfoam (since it depends on paraview includes which were previously missing).

if version >= Version('2.6.1'):
if version >= Version('2.6.3'):
url += "/releases/download/v{0}/flex-{0}.tar.gz".format(version)
elif version >= Version('2.6.1'):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe I'm missing something, but I don't understand this change.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Related to comment above about issues with resolving 2.6.3, I'll drop this line.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The whole url_for_version() can and should be simplified. In my tests, ALL versions can be obtained from the "archives" folder. Please get rid of url_for_version() entirely and set url='https://github.com/westes/flex/archive/v2.6.3.tar.gz'. That should work; @adamjstewart is there an easy way to test that all versions can download and checksum with the new URL?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@citibeth

$ spack fetch flex@2.6.1

for each version.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After reading more about flex problems - I don't really want to touch this package. It will not be part of any updated pull request.

if self.parallel:
nprocs = self.make_jobs \
if self.make_jobs else multiprocessing.cpu_count()
if nprocs > 1: args.extend(['-j', str(nprocs)])

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should just use make_jobs. This variable is set when you run spack install --jobs 8 or spack install -j16. Otherwise, the user wouldn't be able to change this value.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How can the user then compile with all available? In OpenFOAM we distinguish between build with '-j NUM' and a plain '-j', the latter meaning 'use all available cpus'.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

by default spack install <package> will use all available cpus, but a user can also specify -j X. So i would say you should always use -j make_jobs, which will use all CPUs by default.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good, I just started learning spack this week.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is one (minor) example where an individual package build system tries to be too smart, doing things for itself that are better managed externally.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I traced back to where this arose. In older versions of OpenFOAM we only had an environment variable for this. If self.make_jobs == None, then I don't see how else to pass in a default number. With a normal make it is indeed no problem, just use -j without a number.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use:

if make_jobs:
    args.extend(['-j', str(make_jobs)])

I assume the default is not parallel.

@olesenm

olesenm commented Mar 24, 2017

Copy link
Copy Markdown
Contributor Author

Is there a minimum python version that is causing the failures? Or is it due to the +paraview resolution issues that I experienced before?

@davydden

davydden commented Mar 24, 2017

Copy link
Copy Markdown
Member

Is there a minimum python version that is causing the failures?

Not sure if that's what you ask, but you can check failures of Travis if you click on Details, it tests Python 2.6 and 2.7. You'll see that it complains about openfoam/package.py, i.e.

E             File "/home/travis/build/LLNL/spack/var/spack/repos/builtin/packages/openfoam/package.py", line 217
E               open(join_path("etc", "prefs.sh"),  'w') as bsh, \
E                                                              ^
E           SyntaxError: invalid syntax

homepage = "https://github.com/westes/flex"
url = "https://github.com/westes/flex/releases/download/v2.6.1/flex-2.6.1.tar.gz"

version('2.6.3', 'a5f65570cd9107ec8a8ec88f17b31bb1')

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can probably be debugged using the spack spec command; no need to spend time building things.

if version >= Version('2.6.1'):
if version >= Version('2.6.3'):
url += "/releases/download/v{0}/flex-{0}.tar.gz".format(version)
elif version >= Version('2.6.1'):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The whole url_for_version() can and should be simplified. In my tests, ALL versions can be obtained from the "archives" folder. Please get rid of url_for_version() entirely and set url='https://github.com/westes/flex/archive/v2.6.3.tar.gz'. That should work; @adamjstewart is there an easy way to test that all versions can download and checksum with the new URL?

homepage = "https://github.com/westes/flex"
url = "https://github.com/westes/flex/releases/download/v2.6.1/flex-2.6.1.tar.gz"

version('2.6.3', 'a5f65570cd9107ec8a8ec88f17b31bb1')

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing 2.6.3 is consistent with the previous removal of 2.6.2. Obviously, we cannot continue this forever. Both should be added back, and then packages that don't work the new version should be set to use @:2.6.1. Someone needs to grep for packages that use flex, and change/test them accordingly.

Also... versions should be in reverse numerical order.

depends_on('cmake', type='build')

depends_on('scotch ~ metis', when='~ptscotch+scotch')
depends_on('scotch ~ metis + mpi', when='+ptscotch')

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you remove spaces from these strings?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I just need to understand the syntax more (I copied this from elsewhere). I guess this means that I need scotch without the metis variant, but only when scotch has been selected and ptscotch deselected? I can't really figure out why that is useful or needed.
Maybe it's supposed to require "scotch~metis" when the +metis variant has been selected?

Assuming that I get that sorted, what happens when I wish to add another constraint.
For example, if I'm building with +int64, I would like to require scotch also have +int64, but still retain the previous (faulty?) logic for ~ptscotch.

  • depend on scotch+int64

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, your interpretation of that logic is correct. I assume you just copied these from foam-extend? Maybe @nrichart can mention why ~ptscotch is necessary. It sounds like +scotch and +ptscotch are mutually exclusive.

## run_env.set('FOAM_INST_DIR', self.prefix)


def patch(self):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a docstring, please add the justification and reasoning behind these patches.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's especially important because this patch() method is HUGE.



def install(self, spec, prefix):
wrapper = './spack-Allwmake'

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is this wrapper? Please add comment.

if self.parallel:
nprocs = self.make_jobs \
if self.make_jobs else multiprocessing.cpu_count()
if nprocs > 1: args.extend(['-j', str(nprocs)])

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is one (minor) example where an individual package build system tries to be too smart, doing things for itself that are better managed externally.

if nprocs > 1: args.extend(['-j', str(nprocs)])

# Testing only: skip make and test install only
# allwmake = Executable('/usr/bin/mkdir -p platforms')

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what this does. But if it is testing... it should go in a separate test() method??? @adamjstewart ???

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just a very simple short-circuit to test installation semantics without actually building openfoam, which saves about 15-20min, even when compiling on 24cores.

Ideally I would like to have separate build and install stages, but this doesn't seem to be possible you aren't using configure or cmake?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can have separate build and install stages! If you look at class Package in lib/spack/spack/package.py, you'll see that the default phase is install:

class Package(PackageBase):                                                     
    """General purpose class with a single ``install``                          
    phase that needs to be coded by packagers.                                  
    """                                                                         
    #: The one and only phase                                                   
    phases = ['install']                                                        
    #: This attribute is used in UI queries that require to know which          
    #: build-system class we are using                                          
    build_system_class = 'Package'                                              
    # This will be used as a registration decorator in user                     
    # packages, if need be                                                      
    run_after('install')(PackageBase.sanity_check_prefix)    

If you look at AutotoolsPackage (spack edit -b autotools), it defines its own phases:

class AutotoolsPackage(PackageBase):
    ...
    #: Phases of a GNU Autotools package                                        
    phases = ['autoreconf', 'configure', 'build', 'install'] 

So if you add phases = ['build', 'install'] to OpenFOAM, Spack would run build() and install() methods. I really like @alalazo's design on this one.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you want to add tests that occur after build or install, you can do that as well. These wouldn't run by default, but would run if the user asked for spack install --run-tests. Let me know if you are interested in this and I can give you pointers.

@olesenm olesenm Mar 24, 2017

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sound like what I want. The first bit sounds promising phase = ...
The spack info now shows it having both phases. I added a method:

def build(self, spec, prefix):

but invoking it carps at me: has no build phase, or its build phase is not separated from install

So I think there must be something else missing. I also banged in a build_system_class for good measure, but didn't seem to help.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, is that method empty or did you stuff to it? Can you post the exact error message? Maybe @alalazo can help. I know I've done this before in several Python packages.

mark added 3 commits March 29, 2017 01:14
- provide 'openfoam' as virtual package.

- package as openfoam-com to reflect the distribution point.

This initial spack packaging for OpenFOAM supports a number of possible
variants and should handle 64-bit labels properly now that the scotch
package has been updated accordingly.
- provide 'openfoam' as virtual package.

- much of the build is now aligned with how the openfoam-com package
  looks, with the aim of future refactoring.

- avoid installing intermediate targets.

- contains its own environment sourcing script for the build, for more
  flexibility and robustness (doesn't touch the python build environ)
- provide 'openfoam' as a virtual package.

- this is largely a direct copy of the openfoam-com package.
  It has been supplied as a courtesy for users and to ensure maximum
  consistency in quality and naming between the foam-extend,
  openfoam-com and openfoam-org packages.
@olesenm

olesenm commented Mar 28, 2017

Copy link
Copy Markdown
Contributor Author

Refactored and reworked everything. It would indeed be nice to refactor some common code, but this might need more python expertise that I currently have.
Arguably this could be split into 3 merge requests (openfoam-com, openfoam-org, foam-extend), but I think that the similarity of the three packages warrants keeping them together.

I'm hoping that once these packages get into the repo, refactoring will be clearer.
There are some helper functions in the openfoam-com package that could migrate to the llnl fileutils, for example.

@olesenm olesenm changed the title support OpenFOAM package support OpenFOAM package(s) Mar 28, 2017

@tgamblin tgamblin left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @olesenm:

I wasn't able to get through the whole package, but there is a lot going on here and I'm not so familiar with wmake. The only major comments I have are:

  1. it looks like you're using uname to get arch info, but you want to be faithful to what Spack asks for in the spec -- especially for cross-compiled builds, and

  2. This seems to throw errors when it encounters an unknown OS/compiler pair, but it would be nice if it didn't so that people can try it with other compilers. I am not sure if that is a function of WMake only taking certain compiler names (Gcc, Clang, etc.) or if it is an artificial limitation. If it's just the former, it might be worthwhile to look at patching wmake to use Spack's wrappers instead of trying to map all the flags. We do this a lot for other make-ish build systems.

depends_on('python')
depends_on('flex')
depends_on('zlib')
depends_on('flex@:2.6.1') # <- restriction due to scotch

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you don't have a restriction on this, you should just rely on the one from scotch -- constraints on each package are merged before installation. Did you run into an issue without this?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without this I ran into the problem that flex offered 2.6.3, but depend required 2.6.1 and the I needed to manually have a flex@:2.6.1 on the command-line. The hard-coded solution is obviously not very nice either, but it looks like newer flex is quite broken.
Besides, I will definitely keep tracking the state of the openfoam builds in the future and chuck out this restriction when possible.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@olesenm: Ok I think that is a bug -- the version is probably getting set too soon in the concretizer. I have some forthcoming work to iron out issues like this, but your constraint should work for now.

return wm_options

# Trivial check
if self.compiler.name not in self.supported_compilers:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this necessary? We generally leave things unconstrained in Spack to make it easy to build new versions of things. If someone tries a strange compiler and it happens to work, is that bad? People encounter this a lot when porting to new machines.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My own experience with trying to build OpenFOAM is that if you aren't using a supported compiler, and a supported version of that compiler, everything breaks in mysterious ways. I tried contacting the developers but they wouldn't help me unless I coughed up money for a support contract.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The compromise solution is to not test for supported compilers, but just provide a few basic translations of spack -> openfoam (eg, intel -> Icc). The balance of compilers (gcc, clang, ...) are just capitalized (Gcc, Clang) for openfoam.
When everything is assembled (architecture, compiler), then check there is a wmake rule for this - and only complain then.

I think this is reasonably flexible, but still avoids trying to build combinations that openfoam doesn't know about. Regarding generalized compilers. I was thinking about this too. For the MPI portion, I've introduced a new USERMPI type for openfoam. Which means that openfoam knows about it, but it is up to the user (or spack) to generate a file with meaningful and relevant entries. Something similar might be possible for the compiler settings too.

We'll see how much effort this all takes, and we can take it off-line (my email is in commits).
Re: comp

)

def set_arch(self):
(sysname, nodename, release, version, machine) = os.uname()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This probably isn't going to work on cross-compiled platforms. You should get the target information for the build from the spec you're passed in install(). So, if you want Spack's machine info, you should do this:

# This is a general machine type.  Spack supports the notion
# of platforms with multiple OSs and targets (like Cray and BG/Q).
# e.g.: linux, darwin, cray, bgq
platform = spec.architecture.platform

# e.g., x86_64, haswell, ppc64le
target = spec.architecture.target

# could be better named.  e.g., redhat6, redhat7, ubuntu14, yosemite
os = spec.architecture.platform_os

Getting this information from uname is going to get you info about the login node. On Cray and BG/Q machines, that is not where you're going to run the code -- it's just where you'll build it.

Here's an example. On a Cray we can say:

spack install foo os=backend target=haswell

This builds for the default backend OS, e.g. compute node linux (CNL) on Cray. haswell is the default target on machines like Cori, but there are also targets like mic-knl if you want to compile for Knights Landing. You'll be handed a spec in install with an architecture triple like cray-cnl-haswell.

spack install foo os=frontend 

This builds for whatever the default frontend (login node) OS is, with a default target. Usually that is something like sles12 and x86_64.

Anyway -- TL; DR, you should get your machine info from the spec. For Linux and Darwin, you can assume for the moment that your target is pretty much what uname would give you, though we do plan to support more specialized targets like haswell on regular Linux boxes, too -- see #3206.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for the tip. This fits in fairly nicely, I've put it in, but I don't know how well openfoam will handle cross-compiling without more effort ... another day

@property
def rpath_info(self):
"""Define 'SPACKOpt' compiler optimization file to have wmake
use spack information with minimum modifications to OpenFOAM

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know much about wmake -- do you need to manually set up RPATHs, or can wmake just rely on Spack's compiler wrappers adding the RPATHs? Typically it's easier to do this if you can patch your make system to use Spack's custom compilers.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current strategy is to grab the rpath info from spack and stuff it all into an special "SPACKOpt" config file for OpenFOAM. This reuses almost all of the openfoam infrastructure by adding (2) generated files.

@adamjstewart adamjstewart Mar 29, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What @tgamblin is saying is that when wmake calls gcc, it is actually calling Spack's compiler wrappers. These compiler wrappers already add -L, -I, and RPATH when it gets built. So the question becomes, do you actually need to add this to SPACKOpt or is it done already?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@olesenm: I looked a little harder at the build. It looks like you do this:

    wm_compiler = self.supported_compilers[self.compiler.name]

and then you use this to pick an existing configuration file in OpenFOAM's build.

Suppose you picked the Gcc configuration. If that configuration ends up just trying to execute gcc and g++, then you shouldn't need to set RPATHs at all -- you'll pick up our wrappers (named gcc and g++ for GCC builds, icc and icpc for Intel builds, etc.), which already add -I, -L, and -Wl,-rpath flags for dependency include and lib(64) directories. We also set CC, CXX, and friends to point at the wrappers, so if your build automatically picks up the compiler from those, then it should work fine without manually setting any RPATHs.

If, on the other hand, wmake has its own fancy compiler search and tries to find an absolute path to some compiler, you may end up circumventing Spack's wrappers. If you look at the lua, parpack, or wannier90 builds (e.g., try spack edit wannier90) -- they all manually substitute the Spack compilers into a build in one way or another. Typically that is necessary for plain Makefile builds. You may need to do this if the build doesn't call plain gcc or if it does not respect CC, CXX, F77, and FC environment variables.

Does that make sense?


# 'DBUG': rpaths
return '{0}{1} {2}{3}'.format(
self.compiler.cxx_rpath_arg, install_libpath,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that the compiler adds these to the compile line anyway...

@tgamblin

Copy link
Copy Markdown
Member

@citibeth @adamjstewart @davydden: are your concerns addressed?

@tgamblin

tgamblin commented Mar 29, 2017

Copy link
Copy Markdown
Member

Oh and thanks for the PR, @olesenm! It will be great to get OpenFOAM into Spack!

@adamjstewart

Copy link
Copy Markdown
Member

I'll try to take a look at this later today, but this is awesome! My own users have been begging for OpenFOAM for years and I've been (unsuccessfully) trying to package this for months. See develop...adamjstewart:features/openfoam for my own attempt at getting things working. I basically just ported the open-foam package and hacked at it until it built, but I never got it completely working.

@citibeth

citibeth commented Mar 29, 2017 via email

Copy link
Copy Markdown
Member

@tgamblin

Copy link
Copy Markdown
Member

Apologies if this has been answered... but if this restriction is due to scotch, shouldn't it be in scotch/package.py?

It is there -- @olesenm is hitting a bug. I move to keep the workaround until I can make the concretizer smarter.

mark added 4 commits March 30, 2017 00:03
- also simplify the generation of mplib/compiler rules
- this setup requires more environment settings than USERMPI
  (openfoam-com), but is currently the only means of integration
  for openfoam-org

- simplify generation of mplib/compiler rules
- rename mpi rules from SPACK,SPACKMPI to USER,USERMPI for consistency
  with openfoam-com and to generalize for any build system.
@olesenm

olesenm commented Mar 29, 2017

Copy link
Copy Markdown
Contributor Author

Addressed various points from above. The amount of cleanup is more substantial than the number of commits may suggest. In a future version, I would like to factor out part of OpenfoamCom into something like OpenfoamPackage and inherit from that. This started too go fairly well, but I hit something where I couldn't see the parent's variables (eg, self.foam_cfg) and got lots of python errors.
But this is definitely something for a later round. I think we should get this in and one of the python guys can help with refactoring (I hope).

@olesenm

olesenm commented Mar 29, 2017

Copy link
Copy Markdown
Contributor Author

@davydden @adamjstewart @citibeth : please verify/signoff for having resolved the discussion.

- can be useful for future diagnostics and general record keeping

@tgamblin tgamblin left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@citibeth @adamjstewart: I'm ok with putting this in with all the upates @olesenm has done. I think we can continue to iterate on it in future PRs once that is done.

@citibeth

citibeth commented Mar 30, 2017 via email

Copy link
Copy Markdown
Member

@tgamblin

Copy link
Copy Markdown
Member

To their credit, they integrated their own build system, which is more than many folks! Also OpenFOAM may predate the widespread use of the more modern systems, so cut them some slack 😄.

@tgamblin
tgamblin merged commit 9e1abb1 into spack:develop Mar 30, 2017
diaena pushed a commit to diaena/spack that referenced this pull request May 26, 2017
* ENH: add package for building OpenFOAM (1612) from www.openfoam.com
- provide 'openfoam' as virtual package.
- package as openfoam-com to reflect the distribution point.

This initial spack packaging for OpenFOAM supports a number of possible
variants and should handle 64-bit labels properly now that the scotch
package has been updated accordingly.

* ENH: update package for foam-extend (extend-project.de)

- provide 'openfoam' as virtual package.

- much of the build is now aligned with how the openfoam-com package
  looks, with the aim of future refactoring.

- avoid installing intermediate targets.

- contains its own environment sourcing script for the build, for more
  flexibility and robustness (doesn't touch the python build environ)

* ENH: added package for building from openfoam.org

- provide 'openfoam' as a virtual package.

- this is largely a direct copy of the openfoam-com package.
  It has been supplied as a courtesy for users and to ensure maximum
  consistency in quality and naming between the foam-extend,
  openfoam-com and openfoam-org packages.

* CONFIG: add openfoam into bash completion providers list

* ENH: have openfoam-com use spack as USERMPI

- also simplify the generation of mplib/compiler rules

* ENH: have openfoam-org use spack as SYSTEMMPI

- this setup requires more environment settings than USERMPI
  (openfoam-com), but is currently the only means of integration
  for openfoam-org

- simplify generation of mplib/compiler rules

* ENH: simplify generation of mplib/compiler rules (foam-extend)

- rename mpi rules from SPACK,SPACKMPI to USER,USERMPI for consistency
  with openfoam-com and to generalize for any build system.

* STYLE: record spack tree as a log file (openfoam)

- can be useful for future diagnostics and general record keeping
xavierandrade pushed a commit to xavierandrade/spack that referenced this pull request Jun 16, 2017
* ENH: add package for building OpenFOAM (1612) from www.openfoam.com
- provide 'openfoam' as virtual package.
- package as openfoam-com to reflect the distribution point.

This initial spack packaging for OpenFOAM supports a number of possible
variants and should handle 64-bit labels properly now that the scotch
package has been updated accordingly.

* ENH: update package for foam-extend (extend-project.de)

- provide 'openfoam' as virtual package.

- much of the build is now aligned with how the openfoam-com package
  looks, with the aim of future refactoring.

- avoid installing intermediate targets.

- contains its own environment sourcing script for the build, for more
  flexibility and robustness (doesn't touch the python build environ)

* ENH: added package for building from openfoam.org

- provide 'openfoam' as a virtual package.

- this is largely a direct copy of the openfoam-com package.
  It has been supplied as a courtesy for users and to ensure maximum
  consistency in quality and naming between the foam-extend,
  openfoam-com and openfoam-org packages.

* CONFIG: add openfoam into bash completion providers list

* ENH: have openfoam-com use spack as USERMPI

- also simplify the generation of mplib/compiler rules

* ENH: have openfoam-org use spack as SYSTEMMPI

- this setup requires more environment settings than USERMPI
  (openfoam-com), but is currently the only means of integration
  for openfoam-org

- simplify generation of mplib/compiler rules

* ENH: simplify generation of mplib/compiler rules (foam-extend)

- rename mpi rules from SPACK,SPACKMPI to USER,USERMPI for consistency
  with openfoam-com and to generalize for any build system.

* STYLE: record spack tree as a log file (openfoam)

- can be useful for future diagnostics and general record keeping
EmreAtes pushed a commit to EmreAtes/spack that referenced this pull request Jul 10, 2017
* ENH: add package for building OpenFOAM (1612) from www.openfoam.com
- provide 'openfoam' as virtual package.
- package as openfoam-com to reflect the distribution point.

This initial spack packaging for OpenFOAM supports a number of possible
variants and should handle 64-bit labels properly now that the scotch
package has been updated accordingly.

* ENH: update package for foam-extend (extend-project.de)

- provide 'openfoam' as virtual package.

- much of the build is now aligned with how the openfoam-com package
  looks, with the aim of future refactoring.

- avoid installing intermediate targets.

- contains its own environment sourcing script for the build, for more
  flexibility and robustness (doesn't touch the python build environ)

* ENH: added package for building from openfoam.org

- provide 'openfoam' as a virtual package.

- this is largely a direct copy of the openfoam-com package.
  It has been supplied as a courtesy for users and to ensure maximum
  consistency in quality and naming between the foam-extend,
  openfoam-com and openfoam-org packages.

* CONFIG: add openfoam into bash completion providers list

* ENH: have openfoam-com use spack as USERMPI

- also simplify the generation of mplib/compiler rules

* ENH: have openfoam-org use spack as SYSTEMMPI

- this setup requires more environment settings than USERMPI
  (openfoam-com), but is currently the only means of integration
  for openfoam-org

- simplify generation of mplib/compiler rules

* ENH: simplify generation of mplib/compiler rules (foam-extend)

- rename mpi rules from SPACK,SPACKMPI to USER,USERMPI for consistency
  with openfoam-com and to generalize for any build system.

* STYLE: record spack tree as a log file (openfoam)

- can be useful for future diagnostics and general record keeping
amklinv pushed a commit that referenced this pull request Jul 17, 2017
* ENH: add package for building OpenFOAM (1612) from www.openfoam.com
- provide 'openfoam' as virtual package.
- package as openfoam-com to reflect the distribution point.

This initial spack packaging for OpenFOAM supports a number of possible
variants and should handle 64-bit labels properly now that the scotch
package has been updated accordingly.

* ENH: update package for foam-extend (extend-project.de)

- provide 'openfoam' as virtual package.

- much of the build is now aligned with how the openfoam-com package
  looks, with the aim of future refactoring.

- avoid installing intermediate targets.

- contains its own environment sourcing script for the build, for more
  flexibility and robustness (doesn't touch the python build environ)

* ENH: added package for building from openfoam.org

- provide 'openfoam' as a virtual package.

- this is largely a direct copy of the openfoam-com package.
  It has been supplied as a courtesy for users and to ensure maximum
  consistency in quality and naming between the foam-extend,
  openfoam-com and openfoam-org packages.

* CONFIG: add openfoam into bash completion providers list

* ENH: have openfoam-com use spack as USERMPI

- also simplify the generation of mplib/compiler rules

* ENH: have openfoam-org use spack as SYSTEMMPI

- this setup requires more environment settings than USERMPI
  (openfoam-com), but is currently the only means of integration
  for openfoam-org

- simplify generation of mplib/compiler rules

* ENH: simplify generation of mplib/compiler rules (foam-extend)

- rename mpi rules from SPACK,SPACKMPI to USER,USERMPI for consistency
  with openfoam-com and to generalize for any build system.

* STYLE: record spack tree as a log file (openfoam)

- can be useful for future diagnostics and general record keeping
@tgamblin tgamblin added this to the v0.11.0 milestone Nov 12, 2017
@ziolai

ziolai commented Feb 25, 2018

Copy link
Copy Markdown

I am trying to install Openfoam using spack on Mac OS 10.12.6 (Sierra/Darwin). To this end I first make sure that all of the auxilary packages such as boost install well. See #7170. Next, I encounter the difficulty that seemingly no provision has yet been made to install OpenFoam on Mac OS X using spack. The difficulty arises both the gcc and clang compilers. See below. Could you please advice?

==> Installing openfoam-com
==> OpenFOAM minimal env /Users/ziolai/spack/opt/spack/darwin-sierra-x86_64/gcc-4.6.4/openfoam-com-1706-ajiledeweiukg7lpiyiciffeakrkxhpf
==> Using cached archive: /Users/ziolai/spack/var/spack/cache/openfoam-com/openfoam-com-1706.tgz
==> Staging archive: /Users/ziolai/spack/var/spack/stage/openfoam-com-1706-ajiledeweiukg7lpiyiciffeakrkxhpf/OpenFOAM-v1706.tgz
==> Created stage in /Users/ziolai/spack/var/spack/stage/openfoam-com-1706-ajiledeweiukg7lpiyiciffeakrkxhpf
==> Added file spack-Allwmake
==> Added file README-spack
==> Ran patch() for openfoam-com
==> Building openfoam-com [Package]
==> Executing phase: 'configure'
==> Executing phase: 'build'
==> Error: InstallError: No wmake rule for darwinIntel64Gcc
InstallError: InstallError: No wmake rule for darwinIntel64Gcc

/Users/ziolai/spack/var/spack/repos/builtin/packages/openfoam-com/package.py:631, in build:
2 its environment first.
3 Only build if the compiler is known to be supported.
4 """

5 self.foam_arch.has_rule(self.stage.source_path)
6 self.foam_arch.create_rules(self.stage.source_path, self)
7
8 args = ['-silent']

See build log for details:
/Users/ziolai/spack/var/spack/stage/openfoam-com-1706-ajiledeweiukg7lpiyiciffeakrkxhpf/OpenFOAM-v1706/spack-build.out

==> Installing openfoam-com
==> OpenFOAM minimal env /Users/ziolai/spack/opt/spack/darwin-sierra-x86_64/clang-9.0.0-apple/openfoam-com-1706-qtosjuuluglsuyrvkai3z7o62iddnpas
==> Fetching https://sourceforge.net/projects/openfoamplus/files/v1706/OpenFOAM-v1706.tgz
######################################################################## 100.0%
==> Staging archive: /Users/ziolai/spack/var/spack/stage/openfoam-com-1706-qtosjuuluglsuyrvkai3z7o62iddnpas/OpenFOAM-v1706.tgz
==> Created stage in /Users/ziolai/spack/var/spack/stage/openfoam-com-1706-qtosjuuluglsuyrvkai3z7o62iddnpas
==> Added file spack-Allwmake
==> Added file README-spack
==> Ran patch() for openfoam-com
==> Building openfoam-com [Package]
==> Executing phase: 'configure'
==> Executing phase: 'build'
==> Error: InstallError: No wmake rule for darwinIntel64Clang
InstallError: InstallError: No wmake rule for darwinIntel64Clang

/Users/ziolai/spack/var/spack/repos/builtin/packages/openfoam-com/package.py:631, in build:
2 its environment first.
3 Only build if the compiler is known to be supported.
4 """

5 self.foam_arch.has_rule(self.stage.source_path)
6 self.foam_arch.create_rules(self.stage.source_path, self)
7
8 args = ['-silent']

See build log for details:
/Users/ziolai/spack/var/spack/stage/openfoam-com-1706-qtosjuuluglsuyrvkai3z7o62iddnpas/OpenFOAM-v1706/spack-build.out

@davydden

Copy link
Copy Markdown
Member

Please create a new issue for this problem

@ziolai

ziolai commented Feb 25, 2018

Copy link
Copy Markdown

Thanks. I just did.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants