Add/fix build capability for Gaea-C5, Gaea-C6, and container #87

DavidBurrows-NCO · 2024-10-24T20:15:19Z

Description

After the recent Gaea-C5 OS upgrade, gfs_utils fails to build.
This corrects Gaea-C5 build, adds Gaea-C6 build capability (following ufs-wx-model 2448), and adds containerized build capability.

Refs NOAA-EMC/global-workflow 3011
Refs NOAA-EMC/global-workflow 3025
Resolve #86

Type of change

Bug fix (fixes something broken)
New feature (adds functionality)

Change characteristics

Is this a breaking change (a change in existing functionality)? NO
Does this change require a documentation update? NO

How has this been tested?

Cloned and built on Gaea-C5, Gaea-C6, and container

Checklist

Any dependent changes have been merged and published
My code follows the style guidelines of this project
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
My changes generate no new warnings
New and existing tests pass with my changes
I have made corresponding changes to the documentation if necessary

…fs-utils into feature/c5c6conbuild

ush/module-setup.sh

WalterKolczynski-NOAA · 2024-11-12T16:16:57Z

CMakeLists.txt

@@ -25,6 +25,7 @@ endif()

 # User options.
 option(OPENMP "use OpenMP threading" ON)
+option(LINK_W3EMC "Link w3emc library" ON)


Not sure the purpose of this new option. Shouldn't this build in the same manner on all platforms? Have the other platforms been tested with this change?

This change is required for building with spack-stack-1.8.0. In spack-stack-1.8.0, the skgb function has been added to the g2 library and will throw an error for multiple definitions (it is also in w3emc). Once gfs_utils gets updated to spack-stack-1.8.0 everywhere, the option can be removed.

aerorahul · 2024-11-13T14:44:36Z

modulefiles/gfsutils_generic.lua

@@ -0,0 +1,22 @@
+help([[


What is the purpose of this module file?

It is a version-less replacement for gfsutils_common.lua that is used when building with a container. We can add in the version numbers, if you want, but leaving them out allows more flexibility to update the container without having to update module files in all repos. I understand that having specific version numbers is desirable for testing and reproducibility, but allowing flexibility in them for the container case improves portability and extensibility.

I understand it improves portability and extensibility, but they come at the cost of reproducibility, reliability, and maintainability. Is there a way to have minimum versions in lua modulefiles? In cmake one can provide a minimum version.
The gfsutils_common.lua provides a mechanism for the user to provide a specific version. See here. If the container has a different version, it can set those version numbers and still be able to leverage gfsutils_common.lua. I think if default is provided, it would be similar to this file.

I'll switch it around so that the container just uses the gfs_common.lua module file along with environment variables to specify the versions available to the container.

sounds good. Thanks @mark-a-potts

src/tave.fd/CMakeLists.txt

aerorahul · 2024-11-13T14:47:06Z

modulefiles/gfsutils_container.intel.lua

+load(pathJoin("stack-intel-oneapi-mpi", stack_impi_ver))
+load(pathJoin("cmake", cmake_ver))
+
+load("gfsutils_generic")


Why would the container not load the gfsutils_common.lua modulefile?

The version numbers are not the same in the container, since it is based on spack-stack 1.8.0.

Understood. Thanks for explaining.
@DavidBurrows-NCO Please create an issue to replace this and consolidate w/ gfsutils_common.lua and remove this file when spack-stack is upgraded to 1.8.0 so that we can revisit it then.
We can accept this for now, but please we cannot afford to maintain multiple copies of modulefiles for different spack-stack versions. That would be unmanageable for maintenance teams.

@aerorahul @mark-a-potts Issue #88 created

aerorahul

@DavidBurrows-NCO can you please address some questions from the review?

…umbers

aerorahul · 2024-11-13T19:02:00Z

modulefiles/gfsutils_gaeac5.intel.lua

@@ -3,15 +3,15 @@ help([[
  on the NOAA RDHPC machine Gaea C5 using Intel-2023.1.0.
 ]])

-whatis([===[Loads libraries needed for building the UFS Weather Model on Gaea ]===])
+whatis([===[Loads libraries needed for building the UFS Weather Model on Gaea C5]===])


Suggested change

whatis([===[Loads libraries needed for building the UFS Weather Model on Gaea C5]===])

whatis([===[Loads libraries needed for building the GFS utilities Gaea C5]===])

aerorahul · 2024-11-13T19:02:28Z

modulefiles/gfsutils_gaeac6.intel.lua

+  on the NOAA RDHPC machine Gaea C6 using Intel-2023.1.0.
+]])
+
+whatis([===[Loads libraries needed for building the UFS Weather Model on Gaea C6 ]===])


Suggested change

whatis([===[Loads libraries needed for building the UFS Weather Model on Gaea C6 ]===])

whatis([===[Loads libraries needed for building the GFS utilities on Gaea C6 ]===])

aerorahul · 2024-11-13T19:06:52Z

modulefiles/gfsutils_gaeac6.intel.lua

+stack_python_ver=os.getenv("stack_python_ver") or "3.11.6"
+load(pathJoin("stack-python", stack_python_ver))


What is using python in this repo?

aerorahul · 2024-11-13T19:07:30Z

modulefiles/gfsutils_gaeac6.intel.lua

+prepend_path("MODULEPATH", "/ncrc/proj/epic/spack-stack/c6/spack-stack-1.6.0/envs/unified-env/install/modulefiles/Core")
+prepend_path("MODULEPATH", "/ncrc/proj/epic/spack-stack/c6/spack-stack-1.6.0/envs/gsi-addon/install/modulefiles/Core")


please remove the duplicate line

I notice its not duplicate. Why are both unified-env and gsi-addon envs needed to be loaded.

@aerorahul unified-env has bufr/12.0.1, and gsi-addon has bufr/11.7.0 needed by gfs-utils. Initially when I ported to C5, I had to have both MODULEPATHs. I just tested C5 with only gsi-addon, and the build was successful. I haven't had access to C6 today so will test that build when I can.

aerorahul · 2024-11-13T19:07:49Z

modulefiles/gfsutils_gaeac5.intel.lua

 load(pathJoin("stack-intel", stack_intel_ver))

-stack_cray_mpich_ver=os.getenv("stack_cray_mpich_ver") or "8.1.25"
+stack_cray_mpich_ver=os.getenv("stack_cray_mpich_ver") or "8.1.28"
 load(pathJoin("stack-cray-mpich", stack_cray_mpich_ver))

 stack_python_ver=os.getenv("stack_python_ver") or "3.11.6"


What is using python in this repo?

@aerorahul I have removed the python load and retested the C5 build successfully. I haven't had access to C6 today so will test that build when I can.

DavidBurrows-NCO and others added 5 commits October 24, 2024 14:36

updates to build on Gaea-C5 and C6

310fbe5

updates for container

b425016

added option for building without w3emc in tave and vint

768d409

update mod file descriptions

06153e7

Merge branch 'feature/c5c6conbuild' of https://github.com/NOAA-EPIC/g…

75426fc

…fs-utils into feature/c5c6conbuild

github-advanced-security bot found potential problems Oct 24, 2024

View reviewed changes

ush/module-setup.sh Show resolved Hide resolved

DavidBurrows-NCO mentioned this pull request Oct 30, 2024

Enable DA cycling on gaea NOAA-EMC/global-workflow#3032

Open

10 tasks

point to C6 install of spack-stack

1d1dc63

WalterKolczynski-NOAA reviewed Nov 12, 2024

View reviewed changes

aerorahul reviewed Nov 13, 2024

View reviewed changes

src/tave.fd/CMakeLists.txt Show resolved Hide resolved

aerorahul reviewed Nov 13, 2024

View reviewed changes

aerorahul requested changes Nov 13, 2024

View reviewed changes

switched container modules to use environment variables for version n…

fc46a1e

…umbers

aerorahul reviewed Nov 13, 2024

View reviewed changes

DavidBurrows-NCO mentioned this pull request Nov 13, 2024

For container build, revert 3 scripts once spack-stack is upgraded to 1.8.0 #88

Open

aerorahul reviewed Nov 13, 2024

View reviewed changes

Update C5 C6 modulefiles

22ae6c3

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add/fix build capability for Gaea-C5, Gaea-C6, and container #87

Add/fix build capability for Gaea-C5, Gaea-C6, and container #87

DavidBurrows-NCO commented Oct 24, 2024

WalterKolczynski-NOAA Nov 12, 2024

mark-a-potts Nov 13, 2024

aerorahul Nov 13, 2024

mark-a-potts Nov 13, 2024

aerorahul Nov 13, 2024

mark-a-potts Nov 13, 2024

aerorahul Nov 13, 2024

aerorahul Nov 13, 2024

mark-a-potts Nov 13, 2024

aerorahul Nov 13, 2024

DavidBurrows-NCO Nov 13, 2024

aerorahul left a comment

aerorahul Nov 13, 2024

aerorahul Nov 13, 2024 •

edited

Loading

aerorahul Nov 13, 2024

aerorahul Nov 13, 2024

aerorahul Nov 13, 2024

DavidBurrows-NCO Nov 13, 2024

aerorahul Nov 13, 2024

DavidBurrows-NCO Nov 13, 2024

	whatis([===[Loads libraries needed for building the UFS Weather Model on Gaea C5]===])
	whatis([===[Loads libraries needed for building the GFS utilities Gaea C5]===])

		stack_python_ver=os.getenv("stack_python_ver") or "3.11.6"
		load(pathJoin("stack-python", stack_python_ver))

		prepend_path("MODULEPATH", "/ncrc/proj/epic/spack-stack/c6/spack-stack-1.6.0/envs/unified-env/install/modulefiles/Core")
		prepend_path("MODULEPATH", "/ncrc/proj/epic/spack-stack/c6/spack-stack-1.6.0/envs/gsi-addon/install/modulefiles/Core")

Add/fix build capability for Gaea-C5, Gaea-C6, and container #87

Are you sure you want to change the base?

Add/fix build capability for Gaea-C5, Gaea-C6, and container #87

Conversation

DavidBurrows-NCO commented Oct 24, 2024

Description

Type of change

Change characteristics

How has this been tested?

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aerorahul left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aerorahul Nov 13, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aerorahul Nov 13, 2024 •

edited

Loading