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

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

Merged
merged 706 commits into from
Sep 3, 2024

Conversation

DeniseWorthen
Copy link
Collaborator

Description of changes

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

Specific notes

DeniseWorthen and others added 30 commits June 14, 2023 15:52
Add new XML variables for flexible controls of GPU configuration
Add length to logic format descriptor, needed for nag.
@DeniseWorthen DeniseWorthen marked this pull request as ready for review August 12, 2024 13:02
@NickSzapiro-NOAA
Copy link
Collaborator

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
Copy link
Collaborator Author

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
Copy link
Collaborator

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

  • use_float=.false. is b4b with mediator history output
  • use_float=.true. is similar but not b4b

Both ways seem to be working

Copy link
Collaborator

@dpsarmie dpsarmie left a comment

Choose a reason for hiding this comment

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

No comments

@DeniseWorthen
Copy link
Collaborator Author

@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
Copy link
Collaborator

NickSzapiro-NOAA commented Aug 14, 2024

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
Copy link
Collaborator Author

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
Copy link
Collaborator

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
Copy link
Collaborator Author

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
Copy link
Collaborator

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
Copy link
Collaborator Author

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
Copy link
Collaborator

Sure: ufs-community/ufs-weather-model#2398

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

@DeniseWorthen
Copy link
Collaborator Author

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

@NickSzapiro-NOAA
Copy link
Collaborator

NickSzapiro-NOAA commented Aug 14, 2024

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
Copy link
Collaborator Author

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

@zach1221
Copy link

zach1221 commented Sep 3, 2024

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

@DeniseWorthen DeniseWorthen merged commit 663554e into NOAA-EMC:emc/develop Sep 3, 2024
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.

Update CMEPS for aux-history fix