-
Notifications
You must be signed in to change notification settings - Fork 79
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
Add index to cpl_scalars to allow CSG or regional (single tile) mediator history files for ATM component in UFS #436
Conversation
… air-sea flux computation (NOAA-EMC#107) * Update ufs/ccpp/data/MED_typedefs.meta.
Sync w/ ESCOMP, adding CDEPS inline capability to CMEPS
* add new cpl_scalar for mediator history files for tiled gridded domains, eg cube-sphere. Replaces existing use of config variables which restricted the use to 6-tiles domain * remove unnecessary trims, fix minor typos and indentation
@uturuncoglu I have this working in UFS when testing w/ a feature branch of FV3 to allow the export of the cpl_scalars from FV3. Would you please test in CESM. I believe it should work but I would prefer this to get tested and merged at ESCOMP first. I also could add the required changes to the NOAHMP cap also. |
@DeniseWorthen Sorry for late response. Last two days were full of meetings. Let me check this with CESM. I think I'll follow same approach and create baseline first and then check against it with your changes. @jedwards4b Please let me know if we have a baseline already. |
@uturuncoglu Thanks. Let me know if you run into any issues. Right now this is draft, but if you don't run into any issues, I'd like to commit it here first. Note the PR also contains the final changes we made to get the CDEPS inline work committed for HAFS implementation. |
@DeniseWorthen Okay. I was going to ask those changes. I'll update you once I finalize the tests. |
@jedwards4b @fischer-ncar I created my own baseline using
FYI, the commands that I used to create baseline and check agains it,
|
@uturuncoglu I wouldn't worry about these failures. There's issues with the generation of the cpl-mem.log and cpl-tput.log. |
@fischer-ncar Thanks for the confirmation and help. I think this PR is fine to go to main branch. Please let me know if you need more test. Otherwise, I think @DeniseWorthen could go ahead and make this PR ready for review. As a side note, I think these kinds of issues are annoying for the unexperienced users like me and make hard to test the CESM. Since, this is shared component with UFS, I think we need to have more smooth testing and maybe guideline for the users that want to contribute to CMEPS and CDEPS. So, they could test easily until we have automated testing capability. |
@uturuncoglu Thanks for your testing. I did want to apologize for combining multiple things in this PR. I'll refrain in the future. |
@DeniseWorthen No worries. I think it is better to combine them since testing in CESM side takes time. |
There is a small fix required to write mediator fractions on ntiles for the case where the
|
* testing of this feature w/ UFS noahmp lnd component, which currently runs on the CSG, found two issues. One to write the mediator fractions and areas on the tiles when using the single history file. A second fix is the mapping masking for lnd-atm coupling in UFS.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@DeniseWorthen This looks good to me. Let me try to run CESM tests one more time if you don't mind to be sure that we are not changing anything in CESM side.
@DeniseWorthen I have just start test. Will update you soon. |
@DeniseWorthen I run CMEPS aux tests with cesm2_3_alpha17b and compare agains my previously generated baseline and it look fine. @fischer-ncar @jedwards4b please let me know if you want me to do more test. Other than that I think this is ready to merge. |
@DeniseWorthen @jedwards4b @fischer-ncar Please wait before merging this. We might need to modify mask for lnd->atm for ufs. I'll update you soon. I think that the issue that we see around coastline is created by the masking. I'll try to set it differently and try again. |
@DeniseWorthen If you don't mind could you modify
as
This will solve the coastline issue that we see in the results. I think, the 0 in destination mask is creating issue since there are fractional grid in UFS side so some grid points are fractional. If you want you could go without mask changes at all in this PR that is also fine and I could bring this later. Anyway, let me know what you think. |
@uturuncoglu I will make the change since I've got this CMEPS PR already open. |
@DeniseWorthen Thanks. I think we could merge this but lets wait confirmation from @fischer-ncar and @jedwards4b. |
@uturuncoglu Are there any issues preventing this getting merged? |
@DeniseWorthen As I know this Pr is fine to merge. @jedwards4b Can we merge this since this is preventing UFS level PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Description of changes
Adds an index to
cpl_scalars
to allow 6 tiles of a CSG or a single tile of a regional ATM grid to be written as the required number of 2-d fields in the mediator history files. Replaces current implementation of this feature, which is limited to CSG and component level history files (ie, not history written by the history write phase).Specific notes
The additional index of
cpl_scalars
is specified with the variableScalarFieldIdxGridNTile
.Includes final changes that were required for
CDEPS_INLINE
functionality in UFSIncludes minor fixup of typos, removal of unnecessary string trims on
coupling_mode
.Includes fix for UFS lnd->atm masking
Contributors other than yourself, if any:
CMEPS Issues Fixed (include github issue #): NOAA-EMC#111
UFS Issue ufs-community/ufs-weather-model#2171
Are changes expected to change answers? (specify if bfb, different at roundoff, more substantial)
No
Any User Interface Changes (namelist or namelist defaults changes)?
Only for UFS
Testing performed
Please describe the tests along with the target model and machine(s)
If possible, please also added hashes that were used in the testing