Albany: Add Albany package. by gahansen · Pull Request #8332 · spack/spack

gahansen · 2018-05-31T14:52:06Z

Add package.py to support the Albany GitHub project builds.
Update the Trilinos package.py script to provide additional
options needed for Albany.

davydden · 2018-05-31T14:53:24Z

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

+    maintainers = ['gahanse']
+
+    # Add proper versions and checksums here.
+    version('master', git='https://github.com/gahansen/Albany.git', branch='master')


please call the version develop, http://spack.readthedocs.io/en/latest/packaging_guide.html#version-comparison

I'll fix...

davydden · 2018-05-31T14:53:58Z

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

+    version('master', git='https://github.com/gahansen/Albany.git', branch='master')
+
+    variant('install',          default=True,
+            description='Enable Install')


what does this mean?

There is currently an explicit install option (on or off) in Albany's build process. I can eliminate this if I clean that option up on the Albany side. I'll do that and eliminate this option.

On further inspection, some Albany users need this option. I'll explicitly set it in the update instead of having it as a variant.

davydden · 2018-05-31T14:54:22Z

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

+            description='Enable LCM_SPECULATIVE')
+    variant('lame',          default=False,
+            description='Enable LAME')
+    variant('edebug',          default=False,


elsewhere we call it debug.

I'll fix...

davydden · 2018-05-31T14:55:23Z

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

+            description='Enable ALBANY_CI')
+    variant('ascr',          default=False,
+            description='Enable ALBANY_ASCR')
+    variant('perf',          default=False,


does this run some tests? If so, it's not a variant:

spack install -h --run-tests run package tests during installation (same as --test=all)

No - sorry for the confusion. Albany optionally links to a Configuration Interface (CI) that is a GUI that helps users setup the Albany input and execution spec. I could add a better description than "Enable ALBANY_CI" at the expense of making that string longer. Are there limitations on the length of that string (either technical or project policies?)

There are no limitations on the length of the description. However, if the line goes over 79 characters, flake8 will complain. As long as you keep the entire variant(...) on a single line, flake8 will not check that line.

davydden · 2018-05-31T14:56:19Z

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

+    # Add dependencies
+    depends_on('mpi')
+    depends_on('trilinos@develop:~superlu-dist+isorropia+tempus+rythmos+teko+intrepid+intrepid2+minitensor+phal+pnetcdf+nox+piro+rol+shards+stk+superlu')
+    depends_on('trilinos@develop:~superlu-dist+isorropia+tempus+rythmos+teko+intrepid+intrepid2+minitensor+phal+pnetcdf+nox+piro+rol+shards+stk+superlu', when='+lcm')


why do you need this one if it's the same as the previous line?

I'll fix...

adamjstewart · 2018-05-31T15:05:02Z

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

+       including fluid mechanics, solid mechanics (elasticity and plasticity),
+       ice-sheet flow, quantum device modeling, and many other applications."""
+
+    # Add a proper url for your package's homepage here.


You can remove this comment.

I'll fix...

adamjstewart · 2018-05-31T15:05:35Z

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

+    homepage = "http://gahansen.github.io/Albany"
+    url      = "https://github.com/gahansen/Albany/tarball/master"
+
+    maintainers = ['gahanse']


We usually use GitHub handles in the maintainers list so that we can ping people when there are build problems.

I'll fix...

adamjstewart · 2018-05-31T15:05:48Z

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

+
+    maintainers = ['gahanse']
+
+    # Add proper versions and checksums here.


You can delete this comment.

I'll fix...

adamjstewart · 2018-05-31T15:06:37Z

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

+
+        options.extend([
+            '-DALBANY_TRILINOS_DIR:FILEPATH={0}'.format(trilinos_dir),
+            '-DCMAKE_INSTALL_PREFIX:PATH=%s' % self.prefix


You don't need this one, Spack already adds it for you in CMakePackage.

I'll fix...

aprokop · 2018-05-31T15:09:20Z

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

            description='Enable Intrepid')
    variant('intrepid2',    default=False,
            description='Enable Intrepid2')
+    variant('minitensor',    default=False,


Please add packages in alphabetical order, this makes it easier to maintain.

aprokop · 2018-05-31T15:09:36Z

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

                'ON' if '+intrepid' in spec else 'OFF'),
            '-DTrilinos_ENABLE_Intrepid2=%s' % (
                'ON' if '+intrepid2' in spec else 'OFF'),
+            '-DTrilinos_ENABLE_Phalanx=%s' % (


Please use alphabetical order here.

aprokop · 2018-05-31T15:14:06Z

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

+            '-DTrilinos_ENABLE_Rythmos=%s' % (
+                'ON' if '+rythmos' in spec else 'OFF'),
+            '-DTrilinos_ENABLE_Teko=%s' % (
+                'ON' if '+teko' in spec else 'OFF'),


Please add checks like conflict for configurations you know are incompatible so that they fail in the spack resolution phase and not during Trilinos configure/build. For example, Teko has a required dependency on many packages including Tpetra, ML, etc. Thus, it would be nice to have things like

conflicts('+piro', when='~tpetra')

I understand that this would be time consuming, but I don't see a better solution (would be nice if we could somehow do parts of spack's trilinos package by parsing Trilinos Dependencies.cmake).

I'll add that dependency. TriBits has an effective dependency system in place, we will need to dig through Dependencies.cmake and duplicate the dependencies in the Spack file for the options that I added. Would you be open to helping me with that?

@gahansen Would be glad to help.

Thanks! Email is inbound...

aprokop · 2018-06-04T16:02:34Z

@gahansen I created a separate Trilinos update PR, that incorporates your changes and includes a bunch of checks for package depencies in #8363. Can you please rebase this PR on top of that and test it out?

EDIT: I also renamed the phal option from this PR to its full name phalanx.

gahansen · 2018-06-06T20:07:46Z

Commit ba7f3fa replaces the above. @aprokop very kindly adopted my suggested changes to the Trilinos package and submitted PR outlined in #8363 which provides the changes needed that the Albany package requires. This commit contains only the albany/package.py file that builds on that for the new Albany package. I believe I addressed the above comments in this commit. Thanks to everyone who helped with this!

aprokop

Looks good to me. The only minor nitpick is that depending on just trilinos@develop seems over restrictive to me. It should not be a problem compiling it with trilinos@master, for example, as they are now typically just few days off.

gahansen · 2018-06-06T20:23:27Z

Thanks for the thought @aprokop . I actually tried trilinos@master, but spack fell back to using [email protected] when I did that. I think there might be a priority in spack, with the "@develop" is highest, but the numeric version is preferred over other string identifiers. Or perhaps it was due to an oversight on my part?

adamjstewart · 2018-06-06T20:33:12Z

I think there might be a priority in spack, with the "@develop" is highest, but the numeric version is preferred over other string identifiers.

Correct. By default, Spack installs the newest numeric release for stability. If you want a different release, you have to explicitly request it.

aprokop · 2018-06-06T20:34:31Z

I think you can do something like depends_on('trilinos@master,develop~superlu...). Nalu seems to be doing something similar.

gahansen · 2018-06-06T23:16:28Z

Thanks @aprokop! Your syntax suggestion for the Trilinos dependency works great. I added 0f34beb to change this Albany dependency to your suggestion.

aprokop · 2018-06-07T01:57:49Z

Just one minor point of confusion for me. @adamjstewart Is @develop: same as @develop?

adamjstewart · 2018-06-07T05:06:12Z

Yes, develop sorts higher than any other version.

aprokop · 2018-06-07T12:10:58Z

@adamjstewart Thanks! @gahansen So, you could remove the : at the end of depends_on('trilinos...') line. Or you could leave it as it is. I have no further comments.

gahansen · 2018-06-15T21:40:01Z

@adamjstewart - I'm new to the Spack effort. Do I need to take further action to move forward to a merge?

adamjstewart · 2018-06-15T22:17:33Z

I think the only thing left is @aprokop's comment on the unnecessary colon.

* Add package.py to support the Albany GitHub project builds.

gahansen · 2018-06-18T16:25:57Z

Sounds great. I just pushed 77e92da that has the changes above squashed into it, along with the unnecessary colon removal suggested by @aprokop .

adamjstewart · 2018-06-18T17:02:44Z

Will merge as soon as @davydden approves.

gahansen mentioned this pull request May 31, 2018

Look into supporting an Albany instance in a container (Docker or Spack) sandialabs/Albany#298

Closed

davydden suggested changes May 31, 2018

View reviewed changes

davydden requested a review from aprokop May 31, 2018 14:57

adamjstewart added the new-package label May 31, 2018

adamjstewart requested changes May 31, 2018

View reviewed changes

aprokop suggested changes May 31, 2018

View reviewed changes

gahansen mentioned this pull request Jun 1, 2018

New behavior of installation sandialabs/Albany#303

Closed

aprokop approved these changes Jun 6, 2018

View reviewed changes

adamjstewart approved these changes Jun 8, 2018

View reviewed changes

Albany: Add Albany package.

77e92da

* Add package.py to support the Albany GitHub project builds.

davydden approved these changes Jun 18, 2018

View reviewed changes

adamjstewart merged commit d98d45e into spack:develop Jun 18, 2018

gahansen deleted the Albany_feature_addition branch June 18, 2018 19:28


		maintainers = ['gahanse']

		# Add proper versions and checksums here.

Conversation

gahansen commented May 31, 2018

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!

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!

aprokop commented Jun 4, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gahansen commented Jun 6, 2018

Uh oh!

aprokop left a comment

Choose a reason for hiding this comment

Uh oh!

gahansen commented Jun 6, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

adamjstewart commented Jun 6, 2018

Uh oh!

aprokop commented Jun 6, 2018

Uh oh!

gahansen commented Jun 6, 2018

Uh oh!

aprokop commented Jun 7, 2018

Uh oh!

adamjstewart commented Jun 7, 2018

Uh oh!

aprokop commented Jun 7, 2018

Uh oh!

aprokop commented Jun 4, 2018 •

edited

Loading

gahansen commented Jun 6, 2018 •

edited

Loading