ESCOMP / CMEPS

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

Fix aoflux for UFS #452

Closed DeniseWorthen closed 5 months ago

DeniseWorthen commented 7 months ago

Adds required fields to the ufs FldsExchange and checks for rainc being associated in med_phases_aoflux_mod.

fixes #448

uturuncoglu commented 7 months ago

@DeniseWorthen I was also discussing these with @jedwards4b and it seems that there are some development in NOAA-EMC side related with this issue that is not pushed to ESCOMP yet. Please see following links,

https://github.com/ESCOMP/CMEPS/blob/3b1e50baab57c739434a4e62937a96a7bea3faf3/mediator/med_phases_aofluxes_mod.F90#L1604

https://github.com/NOAA-EMC/CMEPS/blob/4e19850cb083bc474b7cde5dc2f8506ec74cc442/mediator/med_phases_aofluxes_mod.F90#L1600C1-L1603C14

uturuncoglu commented 7 months ago

@DeniseWorthen I think this PR aims to fix those issues and update ESCOMP. Right? I was also looking to this but maybe you already done it. Let me know if you need help.

DeniseWorthen commented 7 months ago

@uturuncoglu Moving the rainc inside the add_gusts was added by you in https://github.com/ESCOMP/CMEPS/commit/64e1c276dee0de636b03a9bd1186fb794699b02d. But at the top of main at ESCOMP, it is no longer under an add_gusts conditional https://github.com/ESCOMP/CMEPS/blob/3b1e50baab57c739434a4e62937a96a7bea3faf3/mediator/med_phases_aofluxes_mod.F90#L1602-L1605

uturuncoglu commented 7 months ago

@DeniseWorthen Okay. It seems that it is removed with other PR. Right? @jedwards4b Do you know why? Was it creating issue with CESM?

uturuncoglu commented 7 months ago

@DeniseWorthen @jedwards4b As I remember we are testing all the PR with CESM.

DeniseWorthen commented 7 months ago

@uturuncoglu I was testing Jim's alarm fix branch and found that it failed for UFS, because of the change in the aoflux that was in main. This may not be the optimal way of fixing aoflux from CESM's perspective, but I can at least run Jim's alarm fix branch if I include these changes.

DeniseWorthen commented 7 months ago

@uturuncoglu @jedwards4b I was able to test your PR #447 if I also merged in this fix. All tests passed for UFS. Let me know how you want to proceed---if these changes work for CESM or you need a different fix.

DeniseWorthen commented 5 months ago

@jedwards4b I've tested this PR again w/ UFS and it passes all our tests. Thanks.

jedwards4b commented 5 months ago

Thank you @DeniseWorthen I'm not going to be able to get to this until after the CESM meeting next week, but I'll make it a high priority then.

DeniseWorthen commented 5 months ago

@jedwards4b I realized there was a much simpler fix for UFS than I initially thought, requiring only a single change in the med_phases_aoflux module. The remaining required changes can go into our UFS fldsExchange.