new multiple ice sheet functionality #140

mvertens · 2020-11-30T03:25:14Z

Description of changes

Permit GLC (CISM, XGLC) to send multiple ice sheets on different meshes

Specific notes

This PR does the following:

Adds new capability to CISM and CMEPS in order for CISM to send multiple ice sheet data on different meshes to the mediator.
Simplifies the addmrg call in esmFldsExchange_cesm.F90, esmFldsExchange_nems.F90 and esmFldsExchange_hafs.F90 so that the integer index is not longer in the merge calls. This was needed due to the addition of new ice sheet functionality - but should have been this way from the beginning.
As an example for nems:

old:  
call addmrg(fldListTo(compatm)%flds, trim(fldname), mrg_from1=compice, mrg_fld1=trim(fldname), mrg_type1='copy')
new: 
call addmrg(fldListTo(compatm)%flds, trim(fldname), mrg_from=compice, mrg_fld=trim(fldname), mrg_type='copy')

As an example for cesm:

old:
call addmrg(fldListTo(compatm)%flds , 'Sx_'//trim(suffix(n)), &
       mrg_from1=complnd, mrg_fld1='Sl_'//trim(suffix(n)), mrg_type1='merge', mrg_fracname1='lfrac', &
       mrg_from2=compice, mrg_fld2='Si_'//trim(suffix(n)), mrg_type2='merge', mrg_fracname2='ifrac', &
       mrg_from3=compmed, mrg_fld3='So_'//trim(suffix(n)), mrg_type3='merge', mrg_fracname3='ofrac')
new:
call addmrg(fldListTo(compatm)%flds , 'Sx_'//trim(suffix(n)), &
        mrg_from=complnd, mrg_fld='Sl_'//trim(suffix(n)), mrg_type='merge', mrg_fracname='lfrac')
call addmrg(fldListTo(compatm)%flds , 'Sx_'//trim(suffix(n)), &
        mrg_from=compice, mrg_fld='Si_'//trim(suffix(n)), mrg_type='merge', mrg_fracname='ifrac')
call addmrg(fldListTo(compatm)%flds , 'Sx_'//trim(suffix(n)), &
         mrg_from=compmed, mrg_fld='So_'//trim(suffix(n)), mrg_type='merge', mrg_fracname='ofrac')

Based on feedback from @billsacks - new wrappers were added to getting pointer data for a field in an ESMF field bundle and ESMF field.
The new wrapper has the following syntax:

old:
call ESMF_FieldBundleGet(is_local%wrap%FBImp(complnd,complnd) , fieldname='Sl_lfrin', field=lfield, rc=rc)
if (ChkErr(rc,__LINE__,u_FILE_u)) return
call ESMF_FieldGet(lfield, farrayPtr=Sl_lfrin,  rc=rc)
if (ChkErr(rc,__LINE__,u_FILE_u)) return
new:
call fldbun_getdata1d(is_local%wrap%FBImp(complnd,complnd) , 'Sl_lfrin', Sl_lfrin, rc)
if (ChkErr(rc,__LINE__,u_FILE_u)) return

This PR needed new branches to be created for cime and cism. These are contained in the nuopc_dev hash below - but separate PRs willneed to be done to bring these to master.

Contributors other than yourself, if any:

CMEPS Issues Fixed (include github issue #):
Resolves #131

Are changes expected to change answers?

bit for bit
different at roundoff level
more substantial

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

Yes
No

Testing performed if application target is CESM:(either UFS-S2S or CESM testing is required):

(required) CIME_DRIVER=nuopc scripts_regression_tests.py
- machines:
- details (e.g. failed tests):
(required) CESM testlist_drv.xml
- machines and compilers: cheyenne/intel
- details (e.g. failed tests): compared to nuopc_dev hash db0d278 (cmeps baseline nov24newtests) and generated new baselines nov28. All tests were bfb except 1 roundoff level field in the X compset tests.
(optional) CESM prealpha test
- machines and compilers
- details (e.g. failed tests):

Testing performed if application target is UFS-S2S:

(required) UFS-S2S testing
- description: verified that all RTs pass for ufs-model (fe5a943) substituting this branch for CMEPS.
- details (e.g. failed tests):

Hashes used for testing:

CESM:
- repository to check out: https://github.com/ESCOMP/CESM.git
- branch: nuopc_dev
- hash: 22a7a27
UFS-S2S, then umbrella repostiory to check out and associated hash:
- repository to check out:
- branch: fe5a943
- hash:

… roundoff from nov06

…bundle fields

mvertens · 2020-11-30T03:26:26Z

@DeniseWorthen @uturuncoglu - can you both please verify as part of the code review that the changes here will not effect nems or hafs. Thanks!

DeniseWorthen

I have verified that all RTs pass for ufs-model (fe5a943) substituting this branch for CMEPS.

mediator/esmFlds.F90

mediator/esmFldsExchange_cesm_mod.F90

uturuncoglu · 2020-11-30T16:31:41Z

@mvertens i am testing with HAFs now. I'll let you know soon.

mvertens · 2020-11-30T16:40:11Z

@DeniseWorthen - thanks so much for verifying this so quickly!!!!

mvertens · 2020-11-30T16:41:21Z

@DeniseWorthen - what repository did you use for the s2s testing - so that I can fill this in the PR.

mvertens · 2020-11-30T16:44:06Z

@uturuncoglu - thanks for the quick response.

uturuncoglu · 2020-11-30T16:47:01Z

@jedwards4b i am getting following error when I try to build the HAFS model with data components

/work/noaa/nems/tufuk/apps/HAFS_docn/sorc/hafs_forecast.fd/CDEPS/streams/dshr_strdata_mod.F90(35): error #6580: Name in only-list does not exist or is not accessible.   [PIO_SHORT]
  use pio              , only : PIO_BCAST_ERROR, PIO_RETURN_ERROR, PIO_NOERR, PIO_INTERNAL_ERROR, PIO_SHORT
--------------------------------------------------------------------------------------------------^
/work/noaa/nems/tufuk/apps/HAFS_docn/sorc/hafs_forecast.fd/CDEPS/streams/dshr_strdata_mod.F90(1408): error #6404: This name does not have a type, and must have an explicit type.   [PIO_SHORT]
          else if(pio_iovartype == PIO_SHORT .and. .not. allocated(data_short2d)) then
-----------------------------------^
/work/noaa/nems/tufuk/apps/HAFS_docn/sorc/hafs_forecast.fd/CDEPS/streams/dshr_strdata_mod.F90(1438): error #6284: There is no matching specific function for this generic function reference.   [PIO_GET_ATT]
          rcode = pio_get_att(pioid, varid, "_FillValue", fillvalue_i2)

I check the commit history and internal PIO update (feature/hafs_cdeps) was already merged with master. At least, it shown like that

https://github.com/ESCOMP/CMEPS/tree/feature/hafs_cdeps

but there is no "merged" sign if you look at the list of branches. I am not sure why I am getting error related with PIO_SHORT. @jedwards4b Do you have any idea? Maybe it is not merged.

uturuncoglu · 2020-11-30T16:48:44Z

@jedwards4b it seems feature/hafs_cdeps is old and not used branch and we merged feature/hafs_cdeps_fixed with #121 to update PIO

uturuncoglu · 2020-11-30T16:54:59Z

@jedwards4b it seems that PIO shipped with master is not updated version.I think following PR revert all PIO changes back https://github.com/ESCOMP/CMEPS/pull/122/files

DeniseWorthen · 2020-11-30T16:56:50Z

@mvertens I tested in my ufs-weather fork @ develop; hash fe5a943 but with CMEPS replaced w/ your branch

uturuncoglu · 2020-11-30T17:03:22Z

@mvertens @DeniseWorthen @rsdunlapiv @jedwards4b @danrosen25 I think that managing internal PIO shipped with CMEPS is creating lots of confusion and it is hard to maintain. Maybe we could define it as external dependency in HAFS application also like S2S. By this way, we could get rid of entire CMEPS/nems/lib directory (but I think we still need to have CMEPS/nems/util, please confirm @DeniseWorthen). Let me know what do you think? If it is okay for you I could approve this PR and work on it.

DeniseWorthen · 2020-11-30T17:07:23Z

@uturuncoglu We will still need nems/util but I agree removing pio makes sense.

uturuncoglu · 2020-11-30T16:39:51Z

mediator/esmFlds.F90

+  integer, public, parameter  :: comprof  = 6
+  integer, public, parameter  :: compwav  = 7
+  integer, public, parameter  :: compglc1 = 8
+  integer, public, parameter  :: ncomps   = 8


Is this correct? Why number of components is still 8?

mvertens · 2020-11-30T23:46:40Z

Because for now we are only using one ice sheet and we need to generalize the implementation. This will be done when the antarctic ice sheet comes in.

…

On Mon, Nov 30, 2020 at 4:43 PM Ufuk Turunçoğlu ***@***.***> wrote: ***@***.**** approved this pull request. ------------------------------ In mediator/esmFlds.F90 <#140 (comment)>: > - integer, public, parameter :: compatm = 2 - integer, public, parameter :: complnd = 3 - integer, public, parameter :: compocn = 4 - integer, public, parameter :: compice = 5 - integer, public, parameter :: comprof = 6 - integer, public, parameter :: compwav = 7 - integer, public, parameter :: compglc = 8 + integer, public, parameter :: compmed = 1 + integer, public, parameter :: compatm = 2 + integer, public, parameter :: complnd = 3 + integer, public, parameter :: compocn = 4 + integer, public, parameter :: compice = 5 + integer, public, parameter :: comprof = 6 + integer, public, parameter :: compwav = 7 + integer, public, parameter :: compglc1 = 8 + integer, public, parameter :: ncomps = 8 Is this correct? Why number of components is still 8? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#140 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AB4XCE4Q6HOVY5DP2GVR56TSSQU3HANCNFSM4UG7XEQA> .

-- Mariana Vertenstein CESM Software Engineering Group Head National Center for Atmospheric Research Boulder, Colorado Office 303-497-1349 Email: mvertens@ucar.edu

Point to Mariana Vertenstein's branch for ESCOMP/CMEPS#140

billsacks · 2020-12-01T17:19:09Z

This PR resolves #131 . I have edited the top-level PR description to note this so that #131 will auto-close.

* adds multiple ice sheet functionality (ESCOMP#140) * adds post phases for performance enhancement (ESCOMP#146) * adds ocn->glc (land ice) coupling at multiple levels (ESCOMP#148)

Mariana Vertenstein added 13 commits November 12, 2020 20:05

first set of changes to have multiple ice sheets

2082024

successfully completed a TG run with just 1 ice sheets - results were…

1270896

… roundoff from nov06

cleaned up interface for addmrg to remove 1 index

ec373dc

fixed compilation issue

c45fa55

more updates and fixes for multiple ice sheets

f5146b2

updates to get I2000 compset identical to the nov15 baseline

a3d7345

updates to changes for mvertens/fixpreglc

0345e83

new wrapper interfaces for readability for getting pointers to field …

9043f00

…bundle fields

Merge branch 'mvertens/fixprepglc' into mvertens/icesheets

3eff0cb

udpates for PR #139

34b67b6

bug fix

23896d4

replace mapconsf with mapconsd in prep_glc_mod.F90

37bde42

bug fixes in testing

6452bec

mvertens requested review from jedwards4b, DeniseWorthen and uturuncoglu November 30, 2020 03:25

DeniseWorthen approved these changes Nov 30, 2020

View reviewed changes

jedwards4b approved these changes Nov 30, 2020

View reviewed changes

mediator/esmFlds.F90 Show resolved Hide resolved

mediator/esmFldsExchange_cesm_mod.F90 Show resolved Hide resolved

mediator/esmFldsExchange_cesm_mod.F90 Show resolved Hide resolved

clarified comments

3786519

uturuncoglu approved these changes Nov 30, 2020

View reviewed changes

billsacks added a commit to ESCOMP/CISM-wrapper that referenced this pull request Dec 1, 2020

Update cmeps external

5aa7286

Point to Mariana Vertenstein's branch for ESCOMP/CMEPS#140

mvertens merged commit 492569c into master Dec 1, 2020

mvertens deleted the mvertens/icesheets branch December 8, 2020 16:34

mvertens mentioned this pull request Dec 9, 2020

run sequence for addition of rtm/mosart/mizuroute is incorrect #57

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

new multiple ice sheet functionality #140

new multiple ice sheet functionality #140

mvertens commented Nov 30, 2020 •

edited by billsacks

Loading

mvertens commented Nov 30, 2020

DeniseWorthen left a comment

uturuncoglu commented Nov 30, 2020

mvertens commented Nov 30, 2020

mvertens commented Nov 30, 2020

mvertens commented Nov 30, 2020

uturuncoglu commented Nov 30, 2020

uturuncoglu commented Nov 30, 2020

uturuncoglu commented Nov 30, 2020

DeniseWorthen commented Nov 30, 2020

uturuncoglu commented Nov 30, 2020

DeniseWorthen commented Nov 30, 2020

uturuncoglu Nov 30, 2020

mvertens commented Nov 30, 2020 via email

billsacks commented Dec 1, 2020

new multiple ice sheet functionality #140

new multiple ice sheet functionality #140

Conversation

mvertens commented Nov 30, 2020 • edited by billsacks Loading

Description of changes

Specific notes

mvertens commented Nov 30, 2020

DeniseWorthen left a comment

Choose a reason for hiding this comment

uturuncoglu commented Nov 30, 2020

mvertens commented Nov 30, 2020

mvertens commented Nov 30, 2020

mvertens commented Nov 30, 2020

uturuncoglu commented Nov 30, 2020

uturuncoglu commented Nov 30, 2020

uturuncoglu commented Nov 30, 2020

DeniseWorthen commented Nov 30, 2020

uturuncoglu commented Nov 30, 2020

DeniseWorthen commented Nov 30, 2020

uturuncoglu Nov 30, 2020

Choose a reason for hiding this comment

mvertens commented Nov 30, 2020 via email

billsacks commented Dec 1, 2020

mvertens commented Nov 30, 2020 •

edited by billsacks

Loading