Add an easyblock for Siesta. by akesandgren · Pull Request #1105 · easybuilders/easybuild-easyblocks

akesandgren · 2017-02-13T12:45:44Z

This is an easyblock for Siesta 4.0 and later

akesandgren · 2017-03-06T09:27:42Z

@boegel If you could get this one into 3.1.1 i'd appreciate it.

Hmm, just noticed that there is another easyblock for siesta in progress...

boegel · 2017-03-06T10:28:19Z

@akesandgren since I intended to release EB v3.1.1 last Friday already, this won't make it in anymore, I'm sorry.

Someone should review & test this; do you have any easyconfig files for this too?

Moreover, as you point out, this and #923 should be merged somehow... Maybe @migueldiascosta is up for taking a look at your pull request, and seeing whether both approaches align?

migueldiascosta · 2017-03-07T03:52:59Z

@boegel @akesandgren the problem with this (and my #923) is that it will only work with 4.0 versions - 4.1-b1 broke the configure script (although it's trivial to fix it in the easyblock) and 4.1-b2 removed it completely - from upstream, "We have discontinued support for the configure script due to missing updates and dependency checks. Hence, later revisions have removed configure" (to be fair, they have always advocated tweaking the arch.make file instead of configure).

From the manual of 4.1-b2, "You are strongly encouraged to look at Obj/DOCUMENTED-TEMPLATE.make for information about the fine points of the arch.make file. There are two sample make files for compilation of SIESTA with gfortran and ifort named Obj/gfortran.make and Obj/intel.make, respectively. Please use those as guidelines for creating the final arch.make."

For version 4.1-b2, I used this. As mentioned in #923, I had decided to wait for a stable version before changing that PR, but it hasn't been released yet.

So, I guess the first question is, do we want to support the versions where a configure script still exists and works? Since Siesta hasn't been supported so far, we could simply start with 4.1-b2...

The second question is, regardless of the first, what's the best way of supporting the new versions without the configure script? I don't really like all those sed commands in my easyblock...

akesandgren · 2017-03-07T06:53:20Z

Ohh..... that's not something i have looked at. Since we have Siesta 4.0 installled there is no reason for us to rush this. I won't have time to look at next ver of siesta for quite a while. (Still lots of sw to install and new HW to experiment with). So from my point of view, waiting for an official 4.1 is early enough before starting to look at how to build it.

boegel · 2017-03-07T08:07:55Z

@migueldiascosta Is the arch.make procedure also supported for versions earlier than 4.1? If so, why not use that approach for all versions?

We have a dedicated function to do "runtime patching", so you don't have to use sed, i.e. apply_regex_substitutions, see for example https://github.com/hpcugent/easybuild-easyblocks/blob/master/easybuild/easyblocks/b/binutils.py#L88

migueldiascosta · 2017-03-07T09:30:42Z

@boegel we would still need to patch different example arch.make files depending on the version (hopefully the new ones won't change much...), but yes, we could

regarding apply_regex_substitutions, of course, there had to be a better way (and now I think you had already mentioned this to me and I forgot)

migueldiascosta · 2017-03-24T06:05:20Z

@boegel in the older versions configure needs to be run anyway, even if arch.make is then adjusted/replaced, so might as well keep it

@akesandgren something like this should make your easyblock also work for the newer versions without configure

start_dir = self.cfg['start_dir']
obj_dir = os.path.join(start_dir, 'Obj')
arch_make = os.path.join(obj_dir, 'arch.make')

netcdff_loc = get_software_root('NetCDF-Fortran')

regex_subs = [
    ('dc_lapack.a', ''),
    ('libsiestaBLAS.a', ''),
    ('libsiestaLAPACK.a', ''),
]

if LooseVersion(self.version) < LooseVersion("4.1-b2"):

    # MPI?
    if self.toolchain.options.get('usempi', None):
        self.cfg.update('configopts', '--enable-mpi')

    # BLAS and LAPACK
    self.cfg.update('configopts', '--with-blas="$LIBBLAS"')
    self.cfg.update('configopts', '--with-lapack="$LIBLAPACK"')

    # ScaLAPACK (and BLACS)
    self.cfg.update('configopts', '--with-scalapack="$LIBSCALAPACK"')
    self.cfg.update('configopts', '--with-blacs="$LIBSCALAPACK"')

    # NetCDF-Fortran
    if netcdff_loc:
        self.cfg.update('configopts', '--with-netcdf=-lnetcdff')

    super(EB_Siesta, self).configure_step(cmd_prefix='../Src/')

    apply_regex_substitutions(arch_make, regex_subs)

else:  # there's no configure on newer versions

    if self.toolchain.comp_family() in [toolchain.INTELCOMP]:
        copy_file(os.path.join(obj_dir, 'intel.make'), arch_make)
    else:
        copy_file(os.path.join(obj_dir, 'gfortran.make'), arch_make)

    if self.toolchain.options.get('usempi', None):
        regex_subs.extend([
            (r"^(CC\s*=\s*).*$", r"\1%s" % os.getenv('MPICC')),
            (r"^(FC\s*=\s*).*$", r"\1%s" % os.getenv('MPIF90')),
            (r"^(FPPFLAGS\s*=.*)$", r"\1 -DMPI"),
            (r"^(FPPFLAGS\s*=.*)$", r"\1\nMPI_INTERFACE = libmpi_f90.a\nMPI_INCLUDE = ."),
        ])
        complibs = os.environ['LIBSCALAPACK']
    else:
        complibs = os.environ['LIBLAPACK']

    regex_subs.extend([
        (r"^(LIBS\s*=\s).*$", r"\1 %s" % complibs),
    ])

    if netcdff_loc:
        regex_subs.extend([
            (r"^(LIBS\s*=.*)$", r"\1 -lnetcdff"),
            (r"^(FPPFLAGS\s*=.*)$", r"\1 -DCDF"),
        ])

    apply_regex_substitutions(arch_make, regex_subs)

akesandgren · 2017-03-24T08:06:29Z

I'll come up with a new easyblock for this that covers all the bases. Might take a day or so due to other stuff...

…lock

Adding handling of openmp and some fixes for broken Util Makefiles.

akesandgren · 2017-03-25T16:14:38Z

@migueldiascosta How does this look?

migueldiascosta

@akesandgren looks better and more complete than any of the many different versions I had :)

migueldiascosta · 2017-03-26T06:13:20Z

+
+            # ScaLAPACK (and BLACS)
+            self.cfg.update('configopts', '--with-scalapack="%s"' % scalapack)
+            self.cfg.update('configopts', '--with-blacs="$%s"' % blacs)


before I found this typo ($%s) I was starting to doubt my sanity :p

migueldiascosta · 2017-03-26T06:14:22Z

+            adjust_permissions('./clean_all.sh', stat.S_IXUSR, recursive=False, relative=True)
+            run_cmd('./clean_all.sh', log_all=True, simple=True, log_output=True)
+
+            if LooseVersion(self.version) >= LooseVersion("4.0"):


migueldiascosta · 2017-03-26T07:03:21Z

+                self.cfg.update('configopts', '--with-netcdf=-lnetcdff')
+
+            super(EB_Siesta, self).configure_step(cmd_prefix='../Src/')
+


for versions 4.0-b1 and 4.0-b2, I also had to patch the resulting Makefile for a missing space in $(CFLAGS)-c

that's weird...

maybe include a comment on that above the lines below to make that clear?

@akesandgren having to patch the $(CFLAGS)-c to include the missing space is weird

migueldiascosta · 2017-03-26T07:06:07Z

+                ]
+
+                apply_regex_substitutions(os.path.join(self.cfg['start_dir'], 'Util', 'TS', 'tshs2tshs', 'Makefile'), regex_subs_TS)
+


also for versions >= 4.1, I also had to patch the CPP line in Src/fdict/Makefile for the Utils to build

migueldiascosta · 2017-03-26T07:20:57Z

+            'dirs': [],
+        }
+
+        super(EB_Siesta, self).sanity_check_step(custom_paths=custom_paths)


with the above changes, it works for me (tested all versions since it was GPL'd, 4.0-b2, 4.0-b1, 4.0, 4.1-b1 and 4.1-b2)

yesterday I had only tested with an intel toolchain, today checked with foss

this is what I changed to get all versions to work on both toolchains

migueldiascosta · 2017-03-26T07:32:13Z

+            ]
+            apply_regex_substitutions(os.path.join(self.cfg['start_dir'], 'Util', 'Optimizer', 'Makefile'), regex_subs_UtilLDFLAGS)
+            apply_regex_substitutions(os.path.join(self.cfg['start_dir'], 'Util', 'JobList', 'Src', 'Makefile'), regex_subs_UtilLDFLAGS)
+


Util/STM/ol-stm/Src/Makefile will use the system's fftw or fail if not available, patched to use FFTW_LIB_DIR and LIBFFT

migueldiascosta · 2017-03-26T08:10:06Z

+                    'SiestaSubroutine/ProtoNEB/Src/protoNEB',
+                    'SiestaSubroutine/SimpleTest/Src/simple_pipes_parallel',
+                    'SiestaSubroutine/SimpleTest/Src/simple_pipes_serial',
+                    'Sockets/f2fmaster', 'Sockets/f2fslave',


SimpleTest and Sockets fail in versions prior to 4.1-b2 due to missing rule for sockets.o - I moved .SUFFIXES to after the include line in order for it to use the implicit rule (.SUFFIXES in arch.make doesn't include .c files)

Add fftw libs only if imkl or FFTW is loaded. Adjust netcdf for gfortran. Fix typo for blacs. Add some more fixes of broken Makefiles.

akesandgren · 2017-04-03T07:05:19Z

This last set of changes fixes everything @migueldiascosta has mentioned (except the Src/fdict/Makefile since that isn't needed as far as i can see).

It builds 4.0-b1/b2, 4.0, 4.1-b1/b2 with foss/2017a and intel/2017a.
Does parallel and openmp builds for 4.1 versions, and a bunch och other minor fixes.

It can probably be made a bit cleaner, (esp if one only cares about full release versions) but i think it is good to go.

I'll update the corresponding easyconfig file for Siesta too.

migueldiascosta · 2017-04-03T09:23:11Z

@akesandgren you're right, everything's working now. Thanks!

akesandgren · 2017-04-06T07:10:17Z

@boegel Care to merge this?

boegel · 2017-04-06T07:29:54Z

+"""
+EasyBuild support for building and installing Siesta, implemented as an easyblock
+
+@author: Ake Sandgren (Umea University)


missing @migueldiascosta?

@migueldiascosta Text to use for you will be?

not really necessary but if you want to, "Miguel Dias Costa (National University of Singapore)"

boegel · 2017-04-06T07:29:57Z

@@ -0,0 +1,342 @@
+##
+# Copyright 2009-2016 Ghent University


boegel · 2017-04-06T07:30:13Z

+@author: Ake Sandgren (Umea University)
+"""
+import os
+import re


boegel · 2017-04-06T07:30:26Z

+        run_cmd('make %s' % par, log_all=True, simple=True, log_output=True)
+
+        # Put binary in temporary install dir
+        shutil.copy(os.path.join(self.cfg['start_dir'], 'Obj', 'siesta'),


please use copy_file from filetools

boegel · 2017-04-06T07:30:42Z

+                ])
+
+            for f in expected_utils:
+                shutil.copy(os.path.join(self.cfg['start_dir'], 'Util', f), bindir)


use copy_files from filetools?

boegel · 2017-04-06T07:40:51Z

+                    (r"^(INCFLAGS.*)$", r"\1 -I%s" % obj_dir),
+                ]
+
+                apply_regex_substitutions(os.path.join(self.cfg['start_dir'], 'Util', 'TS', 'tshs2tshs', 'Makefile'), regex_subs_TS)


line too long

makefile = os.path.join(self.cfg['start_dir'], 'Util', 'TS', 'tshs2tshs', 'Makefile') apply_regex_substitutions(makefile, regex_subs_TS)

@akesandgren line still too long?

boegel · 2017-04-06T07:41:20Z

+                (r'^(include\s*\$\(ARCH_MAKE\).*)$', r'\1\n.SUFFIXES:\n.SUFFIXES: .c .f .F .o .a .f90 .F90'),
+            ]
+            apply_regex_substitutions(os.path.join(self.cfg['start_dir'], 'Util', 'Sockets', 'Makefile'), regex_subs_suffix)
+            apply_regex_substitutions(os.path.join(self.cfg['start_dir'], 'Util', 'SiestaSubroutine', 'SimpleTest', 'Src', 'Makefile'), regex_subs_suffix)


line too long...

boegel · 2017-04-06T07:41:26Z

+                (r'(\$\(FC\)\s*-o\s)', r'$(FC) %s %s -o ' % (os.environ['FCFLAGS'], os.environ['LDFLAGS'])),
+            ]
+            apply_regex_substitutions(os.path.join(self.cfg['start_dir'], 'Util', 'Optimizer', 'Makefile'), regex_subs_UtilLDFLAGS)
+            apply_regex_substitutions(os.path.join(self.cfg['start_dir'], 'Util', 'JobList', 'Src', 'Makefile'), regex_subs_UtilLDFLAGS)


lines too long...

boegel · 2017-04-06T07:41:58Z

+            run_cmd('./build_all.sh', log_all=True, simple=True, log_output=True)
+
+            # Now move all the built utils to the temp installdir
+            expected_utils = [


any risk of this list changing over times?

can we come up with a smarter way to copy all utils?

It WILL change over time, but figuring out what is actually built needs to be done by hand i think

more lazier than smarter, I've simply used buildininstalldir=True instead, also because there are useful scripts and examples in the Utils folders

cleaner would be to copy the whole tree but without source and object files, and create links to the binaries in the bin folder

but none of this solves the need for a list of expected utils to sanity check - the build_all.sh iterates over all subdirs that have a makefile, we could get the default targets from each of them (?)

well, an explicit sanity check doesn't hurt, it'll just be a PITA to maintain

but we can adjust accordingly later if it really becomes a problem

ah, wait, this isn't the sanity check...

well, it'll fail anyway when we try to copy files that aren't there

boegel · 2017-04-06T07:42:25Z

+        if self.cfg['with_transiesta']:
+            # Build transiesta
+            try:
+                os.chdir(obj_dir)


use change_dir

Fixed lots of comments from Kenneth. Removed all tabs.

boegel · 2017-04-06T15:11:43Z

+
+        # binary
+        bindir = os.path.join(self.installdir, 'bin')
+        copytree(os.path.join(self.cfg['start_dir'], 'bin'), bindir)


@akesandgren we deprecated copytree in develop for EasyBuild v3.2.0, in favour of copy_dir

boegel · 2017-04-06T15:12:45Z

+            # Needed for gfortran at least
+            regex_subs.extend([
+                (r"^(ARFLAGS_EXTRA\s*=.*)$", r"\1\nNETCDF_INCFLAGS = -I%s/include" % netcdff_loc),
+            ])


just use append to add a single item

regex_subs.append((r"^(ARFLAGS_EXTRA\s*=.*)$", r"\1\nNETCDF_INCFLAGS = -I%s/include" % netcdff_loc))

boegel · 2017-04-06T15:13:09Z

+
+        if fftw:
+            fftw_inc, fftw_lib = os.environ['FFTW_INC_DIR'], os.environ['FFTW_LIB_DIR']
+            regex_subs.extend([


same here, just use .append?

boegel · 2017-04-06T15:16:13Z

+                    'TS/ts2ts/ts2ts', 'TS/tshs2tshs/tshs2tshs', 'TS/TBtrans/tbtrans',
+                ])
+
+            for f in expected_utils:


@akesandgren don't use single-letter variable names outside of list comprehensions please, for util in expected_utils:

akesandgren · 2017-05-08T07:38:19Z

Hmm, time to start finishing this ...

What did you think was weird above? Comment Apr 6th "maybe include a comment on that above the lines below to make that clear"

boegel · 2017-05-08T07:44:08Z

@akesandgren having to patch $(CFLAGS)-c, see https://github.com/hpcugent/easybuild-easyblocks/pull/1105/files#r108053518

migueldiascosta · 2017-05-09T00:58:11Z

@boegel, @akesandgren regarding the $(CFLAGS)-c problem, I guess they didn't notice because their CFLAGS, from their configure or the template arch.make, must have been empty, but they did fix it as soon as I mentioned it to them: https://bugs.launchpad.net/siesta/+bug/1582978

boegel · 2017-05-09T12:54:13Z

+            regex_subs.append((r"^(ARFLAGS_EXTRA\s*=.*)$", r"\1\nNETCDF_INCFLAGS = -I%s/include" % netcdff_loc))
+
+        if fftw:
+            fftw_inc, fftw_lib = os.environ['FFTW_INC_DIR'], os.environ['FFTW_LIB_DIR']


You should be using FFT_INC_DIR and FFT_LIB_DIR here, which are also defined if imkl is being used

This was fixed in akesandgren#4, together with a bunch of minor style fixes.

fix specifying FFT(W) dirs + minor style fixes

boegel · 2017-05-09T18:31:53Z

Thanks for all the hard work on this @akesandgren and @migueldiascosta!

Tested with easybuilders/easybuild-easyconfigs#2965, looking good, so going in!

If you have any other Siesta easyconfigs (different versions/toolchains), please open a PR for them.

Add an easyblock for Siesta.

c9fe8a7

boegel added this to the 3.2.0 milestone Mar 6, 2017

akesandgren added 2 commits March 25, 2017 17:07

Merge remote-tracking branch 'upstream/develop' into add-siesta-easyb…

175cbdd

…lock

Adjust siesta.py to handle versions from 3.x to 4.1-b2 an onwards.

c7e5793

Adding handling of openmp and some fixes for broken Util Makefiles.

migueldiascosta requested changes Mar 26, 2017

View reviewed changes

Add parallel building for 4.1 and later only.

2c31444

Add fftw libs only if imkl or FFTW is loaded. Adjust netcdf for gfortran. Fix typo for blacs. Add some more fixes of broken Makefiles.

boegel requested changes Apr 6, 2017

View reviewed changes

Added Miguel as author.

5f8a309

Fixed lots of comments from Kenneth. Removed all tabs.

boegel reviewed Apr 6, 2017

View reviewed changes

boegel modified the milestones: 3.2.0, 3.3.0 May 2, 2017

Fix some more comments from Kenneth.

45fbba5

fix specifying FFT(W) dirs + minor style fixes

8657b42

boegel mentioned this pull request May 9, 2017

fix specifying FFT(W) dirs + minor style fixes akesandgren/easybuild-easyblocks#4

Merged

boegel reviewed May 9, 2017

View reviewed changes

Merge pull request #4 from boegel/add-siesta-easyblock

7a23026

fix specifying FFT(W) dirs + minor style fixes

boegel approved these changes May 9, 2017

View reviewed changes

boegel mentioned this pull request May 9, 2017

add easyblock for siesta (WIP) #923

Closed

boegel modified the milestones: 3.2.1, 3.3.0 May 9, 2017

boegel mentioned this pull request May 9, 2017

{phys}[intel/2016.02-GCC-4.9] Siesta 4.0 easybuilders/easybuild-easyconfigs#2965

Closed

boegel merged commit 79e00ce into easybuilders:develop May 9, 2017

akesandgren deleted the add-siesta-easyblock branch May 9, 2017 20:17

		self.cfg.update('configopts', '--with-netcdf=-lnetcdff')

		super(EB_Siesta, self).configure_step(cmd_prefix='../Src/')

		]

		apply_regex_substitutions(os.path.join(self.cfg['start_dir'], 'Util', 'TS', 'tshs2tshs', 'Makefile'), regex_subs_TS)

Conversation

akesandgren commented Feb 13, 2017

Uh oh!

akesandgren commented Mar 6, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

boegel commented Mar 6, 2017

Uh oh!

migueldiascosta commented Mar 7, 2017

Uh oh!

akesandgren commented Mar 7, 2017

Uh oh!

boegel commented Mar 7, 2017

Uh oh!

migueldiascosta commented Mar 7, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

migueldiascosta commented Mar 24, 2017

Uh oh!

akesandgren commented Mar 24, 2017

Uh oh!

akesandgren commented Mar 25, 2017

Uh oh!

migueldiascosta left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

akesandgren commented Apr 3, 2017

Uh oh!

migueldiascosta commented Apr 3, 2017

Uh oh!

akesandgren commented Apr 6, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

akesandgren commented Mar 6, 2017 •

edited

Loading

migueldiascosta commented Mar 7, 2017 •

edited

Loading

migueldiascosta commented May 9, 2017 •

edited

Loading