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

WIP: Update MAPL and other GEOS-ESM packages #174

Conversation

mathomp4
Copy link
Collaborator

@mathomp4 mathomp4 commented Sep 19, 2022

This is an update to the MAPL and other GEOS-ESM packages in NOAA-EMC/spack.

The biggest changes would be in MAPL. I removed the ESMA_GFE_NAMESPACE option as it is nigh-ancient and I'm not sure any version here needs it (might be wrong, though.) I also added dependencies for yafyaml when building with ExtData2G support. There was an interface change at MAPL 2.23 so you need a newer yafyaml. (This also triggers a split in pflogger.)

cc: @climbfuji @tclune


Changed:

  • mapl
  • esmf (slight bug fix)
  • FMS (need a new variant)

Additions:

  • flap (A weird one. flap is also in canonical spack, but we use a fork of it. This can probably never go back to mainline spack. I've hesitated pushing our changes to the upstream flap repo as MAPL eventually will move to fargparse or, most likely, move away from command line arguments altogether and go to YAML.)
  • geosgcm (testing at the moment)
  • mepo (for geosgcm, though maybe it should go away in favor of tarfiles?)

Testing:

  • Building:
    • Does it build in spack-stack?
  • UFS
    • Do the libraries it builds, build with MAPL-in-UFS?
    • Does the MAPL in this PR work with UFS?
    • Can it run?
  • GEOSgcm
    • Internal MAPL and Internal FMS
      • Does it build GEOSgcm?
      • Does GEOSgcm run?
    • Internal MAPL and Spack FMS
      • Does it build GEOSgcm?
      • Does GEOSgcm run?
    • Spack MAPL and Internal FMS
      • Does it build GEOSgcm?
      • Does GEOSgcm run?
    • Spack MAPL and Spack FMS
      • Does it build GEOSgcm?
      • Does GEOSgcm run?

@mathomp4
Copy link
Collaborator Author

A few updates to remind myself. I have gotten, I think, a good set of libraries. I did have to do a few things.

In the common/packages.yaml file for my empty environment I added:

     pflogger:
       variants: +mpi
     pfunit:
       variants: +mpi+fhamcrest

to lock in some variants we want.

I also really only want ESMF 8.3.0 but I just could not get spack to build it. The only way was to comment out the 8.3.0b09 line here:

https://github.com/NOAA-EMC/spack/blob/1248dfd679e9ee8095222b34b0a71217c52c272c/var/spack/repos/builtin/packages/esmf/package.py#L24-L34

because spack seemed to think 8.3.0b09 was newer than 8.3.0. Maybe @climbfuji knows a better way, but I'm at the "throw things at the wall" stage 😄

But I was able to build:

spack install [email protected]+extdata2g+flap+pflogger

I'm currently trying to build GEOSgcm...but not with spack MAPL, but instead an internal MAPL. I'm also going to try both a internal-FMS as well as an FMS-from-spack. But to do that, I'll need to slightly fix up the FMS here in this repo, I think. I'll report back!

@tclune
Copy link

tclune commented Sep 21, 2022

A few updates to remind myself. I have gotten, I think, a good set of libraries. I did have to do a few things.

In the common/packages.yaml file for my empty environment I added:

     pflogger:
       variants: +mpi
     pfunit:
       variants: +mpi+fhamcrest

I wonder if pfunit should have an openmp variant as well?

@mathomp4
Copy link
Collaborator Author

mathomp4 commented Sep 21, 2022

A few updates to remind myself. I have gotten, I think, a good set of libraries. I did have to do a few things.
In the common/packages.yaml file for my empty environment I added:

     pflogger:
       variants: +mpi
     pfunit:
       variants: +mpi+fhamcrest

I wonder if pfunit should have an openmp variant as well?

It does have one:

    variant("openmp", default=False, description="Enable OpenMP")

I just didn't turn it on by default. We might want to? I just figure I usually build it with --DSKIP_OPENMP=ON and...well crap. Now I think the package doesn't handle that correctly. Let me take a look...

ETA: Ah it does:

        if spec.satisfies("@4.0.0:"):
            args.append("-DSKIP_MPI=%s" % ("YES" if "~mpi" in spec else "NO"))
            args.append("-DSKIP_OPENMP=%s" % ("YES" if "~openmp" in spec else "NO"))
            args.append("-DSKIP_FHAMCREST=%s" % ("YES" if "~fhamcrest" in spec else "NO"))
            args.append("-DSKIP_ESMF=%s" % ("YES" if "~esmf" in spec else "NO"))
        else:
            args.append(self.define_from_variant("MPI", "mpi"))
            args.append(self.define_from_variant("OPENMP", "openmp"))

I was just looking in the wrong place.

@tclune
Copy link

tclune commented Sep 21, 2022

@mathomp4 OK - glad you are on it. Agree that we want it off for now; I was just keying off the absence. So ignore what I wrote.

@mathomp4
Copy link
Collaborator Author

mathomp4 commented Feb 27, 2023

@climbfuji So, I updated the files here and I was able to install MAPL. So, yay. I then wanted to test building geosgcm but I saw I needed a newer ESMF. Fine:

$ spack install [email protected]
==> Error: Package 'mapl' not found.
You may need to run 'spack clean -m'.

Huh. I'm new at all this stack/environment stuff. Any idea what I might have done wrong for spack to say this?

ETA: Oh. wait:

$ spack find esmf
==> In environment /gpfsm/dnb44/mathomp4/spack-stack/envs/geosgcm.discover
==> Root specs
geosgcm  mapl@spacktest  [email protected] +extdata2g+fargparse+flap+pflogger+pfunit  [email protected]  mapl@develop  mepo  py-pyyaml

==> Installed packages
-- linux-sles12-skylake_avx512 / [email protected] -----------------
[email protected]
==> 1 installed package

Already installed. Maybe I need to do some weird concretize things?

@climbfuji
Copy link
Collaborator

climbfuji commented Feb 27, 2023 via email

@mathomp4
Copy link
Collaborator Author

spack clean -a doesn’t help?

Nope. But I'm going to try a few things. Worse comes to worst, I just reclone!

@climbfuji
Copy link
Collaborator

spack clean -a doesn’t help?

Nope. But I'm going to try a few things. Worse comes to worst, I just reclone!

If the spack version is too different, you may have to re-bootstrap from a fresh clone

@mathomp4
Copy link
Collaborator Author

Well, I think my environment was just not good. I did a:

spack concretize -Uf

as that often fixes things and things changed. A lot. I think I just need to start afresh.

@mathomp4
Copy link
Collaborator Author

mathomp4 commented Mar 7, 2023

@climbfuji I am still working on this and found a bug in my (every?) MAPL version if you try and build +pfunit. I think I have a fix, but the fun part is testing the fix. I added a new branch for MAPL, and the weird thing is, spack keeps wanting to patch it. But in my MAPL package.py I've commented out every patch command I see...and it's still trying to patch!

==> Installing mapl-spack-eprnncn4hdrgc6bllawpudthctbt7qhe
==> mapl exists in binary cache but with different hash
==> No binary for mapl-spack-eprnncn4hdrgc6bllawpudthctbt7qhe found: installing from source
==> Error: NoSuchPatchError: Couldn't find patch for package builtin.mapl with sha256: 7b9c28b9343403b0d1d8e58478982e575b839a811db3c95f0b3987dcfd841287

/gpfsm/dnb44/mathomp4/spack-stack/spack/lib/spack/spack/package_base.py:1456, in do_fetch:
       1453            self.stage.check()
       1454
       1455        self.stage.cache_local()
  >>   1456
       1457        for patch in self.spec.patches:
       1458            patch.fetch()
       1459            if patch.stage:

Any idea where this might be coming from?

@climbfuji
Copy link
Collaborator

climbfuji commented Mar 7, 2023 via email

@mathomp4
Copy link
Collaborator Author

mathomp4 commented Mar 7, 2023

You need to re-concretize for spack to see changes regarding patches

Ahhh. Okay. I keep forgetting about that! On to other fun not using a version number!

@mathomp4
Copy link
Collaborator Author

mathomp4 commented Mar 9, 2023

@AlexanderRichert-NOAA I think I finally have the MAPL package here in good shape. If you have a chance, can you try it out with your code? If it works, I can undraft this PR and even add it to spack mainline for you.

@AlexanderRichert-NOAA
Copy link
Collaborator

Awesome, I'll test it out ASAP.

@mathomp4
Copy link
Collaborator Author

mathomp4 commented Mar 9, 2023

A note to @climbfuji. The pfunit here will of course be superseded by the newer better one in current develop spack. Same for fms. I've only added FMS here because this doesn't have the new "precision" one.

I'm debating if mepo, geosgcm and flap need to be here or not. I'm keeping them here just in case. Eventually FLAP use in GEOS is going away, and I'm pretty sure my GEOSgcm package doesn't need mepo anymore.

@@ -59,6 +59,11 @@ class Fms(CMakePackage):
git = "https://github.com/JCSDA/fms.git"
# *DH 20220602

variant(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Instead of this we should pull in the updates from https://github.com/spack/spack/blob/develop/var/spack/repos/builtin/packages/fms/package.py (can be a separate PR so you don't have to deal with it)

Copy link
Collaborator

Choose a reason for hiding this comment

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

I can do this if you like

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@climbfuji If you can, please. I have a PR in to spack mainline to update some other GFE packages.

It's also possible we might want to wait for the ESMF PR from @theurich before pulling in spack? Not sure.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Also, I will say I tried using the newer spack maintainers() function in this PR but spack wasn't happy. It's possible my spack was old?

Copy link
Collaborator

Choose a reason for hiding this comment

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

We haven't yet pulled that functionality from main spack, so with NOAA-EMC spack it will yell at you about the function being undefined :)

# but we don't want to do this in general due to the speed of MAPL development

# When we move to FMS as library, we'll need to add this:
#depends_on("[email protected]:~gfs_phys+fpic~quad_precision+32bit+64bit+yaml constants=GEOS")
Copy link
Collaborator

Choose a reason for hiding this comment

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

When the time comes, we'll need a way to describe the different variants in the module names (not now!), e.g. fms/2022.04-geos based on the constants value).

@AlexanderRichert-NOAA
Copy link
Collaborator

This worked for me on hera with UFS (ea0b6e4) using ufs-weather-model-static. The only change needed was setting fms +32bit.

@mathomp4
Copy link
Collaborator Author

mathomp4 commented Mar 9, 2023

This worked for me on hera with UFS (ea0b6e4) using ufs-weather-model-static. The only change needed was setting fms +32bit.

Ooh. Nice!

@mathomp4
Copy link
Collaborator Author

Closing. Will make a new PR with geosgcm (and maybe mepo and flap) when #241 is pulled in and tested.

@mathomp4 mathomp4 closed this May 12, 2023
@mathomp4 mathomp4 deleted the feature/mathomp4/update-add-gfe-mapl branch May 12, 2023 14:31
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.

4 participants