NOAA-EMC / CMEPS

NUOPC Community Mediator for Earth Prediction Systems
https://escomp.github.io/CMEPS/
0 stars 20 forks source link

Sync with ESCOMP; fix aux history files for use_float=.true. #124

Closed DeniseWorthen closed 1 week ago

DeniseWorthen commented 1 month ago

Description of changes

Syncs with ESCOMP, bringing back fix for aux history files so that use_float=.true. works correctly.

Specific notes

NickSzapiro-NOAA commented 1 month ago

Looks good to me. My expectation is that the atm_ds2s_docn_dice test is b4b when configured to use aux history rather than mediator history. Can we include that here (+ run on more platforms) or would you prefer to keep that separate?

DeniseWorthen commented 1 month ago

Right now, this is no-baseline change but it probably makes sense to combine even if there is a baseline change for that one test. So I'd say go ahead and test.

NickSzapiro-NOAA commented 1 month ago

For atm_ds2s_docn_dice test switched to auxiliary history output (https://github.com/NickSzapiro-NOAA/ufs-weather-model/tree/aux_cplhist)

Both ways seem to be working

DeniseWorthen commented 1 month ago

@NickSzapiro-NOAA I thought you had made a comment w/rt the S4 issue that ncrcat commands/processing of ouput would not be required once you switched to aux history. I don't see any changes like that in your feature branch. Am I mis-remembering?

NickSzapiro-NOAA commented 1 month ago

Yes, in tests/fv3_conf/cpld_docn_dice.IN as we can write many auxiliary history times to one file directly image

Maybe the * is misleading...I wasn't sure exactly how the files would be named. It's just 1 aux file for ice and 1 for ocean from the preceding coupled run

DeniseWorthen commented 1 month ago

So by setting ntperfile=9999 you get all 24 1-hour timesteps in one file, correct? Is there any reason you didn't explicitly use ntperfile=24 ?

NickSzapiro-NOAA commented 1 month ago

Yes. Since it's nice to run tests with longer forecast lengths and this is hardcoded in the ufs.configure, I left a generous cushion. If a concern, maybe the most proper is to add another atparse variable

DeniseWorthen commented 1 month ago

I think if you make this 24, then for longer forecasts you'd get more than one aux file, but each aux file would contain 24 hours. Does the stream specification not allow for more than one aux history file containing the fields required?

NickSzapiro-NOAA commented 1 month ago

It does. We exercise multiple stream files for cdeps in a few tests, like: https://github.com/ufs-community/ufs-weather-model/blob/develop/tests/tests/datm_cdeps_multiple_files_cfsr

Meanwhile, I've lost days trying to script the formatting right for flexible forecast lengths and it still not working properly. There are some subtleties in atparse, quotes, and ESMF config format,... like with

!in tests/tests/hafs_regional_datm_cdeps
export DATA_ATM="\"INPUT/ERA5.TL639.2019.08.200618_subset.nc\" \"INPUT/ERA5.TL639.2019.09.200618_subset.nc\""
!in tests/parm/hafs_datm.streams.era5.IN
stream_data_files01:       @[DATA_ATM]"

and I ran out of time to get it working in the end. It's also unnecessary since we can just write to one file and avoid "error prone" solutions (at least for me).

Maybe I'm just missing a main point...why split up the auxiliary history/input stream files?

DeniseWorthen commented 1 month ago

I don't think having the capability for "daily" aux history files is required for us to at least enable them in the test and remove the mediator history file use. But I think for general use, it would be good to not rely on one big file if possible.

Could you make an issue on UWM (if you haven't already) to describe switching to aux history? I'm fine combining this feature branch into my UWM PR and closing that issue.

NickSzapiro-NOAA commented 1 month ago

Sure: https://github.com/ufs-community/ufs-weather-model/issues/2398

Yes, I'm happy to work more on it if there's interest

DeniseWorthen commented 1 month ago

OK, now can you make a PR to me on my fork (https://github.com/DeniseWorthen/ufs-weather-model/tree/feature/fixaux)?

NickSzapiro-NOAA commented 1 month ago

Please let me know if you want me to fill out the PR more fully, @DeniseWorthen

While I see use_float=.true. in ESCOMP/CMEPS, can we set use_float=.false. (more precision + b4b)?

DeniseWorthen commented 1 month ago

@NickSzapiro-NOAA There is currently no way to optionally configure use_float to false.

zach1221 commented 1 week ago

@DeniseWorthen testing is complete on WM PR 2395. Please feel free to merge here.