Make signs of fields exported by ATM consistent with positive-down for CMEPS

[DeniseWorthen]

Description

Current special merges in CMEPS and/or sign changes in component caps are required because the ATM component exports fields with the wrong sign (from the perspective of the component) and/or with because the ATM is exporting latent heat when the component expects the evaporative flux.

The fields affected are the momentum stresses and the sensible heat flux, both of which have the incorrect sign for MOM6 and/or HYCOM and the latent heat flux, which must be converted to evaporative flux for MOM6 within CMEPS.

Note: The change to allow ATM to export evap flux was merged previously in PR #1812

This PR applies the change of sign and/or unit conversion when the field is exported by the ATM and removes the specialized conversions and/or merges in CMEPS or the relevant cap. The exported fields will be consistent w/ the coupling definition of "positive down". This will make it easier to extend both the use of the diagnostic capabilities of CMEPS as well refactoring the esmFldsExhange_nems to remove much of the specialized logic for merges depending on the coupling_mode (ie, nems_orig, nems_orig_data etc).

Input data additions/changes

• No changes are expected to input data.
• Changes are expected to input data:
  • New input data.
  • Updated input data.

Anticipated changes to regression tests:

• No changes are expected to any regression test.
• Changes are expected to the following tests:

Cpld and hafs test change baselines because sign for the sensible, evaporative, latent and momentum within the mediator files are different. This was verified by running the full RT was run on Hera at commit f767818. In this case, all files except mediator history and restart files reproduce. Note that all DATM (ng-godas) tests reproduce because the fluxes in those cases are computed in the mediator and have the correct sense of positive down.

RegressionTests_hera.log

Failed tests

7399:001 cpld_control_p8_mixedmode_intel failed in check_result
7401:002 cpld_control_gfsv17_intel failed in check_result
7403:003 cpld_control_p8_intel failed in check_result
7405:005 cpld_control_qr_p8_intel failed in check_result
7407:007 cpld_2threads_p8_intel failed in check_result
7409:008 cpld_decomp_p8_intel failed in check_result
7411:009 cpld_mpi_p8_intel failed in check_result
7413:010 cpld_control_ciceC_p8_intel failed in check_result
7415:011 cpld_control_c192_p8_intel failed in check_result
7417:013 cpld_bmark_p8_intel failed in check_result
7419:015 cpld_control_noaero_p8_intel failed in check_result
7421:016 cpld_control_nowave_noaero_p8_intel failed in check_result
7423:017 cpld_debug_p8_intel failed in check_result
7425:018 cpld_debug_noaero_p8_intel failed in check_result
7427:020 cpld_control_c48_intel failed in check_result
7429:021 cpld_control_p8_faster_intel failed in check_result
7431:022 cpld_control_pdlib_p8_intel failed in check_result
7433:025 cpld_debug_pdlib_p8_intel failed in check_result
7435:132 hafs_regional_atm_ocn_intel failed in check_result
7437:134 hafs_regional_atm_ocn_wav_intel failed in check_result
7439:149 hafs_regional_docn_intel failed in check_result
7441:150 hafs_regional_docn_oisst_intel failed in check_result
7443:151 hafs_regional_datm_cdeps_intel failed in check_result
7445:232 cpld_control_p8_gnu failed in check_result
7447:233 cpld_control_nowave_noaero_p8_gnu failed in check_result
7449:234 cpld_debug_p8_gnu failed in check_result
7451:235 cpld_control_pdlib_p8_gnu failed in check_result
7453:236 cpld_debug_pdlib_p8_gnu failed in check_result

Non-reproducing files

tests/logs/log_hera: grep NOT rt_* 

rt_001_cpld_control_p8_mixedmode_intel.log:64: Comparing RESTART/ufs.cpld.cpl.r.2021-03-23-21600.nc ............ALT CHECK......NOT OK
rt_002_cpld_control_gfsv17_intel.log:63: Comparing RESTART/ufs.cpld.cpl.r.2021-03-23-21600.nc ............ALT CHECK......NOT OK
rt_003_cpld_control_p8_intel.log:64: Comparing RESTART/ufs.cpld.cpl.r.2021-03-23-21600.nc ............ALT CHECK......NOT OK
rt_005_cpld_control_qr_p8_intel.log:64: Comparing RESTART/ufs.cpld.cpl.r.2021-03-23-21600.nc ............ALT CHECK......NOT OK
rt_007_cpld_2threads_p8_intel.log:52: Comparing RESTART/ufs.cpld.cpl.r.2021-03-23-21600.nc ............ALT CHECK......NOT OK
rt_008_cpld_decomp_p8_intel.log:52: Comparing RESTART/ufs.cpld.cpl.r.2021-03-23-21600.nc ............ALT CHECK......NOT OK
rt_009_cpld_mpi_p8_intel.log:52: Comparing RESTART/ufs.cpld.cpl.r.2021-03-23-21600.nc ............ALT CHECK......NOT OK
rt_010_cpld_control_ciceC_p8_intel.log:64: Comparing RESTART/ufs.cpld.cpl.r.2021-03-23-21600.nc ............ALT CHECK......NOT OK
rt_011_cpld_control_c192_p8_intel.log:52: Comparing RESTART/ufs.cpld.cpl.r.2021-03-23-43200.nc ............ALT CHECK......NOT OK
rt_013_cpld_bmark_p8_intel.log:47: Comparing RESTART/ufs.cpld.cpl.r.2013-04-01-21600.nc ............ALT CHECK......NOT OK
rt_015_cpld_control_noaero_p8_intel.log:63: Comparing RESTART/ufs.cpld.cpl.r.2021-03-23-21600.nc ............ALT CHECK......NOT OK
rt_016_cpld_control_nowave_noaero_p8_intel.log:63: Comparing RESTART/ufs.cpld.cpl.r.2021-03-23-21600.nc ............ALT CHECK......NOT OK
rt_017_cpld_debug_p8_intel.log:52: Comparing RESTART/ufs.cpld.cpl.r.2021-03-22-32400.nc ............ALT CHECK......NOT OK
rt_018_cpld_debug_noaero_p8_intel.log:51: Comparing RESTART/ufs.cpld.cpl.r.2021-03-22-32400.nc ............ALT CHECK......NOT OK
rt_020_cpld_control_c48_intel.log:51: Comparing RESTART/ufs.cpld.cpl.r.2021-03-23-21600.nc ............ALT CHECK......NOT OK
rt_021_cpld_control_p8_faster_intel.log:64: Comparing RESTART/ufs.cpld.cpl.r.2021-03-23-21600.nc ............ALT CHECK......NOT OK
rt_022_cpld_control_pdlib_p8_intel.log:63: Comparing RESTART/ufs.cpld.cpl.r.2021-03-23-21600.nc ............ALT CHECK......NOT OK
rt_025_cpld_debug_pdlib_p8_intel.log:51: Comparing RESTART/ufs.cpld.cpl.r.2021-03-22-32400.nc ............ALT CHECK......NOT OK
rt_132_hafs_regional_atm_ocn_intel.log:9: Comparing ufs.hafs.cpl.hi.2019-08-29-21600.nc ............ALT CHECK......NOT OK
rt_132_hafs_regional_atm_ocn_intel.log:10: Comparing ufs.hafs.cpl.r.2019-08-29-21600.nc ............ALT CHECK......NOT OK
rt_134_hafs_regional_atm_ocn_wav_intel.log:12: Comparing ufs.hafs.cpl.r.2019-08-29-21600.nc ............ALT CHECK......NOT OK
rt_149_hafs_regional_docn_intel.log:7: Comparing ufs.hafs.cpl.hi.2019-08-29-21600.nc ............ALT CHECK......NOT OK
rt_149_hafs_regional_docn_intel.log:8: Comparing ufs.hafs.cpl.r.2019-08-29-21600.nc ............ALT CHECK......NOT OK
rt_150_hafs_regional_docn_oisst_intel.log:7: Comparing ufs.hafs.cpl.hi.2019-08-29-21600.nc ............ALT CHECK......NOT OK
rt_150_hafs_regional_docn_oisst_intel.log:8: Comparing ufs.hafs.cpl.r.2019-08-29-21600.nc ............ALT CHECK......NOT OK
rt_151_hafs_regional_datm_cdeps_intel.log:5: Comparing ufs.hafs.cpl.hi.2019-08-30-00000.nc ............ALT CHECK......NOT OK
rt_151_hafs_regional_datm_cdeps_intel.log:6: Comparing ufs.hafs.cpl.r.2019-08-30-00000.nc ............ALT CHECK......NOT OK
rt_232_cpld_control_p8_gnu.log:64: Comparing RESTART/ufs.cpld.cpl.r.2021-03-23-21600.nc ............ALT CHECK......NOT OK
rt_233_cpld_control_nowave_noaero_p8_gnu.log:63: Comparing RESTART/ufs.cpld.cpl.r.2021-03-23-21600.nc ............ALT CHECK......NOT OK
rt_234_cpld_debug_p8_gnu.log:52: Comparing RESTART/ufs.cpld.cpl.r.2021-03-22-32400.nc ............ALT CHECK......NOT OK
rt_235_cpld_control_pdlib_p8_gnu.log:63: Comparing RESTART/ufs.cpld.cpl.r.2021-03-23-21600.nc ............ALT CHECK......NOT OK
rt_236_cpld_debug_pdlib_p8_gnu.log:51: Comparing RESTART/ufs.cpld.cpl.r.2021-03-22-32400.nc ............ALT CHECK......NOT OK

Subcomponents involved:

• AQM
• CDEPS
• CICE
• CMEPS
• CMakeModules
• FV3
• GOCART
• HYCOM
• MOM6
• NOAHMP
• WW3
• stochastic_physics
• none

Library Updates/Changes

• Not Needed
• Create separate issue in JCSDA/spack-stack asking for update to library. Include library name, library version.
• Add issue link from JCSDA/spack-stack following this item

Combined with PR's (If Applicable):

Commit Queue Checklist:

• Link PR's from all sub-components involved in section below
• Confirm reviews completed in ALL sub-component PR's
• Add all appropriate labels to this PR.
• Run full RT suite on either Hera/Cheyenne AND attach log to a PR comment.
• Add list of any failed regression tests to "Anticipated changes to regression tests" section.

Linked PR's and Issues:

• fixes ATM needs to export coupling fields to mediator with correct sign or unit NOAA-EMC/fv3atm#669
• fixes CMEPS is forced to use custom merges because sign of field imported from ATM is wrong or unit conversion is required NOAA-EMC/CMEPS#93
• fixes remove aoflux calculation when using coupling_mode='hafs' NOAA-EMC/CMEPS#94

• depends on swap signs on imported fields NOAA-EMC/HYCOM-src#4
• depends on modify cmeps for correct signs from ATM NOAA-EMC/CMEPS#97
• depends on Change signs of exported fields from ATM NOAA-EMC/fv3atm#675

Testing Day Checklist:

• This PR is up-to-date with the top of all sub-component repositories except for those sub-components which are the subject of this PR.
• Move new/updated input data on RDHPCS Hera and propagate input data changes to all supported systems.

Testing Log (for CM's):

• RDHPCS
  • Hera
  • Orion
  • Jet
  • Gaea
  • Cheyenne
• WCOSS2
  • Dogwood/Cactus
  • Acorn
• CI
  • Completed
• opnReqTest
  • N/A
  • Log attached to comment

[junwang-noaa]

Is the sign swapping in HYCOM required just because the sign in swapped in fv3? I still feel this rule of requiring all the components to provide "positive down" in CMEPS could add additional burden in the component level. In this case, it seems to me MOM6 requires positive down while HYCOM needs "positive up". Also how about wind direction? Will CMEPS add some rules on wind direction? I think we need to allow component to be "self described", the direction (up/east as positive) can be defined as attributes. Mediator can decide if sign switching is needs for the coupling fields between two components. I am worried as we are using pointers for the coupling fields between fv3 and GOCART, which does not use CMEPS but connectors. Now we have to force fv3 and gocart to change signs because of CMEPS.

[DeniseWorthen]

@junwang-noaa I've run the full RT suite and the only tests that are changing are the expected ones (coupled or hafs).

Edit--removed previous text since I didn't state it correctly. HYCOM uses the fluxes with a (-1) applied. So it also internally defines "positive down" (sfcflux = ~ rad flux - sensible flux - latent flux).

Basically I think the fields exported by a component should be in the sense "positive down" and consistent between components. To me, having ATM export fields to other components with the sign defined for the ATM's use doesn't make sense---these are the fields that are being sent to other components. Within the coupled system, there should be a consistent definition of the direction of the fluxes.

[junwang-noaa]

I see. So the proposed change here is that fv3atm only changes signs for those fields sent to ocn (mom6/hycom), not all the fields in its export state such as:

https://github.com/NOAA-EMC/fv3atm/blob/develop/atmos_model.F90#L1505

[DeniseWorthen]

The changes are actually all in the block_data_copy of setup_exportdata:

(https://github.com/DeniseWorthen/fv3atm/blob/32fb333e6eb0df3eabcc4e864ef34edb8d7ac0bd/atmos_model.F90#L2994-L2995)

[zach1221]

Hey, @DeniseWorthen . I'm still trying to pin down when these qr tests started to have issues. I think it started around the time this issue was created 1832. Which I guess makes sense if PR 1812 was the last time these tests matched without issue, but I don't understand why it only fails to match against certain files on Cheyenne only. I think I can get it figured out, but it may take some more time.

[jkbk2004]

I agree we need to work on cheyenne adn skip cheyenne for this pr. On my side, I am not able to build even.

[natalie-perlin]

@jkbk2004 @zach1221,
Please let me know what issues still exist on Cheyenne, and how could I reproduce the error. There should be no missing libraries (if the complaint is about the missing libraries).

[zach1221]

Hi, @natalie-perlin . Sure, I'll reach out to you if needed. I created this issue here, to continue working on the problem.

Since it sounds like we're in agreement on the testing being finished for this PR, we can begin the merge process.

[DeniseWorthen]

@jkbk2004 If tests being turned off on Cheyenne then we need a push to turn off the non-reproing tests and then the Cheyenne log posted. As I understand, only a subset of tests is not running on Cheyenne but the baseline otherwise exists and reproduces.

[zach1221]

@jkbk2004 If tests being turned off on Cheyenne then we need a push to turn off the non-reproing tests and then the Cheyenne log posted. As I understand, only a subset of tests is not running on Cheyenne but the baseline otherwise exists and reproduces.

@DeniseWorthen this is correct. I can push the log to this PR that has all the other tests passing. It will just show that those four qr cases failed.

[DeniseWorthen]

What we need is 1) turn off the tests in rt.conf for cheyenne. 2) A passing cheyenne.log with those turned off.

[zach1221]

What we need is 1) turn off the tests in rt.conf for cheyenne. 2) A passing cheyenne.log with those turned off.

@DeniseWorthen sure, I can commit the change to rt.conf now, and re-run the tests on Cheyenne following the change.

[DeniseWorthen]

@zach1221 At some point you had issues with compiling pdlib cases. How were those resolved?

[zach1221]

@zach1221 At some point you had issues with compiling pdlib cases. How were those resolved?

@DeniseWorthen that was an issue with Scotch loading that Natalie resolved.

[DeniseWorthen]

@zach1221 I will update HYCOM and CMEPS. FV3atm is approved, but I can't remember if I have permissions to merge.

[zach1221]

@DeniseWorthen Sounds good. No worries on FV3atm. I believe @jkbk2004 has authority to merge.

[jkbk2004]

fv3atm pr was merged.

[zach1221]

fv3atm pr was merged.

Thanks, Jong. @DeniseWorthen please update submodule pointer and revert .gitmodules for fv3atm.

Edit: for both cmeps and fv3atm I should say.

[Qingfu-Liu]

The changes look good to me.