Adding package namd by nrichart · Pull Request #4321 · spack/spack

nrichart · 2017-05-23T12:59:57Z

This adds the package for namd. To download the source it uses a login password, but if I got it right anyone can ask for it for non-commercial uses.

I did not test all the possible installation, just gcc with and without tcl/python interfaces and fftw=3,
and intel with intel-mpi and fftw=mkl
In both case with charm backend=mpi

While testing this package for intel tools (compiler, mpi, mkl), I add to tweak the package charm and intel-mpi.

It should be only additions and corrections on the way the backends where checked. @eschnett can you check that I did not break anything in charm ?
For intel-mpi @davydden and @alalazo you are the one that wrote it.

davydden · 2017-05-23T13:17:21Z

var/spack/repos/builtin/packages/intel-mpi/package.py

        )

+    @property
+    def mpi_headers(self):


supposedly that's how one overrides spec['mpi'].headers which returns HeaderList object which by default looks for * headers and is recursive. Looking for mpi.h in include64 looks ok to me. Hopefully include64 always exists.

adamjstewart · 2017-05-23T14:51:23Z

var/spack/repos/builtin/packages/intel-mpi/package.py

+        # recurse from self.prefix will find too many things for all the
+        # supported sub-architectures like 'mic'
+        return find_headers(
+            'mpi', root='{0}64'.format(self.prefix.include), recurse=False)


Never seen an include64 before, but Intel does some weird things. If you want, you can add it to lib/spack/spack/util/prefix.py so that you can use self.prefix.include64.

If I do this I will have to check which mpi it is to now which include to use. This way it is transparent for the dependents to mpi. And is is the equivalent to what is done for mpi_libs
In fact <prefix>/include64 in this case is a symbolic link to <prefix>/linux64/include

Oh yes, I'm not suggesting you get rid of mpi_headers, just saying you could replace '{0}64'.format(self.prefix.include) with self.prefix.include64.

adamjstewart · 2017-05-23T14:55:39Z

var/spack/repos/builtin/packages/namd/package.py

+    high-performance simulation of large biomolecular systems."""
+
+    homepage = "http://www.ks.uiuc.edu/Research/namd/"
+    url      = "file://NAMD_2.12_Source.tar.gz"


Does this actually work? For most other packages I've used:

url = 'file://{0}/NAMD_2.12_Source.tar.gz'.format(os.getcwd())

This allows Spack to download the file if it is present in the current directory.

It does not find it and take it from the mirror. But I can change it so there is no need to add it first to a mirror.

adamjstewart · 2017-05-23T14:56:30Z

var/spack/repos/builtin/packages/namd/package.py

+    depends_on('fftw@3:', when="fftw=3")
+
+#    depends_on('fftw@3:', when="fftw=mkl")
+    depends_on('intel-mkl', when="fftw=mkl")


I wonder if we should make fftw a virtual provider.

I am not totally sure but I think that mkl as a different interface but when you install it you can compile the a part to have the proper interface.
And if we add provide('fftw') the package fftw should be renamed.

adamjstewart · 2017-05-23T14:58:27Z

var/spack/repos/builtin/packages/namd/package.py

+            description='Enable the use of FFTW/FFTW3/MKL FFT')
+
+    variant('interface', default='none', values=('none', 'tcl', 'python'),
+            description='Enables TCL and/or python interface')


I'm not sure if this should be a multi-valued variant or not. It wouldn't be hard to have +tcl and +python variants. Multi-valued variants are useful when there are too many possible values to create variants for or if the values are mutually exclusive.

I did it because they are kind of exclusive, according to the help of the build script +python enforce +tcl if it is 2 separated variant I have to also add a check because ~tcl+python is not valid.

adamjstewart · 2017-05-23T15:01:02Z

var/spack/repos/builtin/packages/namd/package.py

+
+    def build(self, spec, prefix):
+        with working_dir('{0}-spack'.format(self.arch)):
+            make()


If you override build_directory, you won't have to do this for every phase:

@property def build_directory(self): return '{0}-spack'.format(self.arch)

ifelsefi · 2017-05-27T03:32:36Z

Hell yes!!!!

Has anyone started VMD?

adamjstewart · 2017-05-31T15:55:33Z

var/spack/repos/builtin/packages/namd/package.py

+
+    def build(self, spec, prefix):
+        with working_dir(self.build_directory):
+            make()


I don't think you need to define build anymore as this is the default implementation.

I think @adamjstewart is right here.

alalazo

LGTM. Only a couple very minor comments.

alalazo · 2017-06-01T11:05:27Z

lib/spack/spack/util/prefix.py

+        s.sbin      = join_path(s, 'sbin')
+        s.etc       = join_path(s, 'etc')
+        s.include   = join_path(s, 'include')
+        s.include64 = join_path(s, 'include64')


OT: I wonder why in this file we have +12/-11 lines diff, while at a first glance it seems you added only one line...

I kept the alignment of spaces before the =

alalazo · 2017-06-01T11:09:07Z

var/spack/repos/builtin/packages/namd/package.py

+
+    def build(self, spec, prefix):
+        with working_dir(self.build_directory):
+            make()


I think @adamjstewart is right here.

nrichart · 2017-06-14T13:20:36Z

ping @adamjstewart

* Initial version of the namd package * Modified charm to consider compile against intel/intel-mpi * Correction of namd to compile with intel-mkl and intel compiler * Adding inclue64 in the prefix * adding property for the build directory * removing useless function build

nrichart added 3 commits May 23, 2017 14:44

Initial version of the namd package

b79cecc

Modified charm to consider compile against intel/intel-mpi

d2da329

Correction of namd to compile with intel-mkl and intel compiler

9c82c01

nrichart requested review from alalazo, davydden and eschnett May 23, 2017 12:59

davydden approved these changes May 23, 2017

View reviewed changes

adamjstewart added the new-package label May 23, 2017

adamjstewart reviewed May 23, 2017

View reviewed changes

nrichart added 2 commits May 24, 2017 01:16

Adding inclue64 in the prefix

c706b21

adding property for the build directory

995b45d

adamjstewart reviewed May 31, 2017

View reviewed changes

alalazo approved these changes Jun 1, 2017

View reviewed changes

removing useless function build

6d57ef0

adamjstewart approved these changes Jun 14, 2017

View reviewed changes

adamjstewart merged commit f06c23e into spack:develop Jun 14, 2017

adamjstewart mentioned this pull request Jun 23, 2017

Allow arbitrary Prefix attributes #4591

Merged

Conversation

nrichart commented May 23, 2017

Uh oh!

davydden May 23, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

ifelsefi commented May 27, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alalazo left a comment

Choose a reason for hiding this comment

Uh oh!

alalazo Jun 1, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

nrichart commented Jun 14, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

davydden May 23, 2017 •

edited

Loading

ifelsefi commented May 27, 2017 •

edited

Loading

alalazo Jun 1, 2017 •

edited

Loading