Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't require develop version of trilinos-for-albany in albany #21

Merged

Conversation

xylar
Copy link
Collaborator

@xylar xylar commented Sep 29, 2023

This should allow us to use other tags of trilinos in the (near) future.

closes #20

@xylar xylar requested a review from ikalash September 29, 2023 19:23
@xylar
Copy link
Collaborator Author

xylar commented Sep 29, 2023

@ikalash, with this mess up your testing? Can you explicitly add trilinos-for-albany@develop as one of your spack package specs so it doesn't have to be hard-coded into the albany recipe?

@ikalash
Copy link

ikalash commented Sep 29, 2023

I'm confused by this PR. Wouldn't this just switch to use Trilinos master (the default branch) instead of Trilinos develop by default?

@xylar
Copy link
Collaborator Author

xylar commented Sep 30, 2023

It would let you specify what version of trilinos-for-albsny you want but would default to the latest release, just as spack does for all packages.

In your workflows, you would need to add trilinos-for-albsny@develop as a dependency along with albany@develop, and I would do trilinos-for-albsny@compass-2023-08-03 and albany@compass-2023-08-03.

@ikalash
Copy link

ikalash commented Sep 30, 2023

@xylar : I see. I think with your changes, when you ask spack to build Albany, it will use the master branch of Trilinos but I can double check. I agree that some of the dependencies in the .py files could be changed later on as you describe.

@xylar
Copy link
Collaborator Author

xylar commented Sep 30, 2023

I think with your changes, when you ask spack to build Albany, it will use the master branch of Trilinos but I can double check

It would be interesting to know but my understanding is that Spack strongly favors release versions over branches (or tags that don't conform to normal release numbering) so I would be very surprised if the default is the Trilinos master branch as opposed to the latest numbered release.

I agree that some of the dependencies in the .py files could be changed later on as you describe.

I was actually referring to your nightly testing and so forth. I would think that would now need a trilinos-with-albany@develop wherever you currently have an albany@develop, since it would default to a different Trilinos version.

@ikalash
Copy link

ikalash commented Oct 1, 2023

I was actually referring to your nightly testing and so forth. I would think that would now need a trilinos-with-albany@develop wherever you currently have an albany@develop, since it would default to a different Trilinos version.

I don't really follow this part. First we don't have a develop branch of Albany. Second, I don't actually build trilinos-with-albany@develop; here is the spack install line I use: spack --insecure install --dirty --keep-stage albany%[email protected]+mpas+py+optimization+mesh_depends_on_params. I would not want our users to build Trilinos separately using spack prior to Albany, if this is what you are suggesting. I think it defeats the purpose of spack and introduced complications, esp. for those who are not experts at building codes / HPC.

@xylar xylar force-pushed the drop-trilinos-develop-from-albany branch 2 times, most recently from db62417 to 08cf6ea Compare October 1, 2023 06:03
@xylar
Copy link
Collaborator Author

xylar commented Oct 1, 2023

@ikalash, that concern makes a lot of sense. How about if your users just need to do:

spack --insecure install --dirty --keep-stage albany@develop%[email protected]+mpas+py+optimization+mesh_depends_on_param

It may be that albany@develop is the default but it's hard for me to tell what spack will prioritize.

I updated the branch so this should work.

First we don't have a develop branch of Albany.

Sorry for the confusion, I didn't look carefully. I thought the develop version of albany would correspond to a develop branch like it does for Trilinos but I now see that that version points to the master branch of Albany.

depends_on("trilinos-for-albany~superlu-dist+exodus+chaco~isorropia+tempus+teko~intrepid+intrepid2+minitensor+phalanx+pnetcdf+nox+piro+shards+stk+amesos2~amesos~hypre+ifpack2~mumps~suite-sparse~epetra~ifpack~ml+muelu~aztec+superlu+rol+frosch gotype=long_long", when="~sandybridge~epetra+optimization")
depends_on("trilinos-for-albany~superlu-dist+exodus+chaco~isorropia+tempus+teko~intrepid+intrepid2+minitensor+phalanx+pnetcdf+nox+piro+shards+stk+amesos2~amesos~hypre+ifpack2~mumps~suite-sparse+sandybridge~epetra~ifpack~ml+muelu~aztec+superlu+rol+frosch gotype=long_long", when="+sandybridge~epetra+optimization")

depends_on("trilinos-for-albany@develop", when="@develop")
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This adds back the trilinos-for-albany@develop dependency but only for the albany@develop version. That would accommodate other albany versions that depend on other trilinos-for-albany versions, as in #22.

@ikalash
Copy link

ikalash commented Oct 1, 2023

Thanks for your patience with my questions and for making the changes! I’ll take a look tomorrow my time and try out your PR. There was something else broken in Albany causing the nightly Spack build to fail but it should be fixed tomorrow meaning it’ll be a good time to test the PR.

@xylar
Copy link
Collaborator Author

xylar commented Oct 1, 2023

Likewise, thank you for your patience! I've been trying to work for a long while toward having tags of Albany and Trilinos that MALI can build from, and we're getting close. The August 3rd tags I'm adding in #22 are almost what I need but there's still the issue in sandialabs/Albany#987 preventing me from using them. But we're getting close!

The broader context is that I was trying to fix an Compass issue unrelated to Albany that required me doing a new Spack deployment on all our supported machines. The fact that Albany's main branch is currently broken has prevented me from doing this new deployment, so that's why I have returned to pushing for working tags. @bartgol made tags for me, which I'm very grateful for, but it seems like Spack builds from tags are just enough different from builds from branches to cause sandialabs/Albany#987, so I'm still stuck until new tags get made. It's not a big deal. The original Compass issue has a workaround that my colleagues are using for now.

@ikalash
Copy link

ikalash commented Oct 1, 2023

@xylar : can you please merge the E3SM spack into your fork? I pushed a fix to it today that is needed to prevent compilation errors. Afterwards, I will test it.

With this merge, we only use `trilinos-for-albany@develop` with
`albany@develop`
@xylar xylar force-pushed the drop-trilinos-develop-from-albany branch from 08cf6ea to b269dd2 Compare October 2, 2023 07:21
@xylar
Copy link
Collaborator Author

xylar commented Oct 2, 2023

@ikalash, done. I rebased onto develop.

@ikalash
Copy link

ikalash commented Oct 2, 2023

Thank you! I will test it / approve (hopefully) today.

Copy link

@ikalash ikalash left a comment

Choose a reason for hiding this comment

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

I tested this and the build works as expected. The nightly test line will need to change to:

spack --insecure install --dirty --keep-stage albany@develop%[email protected]+mpas+py+optimization+mesh_depends_on_param

as will the instructions for building Albany using spack. We can update instructions on how to use different tags once we've gotten that capability finalized.

@xylar
Copy link
Collaborator Author

xylar commented Oct 3, 2023

Thanks very much @ikalash!

@xylar xylar merged commit 30b44ad into E3SM-Project:develop Oct 3, 2023
8 of 10 checks passed
@xylar xylar deleted the drop-trilinos-develop-from-albany branch October 3, 2023 10:34
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.

Need albany to depend on version of trilinos-for-albany other than develop
2 participants