changes to fix budget issues for aoflux on xgrid#262
Conversation
|
@mvertens is this PR ready for testing? I am plaining to test with UFS XGrid implementation but I just want to be sure. |
DeniseWorthen
left a comment
There was a problem hiding this comment.
All UWM cpld/hafs/cdeps tests using CMEPS pass; hashes used for testing have been noted in the PR
|
@mvertens I could not test this yet since Cheyenne is not available. I could port xgrid work to Orion but it could take time. Sorry for delay. |
|
@uturuncoglu - no problem. Please don't take time to port this to Orion. When cheyenne comes back up you can test this. |
|
@mvertens I tested this under UFS xgrid implementation but there is no any difference in my results when I compare it with the older version of CMEPS? Is it expected behavior? All the fluxes are same. |
|
@uturuncoglu - There should be diffs for Xgrid. However, It will not effect any answers that do not use Xgrid.
|
|
@mvertens it is strange. I am using xgrid but no difference. Let me check it again. Maybe there is something wrong when I merge this with my branch. |
|
@mvertens I think I solved issue. Now merge builds without any issue but I am getting following error when I run it, i'll check it what is going on |
|
@mvertens I think I need to update ESMF version. Right? Which version CESM is using currently? |
|
@mvertens okay. I found it. Compiling with newer version. I hope the UFS model is compatible with newer version of ESMF. |
|
@mvertens Okay. I update ESMF to the same version used under CESM and run the model again. Now I am getting following error, I'll try to find the issue but we might need input from Bob. It could be related with the FV3 grid but I am not sure. |
|
It seems following call causes error in my case, Since this is recently added there could be bug in there. @oehmke @rsdunlapiv do you have any input or suggestion about it. The full trace can be seen in the previous comment. |
|
@uturuncoglu - since this works for me can we put an if clause for the model so that we
can use this for cesm?
|
|
@mvertens if you would like to go with temporal solution that is fine for me and I could test it again when you update the branch. It seems that the issue is in ESMF side but I am not full sure. |
|
@uturuncoglu - I have updated the code to only use the agrid->xgrid bilinear mapping if the model is not hafs. Could you please try this again with your case. Also - could you please raise an issue regarding the problem you found - if this new code now passes for you. |
|
@mvertens application is not hafs. It is nems_frac_aoflux but this is not available to you since it is in my fork. |
|
@uturuncoglu - how do you want to proceed with this PR? I can set the model to nems_frac_aoflux for now - and it will simply skip that code unless you have merged this into your branch? Does that sound reasonable? |
|
@mvertens I think you could revert your recent changes back. Then, I will update my fork to put your logic. Once my fork merged with the CMEPS master, we will have the fix. In the meantime, I could bring this issue in the next ESMF core call to discuss possible solutions in the ESMF side. Let me know what do you think? |
|
@uturuncoglu - okay - that is fine with me. I'll do that and push things back. Are you then okay with accepting this PR? |
|
@mvertens I also confirm that patch gives similar error, I am currently testing with 2nd order conservative. I'll let you know soon about the results. |
|
@mvertens if I replaces patch and bilinear with 2nd order, it works. I have still minor differences when I compare it with the older version of CMEPS. Are those expected differences. I know you update the normalization but I was expecting to have differences around icy regions but after one day simulation, I have difference also over open ocean. I could investigate further but testing again to be sure but I don't want to delay the PR. Let me know what do you think? |
|
@uturuncoglu - if you are referring to xgrid only - I'm not sure. Were you using first order conservative before? |
|
@mvertens let me check the version that I was using perviously. maybe you are right and I was using 1st order. |
|
@mvertens I think it was using 2nd order for u and v and 1st order for the rest (I think this is default for creating route handle with xgrid, right?). Let me try same with new version of the code and maybe the difference will disappear. |
uturuncoglu
left a comment
There was a problem hiding this comment.
@mvertens I tested 2nd order with u and v and 1st order for the other variables but I have still differences. Maybe it is introduced by the normalization. It is very minimal (around 1.0e-12) in the first time step. Anyway, I am approving this PR at this point and I'll open an issue related with the patch and bilinear interpolations.
| call ESMF_FieldRegridStore(xgrid, field_x, field_o, routehandle=rh_xgrid2ogrid, rc=rc) | ||
| if (chkerr(rc,__LINE__,u_FILE_u)) return | ||
| ! call ESMF_FieldRegridStore(xgrid, lfield_o, lfield_x, routehandle=rh_ogrid2xgrid_2ndord, & | ||
| ! call ESMF_FieldRegridStore(xgrid, field_o, field_x, routehandle=rh_ogrid2xgrid_2ndord, & |
There was a problem hiding this comment.
This might be removed from the code but if you are planing to activate it later, that is fine.
|
@mvertens just checking on the timeline for getting this merged in? |
|
@uturuncoglu - can I go ahead and merge this? |
|
@mvertens I created an issue in ESMF side about using bilinear and patch under UFS. Bob is looking for the issue. I think you could merge this and I could put workaround for it in the UFS side. Thanks for asking. |
|
Thank you!
…On Tue, Jan 18, 2022 at 10:54 AM Ufuk Turunçoğlu ***@***.***> wrote:
@mvertens <https://github.com/mvertens> I created an issue in ESMF side
about using bilinear and patch under UFS. Bob is looking for the issue. I
think you could merge this and I could put workaround for it in the UFS
side. Thanks for asking.
—
Reply to this email directly, view it on GitHub
<#262 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AB4XCE7XKUK2JUXLT7H5CS3UWWSO7ANCNFSM5KXH4CYQ>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
You are receiving this because you were mentioned.Message ID:
***@***.***>
--
Dr. Mariana Vertenstein
CESM Software Engineering Group Head
National Center for Atmospheric Research
Boulder, Colorado
Office 303-497-1349
Email: ***@***.***
|
Plumber updates This addresses ESCOMP#248 In order to implement PLUMBER capabilities
Description of changes
Fixes to fix budget differences found between the aoflux calculation on the xgrid and aoflux calculatino on the ogrid.
Specific notes
The key difference is that mapping from the xgrid to the agrid is now done so that fields on the xgrid are normalized by the current open ocean fraction on the xgrid and then mapped and subsequently divided by the ocean fraction on the atm grid.
Contributors other than yourself, if any: None
CMEPS Issues Fixed: None
Are changes expected to change answers?
Any User Interface Changes (namelist or namelist defaults changes)?
Testing performed if application target is CESM:(either UFS-S2S or CESM testing is required):
Testing performed if application target is UFS-coupled:
Testing performed if application target is UFS-HAFS:
Hashes used for testing: