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

Add Wave slow loop coupling and Field Exchanges for waves in nems #266

Merged
merged 30 commits into from
Feb 1, 2022

Conversation

DeniseWorthen
Copy link
Collaborator

@DeniseWorthen DeniseWorthen commented Jan 23, 2022

Description of changes

Adds the ability to optionally place waves in the slow coupling loop. Also updates esmFldsExchange_nems to advertise and initialize required fields for the UFS S2SW application.

Specific notes

  • A FBExpAccumWav and counter are added. The prep_wav phase is split into prep_wav_accum and prep_wav_avg, replicating what is done for the the ocean component.

    For waves in the fast coupling loop, the current prep_wav phase in the run sequence should be replaced with prep_wav_accum followed by prep_wav_avg. Answers are B4B in UFS testing when compared to using the current prep_wav phase.

  • For waves in the UFS S2SW application, the required fields are added to esmFldsExchange_nems. This requires a temporary conditional around the relevant code so that the S2SW app can continue to use NUOPC connectors during the transition to CMEPS.

  • For the HAFS applications in UFS using both CMEPS and waves, the src and dst masking in med_map_mod are switched to conform with the usual interpretation of src and dst masking. Implementing this change for the HAFS wave coupled applications will ensure that no additional change to CMEPS will be required when HAFS is switched to the new WW3 cap. For continued use of the existing WW3 cap during the transition period, this requires the setting of these variables correctly for the wave model using nems.configure

 mask_value_water = 1
 mask_value_land = 0
  • An extraneous ice->atm mapping in post_ice has also been removed and additional control of the dststatus_print
    is applied to prevent nonsensical files being produced.

Contributors other than yourself, if any:

CMEPS Issues Fixed (include github issue #):

  • Partially addresses UFS issue 739.

Are changes expected to change answers? (specify if bfb, different at roundoff, more substantial)

  • For UFS, testing was performed using a WW3 branch which contains the required retrieval of the mask values for WW3 as well as updates to the fd_nems.yaml. All tests were B4B with current baselines.

Any User Interface Changes (namelist or namelist defaults changes)?

  • Only for UFS (see above).

Testing performed

Testing performed if application target is CESM:

  • verified that the following tests were bfb with cesm2_3_alpha08a
    • SMS_Ld7.f09_g17.B1850.cheyenne_intel.allactive-defaultio
    • ERI_Vnuopc.T62_g37.2000_DATM%NYF_SLND_CICE6_POP2_DROF%NYF_SGLC_WW3.cheyenne_intel.pop-cice

Testing performed if application target is UFS-coupled:

  • (recommended) UFS-coupled testing
    • description: Testing was performed from a personal fork of UWM.
    • The branch used for testing was feature/updcmeps_wavcpl which uses this branch of CMEPS and a feature branch for WW3.
    • details (e.g. failed tests): None

Testing performed if application target is UFS-HAFS:

  • (recommended) UFS-HAFS testing
    • description: The above feature branch was also tested against the current baselines for all HAFS coupled applications in UFS.
    • details (e.g. failed tests): None

Hashes used for testing:

DeniseWorthen and others added 29 commits September 7, 2021 17:46
* add local flag to control whether to write the dststatus file
for a particular RH. This prevents writing a dststatus file for
consf_aofrac which contains garbage (since the RH is a copy, and
dststatus field is not set) or a dststatus file for mapcopy
* sending only cpl_scalars back in export gave mediator error
ESMF_GeomBaseGet Value unrecognized or out of range
med_methods_mod.F90:424
* updates CMEPS with latests changes from ESCOMP, including

- xgrid capablility (cesm only)
- refactored accumulation field bundles 
- cleaned up med.F90 for mesh creation 
- refactored mediatory history functionality
* this is running but not sure of right setting here
* moved files into ufs
* implemented different interface for ufs flux_atmocn_mod
* add compile fixes for ufs
* removed reference to util and replaced with ufs
* clean up of Makefile
* removed references to use of med_kind_mod and introduced ufs_kind_mod
* removed unneeded files needed by ufs
* Optional tiled history files for ATM (ESCOMP#257)
* mapping is done in prep-atm; this mapping is unneeded
tidy up; changes for hafs tests to work. switch dst/src masking back for hafs mode.
This is not required if wmesmf can read correct values (water=1,land=0) from config
file
revert extraneous changes
* prevent field advertising conflicts when running waves
with connectors. The added conditional can be removed when
s2sw is updated to use cmeps
Copy link
Collaborator

@uturuncoglu uturuncoglu left a comment

Choose a reason for hiding this comment

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

I am assuming that all the tests are passing in UFS side. I am not sure who will test this PR in CESM side? @mvertens do you want to test this. If you let me know about the latest baseline in CESM side, I could also run full test suite. Let me know what do you think?

@@ -159,6 +165,16 @@ subroutine esmFldsExchange_nems(gcomp, phase, rc)
call addmap(fldListFr(compocn)%flds, 'So_t', compatm, maptype, 'ofrac', 'unset')
call addmrg(fldListTo(compatm)%flds, 'So_t', mrg_from=compocn, mrg_fld='So_t', mrg_type='copy')

! temporary conditional to avoid conflicts of advertised fields
! when waves are passing through connectors
if (is_local%wrap%comp_present(compwav)) then
Copy link
Collaborator

Choose a reason for hiding this comment

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

Actually, we were also using these type conditionals in the HAFS specific field exchange file. I think it would be nice to unify those files to single one in the future. I think the conditional could also include the name of the component.

Copy link
Collaborator Author

@DeniseWorthen DeniseWorthen Jan 25, 2022

Choose a reason for hiding this comment

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

I think the use in hafs is not quite the same. For both hafs and cesm, there are two calls to the fldsExchange, one for advertise and one for initialize. The conditionals I see in the hafs and cesm fldExchanges are related to that---in both cases at the initialize call you're doing the addmap,addmerge only if the field is in the expFB and also present in the impFB. I do think it would be nice to have a cleaner way of doing all that checking though.

But for the nems modes, esmFieldsExchange_nems is only called once (for phase=advertise) and the addmap and addmerge are done then. There is no checking for whether the fields are present or not. I did consider this as an issue, and refactored the esmfFieldsExchange_nems to do both an advertise and an initialize phase like for hafs and cesm. But I still had the same issue---with waves going through the connectors, having the mediator also trying to advertise those same fields (even if they're not connected) broke reproducibility. So this conditional completely removes the advertisement of fields if the mediator is not in use for the waves.

@@ -401,18 +403,19 @@ subroutine med_map_routehandles_initfrom_field(n1, n2, fldsrc, flddst, mapindex,
polemethod = ESMF_POLEMETHOD_NONE ! todo: remove this when ESMF tripolar mapping fix is in place.
endif
else if (coupling_mode(1:4) == 'nems') then
if (n1 == compatm .and. (n2 == compocn .or. n2 == compice)) then
if ( (n1 == compocn .or. n1 == compice .or. n1 == compwav) .and. &
Copy link
Collaborator

Choose a reason for hiding this comment

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

This part is getting more complex and confusing. I think we need to find way to make it simpler in the near future.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@uturuncoglu @DeniseWorthen - I agree that we need to refactor this part of the code. Can we meet early next week to come up with a plan. The sooner we do this the better.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, I agree.

@@ -59,18 +59,6 @@ subroutine med_phases_post_ice(gcomp, rc)
call med_fraction_set(gcomp, rc)
if (ChkErr(rc,__LINE__,u_FILE_u)) return

! map ice to atm - scaling by updated ice fraction
Copy link
Collaborator

Choose a reason for hiding this comment

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

I am not sure why this is removed. Is it related with CICE albedos and their usage in FV3?

Copy link
Collaborator Author

@DeniseWorthen DeniseWorthen Jan 25, 2022

Choose a reason for hiding this comment

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

No, this is because there is already a mapping of ice to atm in the prep_atm phase. So this was a duplicate mapping.

@mvertens
Copy link
Collaborator

@uturuncoglu - if you could run the test suite for alpha08a with the new cmeps that would be amazing! Thank you for offering!

@uturuncoglu
Copy link
Collaborator

@mvertens Sure. I'll run and update you through the PR.

@uturuncoglu
Copy link
Collaborator

@mvertens @jedwards4b I am getting following error when I try to run CESM tests. Do I need to load specific version of Python?

Traceback (most recent call last):
  File "./create_test", line 29, in <module>
    from Tools.standard_script_setup import *
  File "/glade/scratch/turuncu/CESM/cime/scripts/Tools/standard_script_setup.py", line 17, in <module>
    import CIME.utils
  File "/glade/scratch/turuncu/CESM/cime/scripts/Tools/../../scripts/lib/CIME/utils.py", line 546
    mod = importlib.import_module(f"{compname}_cime_py")
                                                      ^
SyntaxError: invalid syntax

@uturuncoglu
Copy link
Collaborator

@mvertens @jedwards4b okay. I think module load python solved the issue. BTW, do you want me to run prealpha testes or the test in components/cmeps/cime_config/testdefs/testlist_drv.xml?

@uturuncoglu
Copy link
Collaborator

I run pre-alpha tests and some tests are failed with following error,

        Building test for ERP in directory /glade/scratch/turuncu/ERP_Ln9_Vnuopc.f09_f09_mg17.2000_CAM60_CLM50%SP_CICE6%PRES_DOCN%DOM_MOSART_SGLC_SWAV.cheyenne_intel.cam-outfrq9s.20220125_103330_yfkwiz
        /glade/scratch/turuncu/CESM/components/clm/src/biogeochem/CNFireEmissionsMod.F90(282): error #6460: This is not a field name that is defined in the encompassing structure.   [COEFF]

This is not directly related with the CMEPS version that I used. I have also issue in following test /glade/scratch/turuncu/ERI_Vnuopc.T62_g37.2000_DATM%NYF_SLND_CICE6_POP2_DROF%NYF_SGLC_WW3.cheyenne_intel.pop-cice.20220125_103330_yfkwiz.ref1 and it is failing with

20220125 124123.559 ERROR            PET002 ESM0001:src/addon/NUOPC/src/NUOPC_Driver.F90:5027 Invalid argument  - run phase: 'med_phases_prep_wav' could not be identified.
20220125 124123.559 ERROR            PET002 ESM0001:src/addon/NUOPC/src/NUOPC_Driver.F90:6577 Invalid argument  - Passing error in return code
20220125 124123.559 ERROR            PET002 esm.F90:273 Invalid argument  - Passing error in return code
20220125 124123.569 ERROR            PET002 ESM0001:src/addon/NUOPC/src/NUOPC_Driver.F90:1204 Invalid argument  - Passing error in return code
20220125 124123.569 ERROR            PET002 ensemble:src/addon/NUOPC/src/NUOPC_Driver.F90:2577 Invalid argument  - Phase 'IPDv02p1' Initialize for modelComp 1: ESM0001 did not return ESMF_SUCCESS
20220125 124123.569 ERROR            PET002 ensemble:src/addon/NUOPC/src/NUOPC_Driver.F90:1270 Invalid argument  - Passing error in return code
20220125 124123.569 ERROR            PET002 ensemble:src/addon/NUOPC/src/NUOPC_Driver.F90:457 Invalid argument  - Passing error in return code
20220125 124123.569 ERROR            PET002 esmApp.F90:146 Invalid argument  - Passing error in return code
20220125 124123.569 INFO             PET002 Finalizing ESMF

So, this seems related with the new PR. @DeniseWorthen I think you removed med_phases_prep_wav and split into the pieces like init, accum and avg. So, I think that run sequences of the CESM tests need to be adjusted based on this change. @mvertens let me know what do you think?

@DeniseWorthen
Copy link
Collaborator Author

@uturuncoglu Yes, you'll need to replace the current prep_wave with prep_wave_accum and prep_wave_avg if waves are in the fast loop.

@mvertens
Copy link
Collaborator

@uturuncoglu - sorry for the slow response to this. I am going to push a change back to cmeps/cime_config that implements exactly what @DeniseWorthen suggested. I'll do a few tests and then let everyone know.

@uturuncoglu
Copy link
Collaborator

@mvertens sure. let me know if you need any help.

@mvertens
Copy link
Collaborator

@uturuncoglu @denise - I have pushed a fix back to this branch and verified that the following test:
ERI_Vnuopc.T62_g37.2000_DATM%NYF_SLND_CICE6_POP2_DROF%NYF_SGLC_WW3.cheyenne_intel.pop-cice passes and is bfb the alpha08a baseline.
@uturuncoglu - if you are willing to try the alpha tests again out of an alpha08a sandbox with this cmeps branch in replacing the alpha08a cmeps - that would be great.

@mvertens mvertens merged commit 7419333 into ESCOMP:master Feb 1, 2022
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.

3 participants