Fix UFS replay when using active LSM - Githubissues

NCAR / ccpp-scm

CCPP Single Column Model

Other

13 stars 50 forks source link

Fix UFS replay when using active LSM #434

Closed dustinswales closed 5 months ago

dustinswales commented 7 months ago

This PR includes the following changes to enable Replay:

Always do cold-start for UFS replay with active LSM. If using model initial conditions and forcing from the UFS (e.g. Replay mode) with an active LSM, the SCM performs a cold-start initialization for the LSM. That is, UFS replay mode currently DOES NOT support cases generated from UFS Restart files (e.g spun up (warm) LSM parameters). Associated physics PR to fix bug.
Fix for cases when grid orientation cannot be determined. For testing, I generated SCM cases with a regular 2deg grid that covered CONUS. Some of the locations for these cases fell at locations where the regridding software couldn't determine the orientation. Recall we have IC and UFS history files on different grids, and there are locations where this remapping fails if you pick a point on the edge of a tile.
Add functionality to replay script to use input file containing longitude/latitude of points to generate SCM cases from. There is a simple script to help create this file.
Bring in grid area from DEPHY metadata, instead of SCM case configuration file. (I guess in theory all case configuration info should come in via DEPHY file metadata, and the SCM case configuration file would go away? If we go to all DEPHY at some point this would clean up the repository and SCM workflow)

@grantfirl @hertneky @mzhangw

dustinswales commented 6 months ago

@grantfirl This is the branch I've been working with for my Replay debugging if you wanted to take a look.

grantfirl commented 6 months ago

@dustinswales Has this been tested using the new module-based setup on Hera? I'm trying to do this (setting up the env_ufsreplay conda environment) and not having luck on Hera. Conda gets stuck on "solving environment".

hertneky commented 6 months ago

@grantfirl @dustinswales I will chime in that I had this issue before the module-based setup. I ended up having to do it manually on Hera. Worked okay on Cheyenne/Derecho I think.

grantfirl commented 5 months ago

@dustinswales I'd like to be able to test and merge this. Can you please update your PR branch to the latest main?

grantfirl commented 5 months ago

@dustinswales For posterity and ease of review, could you expand the description? (I.e. what was the problem and how it was fixed)

dustinswales commented 5 months ago

@grantfirl Done. Ready for review.

grantfirl commented 5 months ago

@dustinswales Thanks, Dustin. Looking at the ccpp/physics changes, it looks like the changes are based on if https://github.com/NCAR/ccpp-scm/pull/439 is already merged? I'm just confused seeing the ccpp/physics changes include more than just the GFS_phys_time_vary.scm.F90 file.

grantfirl commented 5 months ago

@dustinswales OK, nevermind, I think. IIRC, there were some merges into ccpp/physics that didn't have associated SCM PRs (since nothing changed in *_typedefs), which is why it is showing more changes in ccpp/physics than it otherwise would. I think that is what is going on.

I'm thinking about merging https://github.com/NCAR/ccpp-scm/pull/439 first, though, just to make sure...

dustinswales commented 5 months ago

@grantfirl Hmmm I don't see any unexpected changes here. I started w/ main, not on top of #439.

grantfirl commented 5 months ago

@grantfirl Hmmm I don't see any unexpected changes here. I started w/ main, not on top of #439.

@dustinswales Ya, I think it was the fact that ccpp-physics had merges that didn't need SCM updates. I've merged in #439, so you would please pull in main and commit/push one more time, please (also for https://github.com/NCAR/ccpp-physics/pull/1069)?

grantfirl commented 5 months ago

@dustinswales This looks fine to me. Only comment that may need to be addressed is the error_msg one. The other one might benefit from an additional inline code comment for clarification.

grantfirl commented 5 months ago

@dustinswales I think you need to merge ccpp-physics main into https://github.com/NCAR/ccpp-physics/pull/1069, then point to it here to fix the CI failures.

dustinswales commented 5 months ago

@dustinswales I think you need to merge ccpp-physics main into NCAR/ccpp-physics#1069, then point to it here to fix the CI failures.

Ugh, I forgot to push my updated physics before pushing the updated ccpp-scm. CI cranking along now. Sorry.

grantfirl commented 5 months ago

@dustinswales OK, time to revert .gitmodules and point to the latest ccpp-physics main: https://github.com/NCAR/ccpp-physics/commit/6d8fccbea12c11387c0bc457dcb3855574cd29aa

grantfirl commented 5 months ago

Thanks @dustinswales