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

Update ufs-bundle to build with head of develop of ufs-weather-model for ATM configuration #43

Merged
merged 49 commits into from
Feb 2, 2024

Conversation

mark-a-potts
Copy link
Collaborator

@mark-a-potts mark-a-potts commented Dec 5, 2023

Description

This PR, in combination with PRs for oops and fv3-jedi, allows the ufs-bundle to be built using the head of develop for the ufs-weather-model. Additionally, new regression tests have been added to fv3-jedi that will update the run directories needed for the UFS model and to run a sample Ensemble Forecast of the UFS.

Issue(s) addressed

Resolves #14

Dependencies

List the other PRs that this PR is dependent on:

Impact

None.

Checklist

  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • I have run the unit tests before creating the PR

@mark-a-potts
Copy link
Collaborator Author

This PR has been tested and works with UFS_APP=ATM, ATMAERO, and S2S configurations. There are no tests for ATMAERO or S2S, but both configurations build. The NG-GODAS option does not build because the soca branch required is out of date with the rest of the bundle and requires fairly significant work.

@climbfuji
Copy link
Collaborator

This PR has been tested and works with UFS_APP=ATM, ATMAERO, and S2S configurations. There are no tests for ATMAERO or S2S, but both configurations build. The NG-GODAS option does not build because the soca branch required is out of date with the rest of the bundle and requires fairly significant work.

I have an updated SOCA branch that I used for a similar effort ... I can make that work with this PR.

@mark-a-potts
Copy link
Collaborator Author

That would be great. I started on a merge and it looked like it was going to be a lot of work. I added a patch to handle an issue with checksum checking of restart files down in atmos_cubed_sphere. I know that there are differences in the MOM6 repo, so maybe we could add a similar patch for those and get everything working with the head of develop of the WM.

@climbfuji
Copy link
Collaborator

@mark-a-potts Let's fix the CI failures first. This is because of a merge last week on the JEDI side that changed the model interfaces, but nobody thought about the fv3-jedi etc branches for the UFS. Let me do this in an hour or so, ok?

@mark-a-potts
Copy link
Collaborator Author

I have updated this to now handle building ATM, ATMAERO, and S2S using the head of develop of the ufs-wm. NG-GODAS is still very broken, but the build with S2S will actually run the ATM regression tests without issue.

@climbfuji
Copy link
Collaborator

@mark-a-potts I resolved the merge conflict in dd72818, please take a look to see if everything looks ok

README.md Outdated
@@ -4,7 +4,7 @@ Bundle for interfacing UFS models with JEDI interfaces

## Required thirdparty libraries

This bundle requires the following spack-stack modules be loaded (all except `[email protected]` are in the `skylab-3.0.0` environment):
This bundle requires the following spack-stack modules be loaded (all except `[email protected]` and `[email protected]` are in the `skylab-3.0.0` environment):
Copy link
Collaborator

Choose a reason for hiding this comment

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

We need to update this section (and the list of modules) to something more recent

@climbfuji
Copy link
Collaborator

There is now a CI failure in the cmake step, related to fv3-jedi-data: https://github.com/JCSDA/ufs-bundle/actions/runs/7133595917/job/19426615092?pr=43

@mark-a-potts
Copy link
Collaborator Author

Just removed the extra fv3-jedi-data section. I think it should work now.

@climbfuji
Copy link
Collaborator

Sigh, there are so many updates to the JEDI code that it seems impossible to get a clean run! I'll hop on the instance and clear the build area, then restart the test.

@mark-a-potts
Copy link
Collaborator Author

The latest update to ioda seems to have broken the buid now.

@climbfuji
Copy link
Collaborator

The latest update to ioda seems to have broken the buid now.

Can you try to pull oops develop into your branch please?

@mark-a-potts
Copy link
Collaborator Author

Yeah, I think that was the issue. Just re-built successfully on my machine, so pushing now.

@climbfuji
Copy link
Collaborator

Last thing is to get the fv3-jedi PR merged, correct? I think you avoided the oops dependency (it's still in the PR description) for now.

@mark-a-potts
Copy link
Collaborator Author

Yes. The fv3-jedi PR can be done without the oops PR, and I think it would be good to do that. I will remove the mentions of oops in the PR descriptions.

@mark-a-potts mark-a-potts requested a review from frolovsa January 9, 2024 17:44
Copy link
Collaborator

@climbfuji climbfuji left a comment

Choose a reason for hiding this comment

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

Please update the PR from ufs-bundle develop, revert the fv3-jedi branch name; this PR also uses temporary branches for soca and fv3-jedi-data?

CMakeLists.txt Outdated
ecbuild_bundle( PROJECT soca GIT "https://github.com/jcsda-internal/soca.git" BRANCH feature/ufs_dom UPDATE ) # updated from develop Dec 12 2023
add_dependencies(soca ufs-weather-model)
endif()
ecbuild_bundle( PROJECT fv3-jedi-data GIT "https://github.com/JCSDA-internal/fv3-jedi-data.git" BRANCH feature/ufs-EnsForecasts )
Copy link
Collaborator

Choose a reason for hiding this comment

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

I still see a branch feature/ufs-EnsForecasts for fv3-jedi-data ?

CMakeLists.txt Outdated
add_dependencies(soca ufs-weather-model)
endif()
ecbuild_bundle( PROJECT fv3-jedi-data GIT "https://github.com/JCSDA-internal/fv3-jedi-data.git" BRANCH feature/ufs-EnsForecasts )
ecbuild_bundle( PROJECT fv3-jedi GIT "https://github.com/jcsda-internal/fv3-jedi.git" BRANCH feature/ufs-EnsForecasts UPDATE )
Copy link
Collaborator

Choose a reason for hiding this comment

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

The fv3-jedi branch feature/ufs-EnsForecasts was just merged into develop

CMakeLists.txt Outdated
elseif(UFS_APP MATCHES "^(NG-GODAS)$")
ecbuild_bundle( PROJECT soca GIT "https://github.com/jcsda-internal/soca.git" BRANCH feature/ufs_dom UPDATE ) # updated from develop Dec 12 2023
# ecbuild_bundle( PROJECT soca GIT "https://github.com/jcsda-internal/soca.git" BRANCH feature/ufs_dom UPDATE )
ecbuild_bundle( PROJECT soca GIT "https://github.com/jcsda-internal/soca.git" BRANCH feature/ufs_mark UPDATE )
Copy link
Collaborator

Choose a reason for hiding this comment

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

Has this been merged already?

@climbfuji
Copy link
Collaborator

There is also a conflict for README.md because my last PR replaced w3nco with w3emc.

@mark-a-potts
Copy link
Collaborator Author

Working on this now. The s3 bucket is currently being synched to the latest rt results, so that is delaying the testing a bit. Will update shortly.

@mark-a-potts
Copy link
Collaborator Author

After the latest update, the ctests pass and builds now work for ATM, ATMAERO, NG-GODAS, and S2S configurations.

Copy link
Collaborator

@climbfuji climbfuji left a comment

Choose a reason for hiding this comment

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

A few minor comments to reduce the amount of unnecessary changes etc, other than that this looks great!


# UFS_APP=ATMAERO
cd ${JEDI_ENV}
mkdir -p build-atmaero
cd build-atmaero
rm -rf *
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is this needed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I had to put it there to make sure the build was started fresh. I'll remove it now.

CMakeLists.txt Outdated Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
README.md Outdated
21) libpng/1.6.37 47) ectrans/1.2.0 73) py-f90nml/1.4.3 99) nemsio/2.5.4
22) g2/3.4.5 48) atlas/0.35.0 74) py-h5py/3.7.0 100) sfcio/1.4.1
23) g2tmpl/1.10.2 49) git-lfs/3.0.2 75) py-cftime/1.0.3.4 101) sigio/2.3.2
24) sp/2.3.3 50) gsibec/1.1.3 76) py-netcdf4/1.5.8 102) w3nco/2.4.1
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is reverting my recent change in README.md that replaced w3nco with w3emc

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah, I missed that. I mostly wanted to update the fms version to 2023.02.01 which is now required by the WM. I'll get that straightened out.

climbfuji
climbfuji previously approved these changes Feb 1, 2024
Copy link
Collaborator

@climbfuji climbfuji left a comment

Choose a reason for hiding this comment

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

Thanks for all your work on this!

@climbfuji
Copy link
Collaborator

@climbfuji climbfuji dismissed their stale review February 1, 2024 20:38

CI tests are failing

Copy link
Collaborator

@climbfuji climbfuji left a comment

Choose a reason for hiding this comment

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

Would have been nice to keep the build directories (much faster), but if it doesn't work ...

@climbfuji climbfuji merged commit 11410cc into develop Feb 2, 2024
1 check passed
@climbfuji climbfuji deleted the feature/ufs-EnsForecasts branch February 2, 2024 03:04
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.

Add the capability to run from UFS coldstart initial conditions
2 participants