Add fv3-jedi and its dependencies (SkyLab 8 version)#395
Add fv3-jedi and its dependencies (SkyLab 8 version)#395rhoneyager-tomorrow wants to merge 10 commits intoJCSDA:spack-stack-devfrom
Conversation
…kylab7_components
|
@rhoneyager-tomorrow This replaces #306, correct? |
|
Yes. I can close the old version. |
|
@rhoneyager-tomorrow Let's see if we can get this PR merged/resolved. My preference would be to apply all the patches that you submitted here to the Skylab v8 release and then do away with anything prior to that in the spack PR. I would also prefer to pick apart this PR and add/update packages individually. For packages that are also in spack mainline, I would prefer submitting the updates to the upstream repo first and then bring them down to our fork. Other notes:
I can assist with or take care of some or most of this work, but hopefully we can split the work somewhat. Thoughts? |
|
@rhoneyager-tomorrow I am going to submit this to femps (note that I removed the Does that look ok for you? |
|
Same for fv3-jedi-linearmodel: |
| variant("mpi", default=True, description="Support for MPI distributed parallelism") | ||
| variant("openmp", default=True, description="Build with OpenMP support") | ||
|
|
||
| conflicts("forecast_model=GEOS", msg="FV3-JEDI-LINEARMODEL: GEOS to be implemented.") |
There was a problem hiding this comment.
I think we are using the current fv3-jedi-linearmodel with GEOS and UFS - @shlyaeva can you comment on this?
There was a problem hiding this comment.
You're using both, but at time of writing this PR GEOS wasn't fully buildable in Spack. Likewise, the ufs-bundle depended on JCSDA-internal, so I didn't have much insight there.
There was a problem hiding this comment.
I think we can merge it as-is now, and update later as needed
| depends_on("ecbuild", type=("build")) | ||
| depends_on("ecbuild@3.3.2:", type=("build"), when="@3.0.7:") | ||
|
|
||
| version("3.0.7", commit="1a02ebaf6f7a4e9f2c2d2dd973fb050e697bcc74") |
There was a problem hiding this comment.
Note the non-standard notation of the tag. In the repo, it's v3.07
| +++ b/CMakeLists.txt | ||
| @@ -42,7 +42,7 @@ find_package( atlas 0.33.0 REQUIRED COMPONENTS OMP_Fortran ) | ||
| if( ENABLE_MKL ) | ||
| find_package( MKL ) |
There was a problem hiding this comment.
In fact, this should rather be
if( ENABLE_MKL )
find_package( MKL REQUIRED )
(at least that is what we decided at JCSDA - if you request a feature then it is an error if it is not available)
There was a problem hiding this comment.
Actually, we already have the change that you made in develop, and your version makes more sense than what I was thinking initially. That's because it's easier going to use Intel MKL or LAPACK, but if both are missing it will abort. I am going to revert that comment. I will make it a hard requirement if someone asks for MKL.
There was a problem hiding this comment.
My idea doesn't work, fixing the underlying problem requires changes across several repositories.
SkyLab 8 isn't released yet, but I expect that it will take some time for all the patches to be applied, so I'm quite fine with everything that you propose.
Great news! Hopefully you can do the same with mpas eventually.
Splitting works for me. Let me know once you finish reviewing. |
|
I created the PRs for femps and fv3-jedi-linearmodel so far based on what I posted above |
|
Regarding the ecmwf-atlas update: version 0.36.0 is already in spack develop, and I just opened spack#43133 to add the missing ecbuild@3.4: dependency. Once that's merged, I can bring both updates down to our fork. |
|
@rhoneyager-tomorrow We merged the femps and fv3-jedi-linearmodel build system bug fixes. Do you want to check the public develop branches? That will be in the Skylab v8 releases. |
|
|
||
| version("master", branch="master") | ||
| version("develop", branch="develop") | ||
| version("0.36.0", sha256="39bf748aa7b22df80b9791fbb6b4351ed9a9f85587b58fc3225314278a2a68f8") |
There was a problem hiding this comment.
Changes to this file will no longer be needed. We merged the missing ecbuild@3.4: dependency upstream (spack#43133); spack develop already had 0.36.0. We'll bring both updates to our spack fork shortly.
| depends_on("netcdf-c") | ||
| depends_on("netcdf-fortran") | ||
|
|
||
| # find_package(ecbuild REQUIRED) is needed when using ecbuild. |
There was a problem hiding this comment.
Patches should no longer be required if we target sklyab v8
| res = [self.define_from_variant("FV3_FORECAST_MODEL", "forecast_model")] | ||
| return res | ||
|
|
||
| # find_package(ecbuild REQUIRED) is needed when using ecbuild. |
There was a problem hiding this comment.
Patches should no longer be required if we target sklyab v8
| @@ -0,0 +1,11 @@ | |||
| --- a/CMakeLists.txt | |||
There was a problem hiding this comment.
No longer required for Skylab v8 because of JCSDA/GFDL_atmos_cubed_sphere#3
|
@rhoneyager-tomorrow Here is an update on the PR: I've merged the following patches (w/ slight modifications into the develop code, to be released in Skylab v8 next month):
I also updated the following package recipes in our spack fork:
What needs to remain in this PR (but updated to only cover the Skylab v8 versions):
|
|
@rickgrubin-noaa @rhoneyager-tomorrow I believe you have whatever JEDI packages you need in Spack at tomorrow.io, correct? With that and my PR JCSDA/spack-stack#1887, should we close this PR for good once JCSDA/spack-stack#1887 is merged? |
Agree with closing this PR once JCSDA/spack-stack#1887 is merged. |
This PR updates the JEDI packages from PR JCSDA/spack#395 that are relevant for NEPTUNE-JEDI (aka FALCON project) and adds them to our custom repo under repos/spack_stack. Building these components in spack-stack and providing them as modules allows us to reduce the NEPTUNE-JEDI build to a small number of repositories that are controlled and developed by NRL only, and - very important - allows us to implement a pure CMake build at the top level (neptune-bundle/neptune-ecosystem) without having to deal with ecbuild. A small change is made to the crtm package (in repos/builtin) to enable basic testing during spack install. Notes/Limitations The hashes added here are the exact versions needed by NEPTUNE-JEDI (aka FALCON), with additional bug fixes added on top of it in form of patches applied by Spack. In FALCON, these were kept in branches created from the commits used here. The Spack solution is cleaner, since it avoids maintaining branches. Most of these patches are already included in newer versions of the JEDI code. I made sure that all ctests that are enabled pass with GCC (tested gcc@13) and Intel oneAPI (tested oneapi@2025.3.0). I made sure that the tests that are disabled are not related to the Spack package, but to the code itself or other issues outside of the scope of this PR. To run these tests, use spack install --test=root ioda, for example. FALCON needs exactly these hashes in spack-stack-2.1.0 to transition from building JEDI components in neptune-bundle to loading them as modules. It is understood between JCSDA and NRL that these new Spack packages will not receive any additional support (i.e. beyond what JCSDA supports already) and that we will make an effort to update the JEDI code in the bundle to newer, official tags of the JEDI repositories. We cannot test the JEDI packages in CI yet, because the ropp-ufo code is coming from the NRL Enterprise GitHub because of the unresolved issue Stub with ROPP as submodule JCSDA-internal/ropp-ufo#1.
|
Replaced by JCSDA/spack-stack#1887 and code internal to tomorrow.io |
Description
The previous PR (#306) has been around for a while. I have updated it for the new SkyLab v7 release.
This provides the same as before, with the addition of mom6 and soca packages.
Errata
I've excluded updates/additions of mpas-model and mpas-jedi from this PR. The mpas package on Spack is entirely incompatible with the JCSDA fork. CMake packaging logic vs a custom Makefile. I tested production of a CMake-ified version mpas-model package, but there is a bug in the CMake package export logic in the JCSDA fork, and this prevents use with mpas-jedi.
Checklist