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

Modified CICE to support RASM coupling #152

Merged
merged 8 commits into from
Jul 9, 2018

Conversation

apcraig
Copy link
Contributor

@apcraig apcraig commented Jun 24, 2018

Modify CICE to support RASM coupling

  • Developer(s): tcraig

  • Please suggest code Pull Request reviewers in the column at right.

  • Are the code changes bit for bit, different at roundoff level, or more substantial?

  • Is the documentation being updated with this PR? (Y/N)
    If not, does the documentation need to be updated separately at a later time? (Y/N)
    Note: "Documentation" includes information on the wiki and .rst files in doc/source/,
    which are used to create the online technical docs at https://readthedocs.org/projects/cice-consortium-cice/.

  • Other Relevant Details:

Update and verify pio interfaces.
Add spiralcenter and wghtfile decompositions.
Update and verify mct coupling interfaces.
Move to single ice_constants file with init and query methods, coupling interfaces sets the constants to be consistent with RASM shr constants.
Add new check for continue mode and none/default ic.

@apcraig apcraig self-assigned this Jun 24, 2018
This was referenced Jun 24, 2018
Copy link
Contributor

@eclare108213 eclare108213 left a comment

Choose a reason for hiding this comment

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

In ice_init.F90:

! tcx tcraig added, is this correct?
if (trim(runtype) /= 'continue' .and. ice_ic == 'none' .or. ice_ic == 'default') then

Yes, this is correct. The code currently is (or is supposed to be) set up so that if it's not a continuation run then the restart flags in namelist don't matter, i.e. the runtype flag takes precedence. In the logic above, I think you need parentheses around the ice_ic '.or.' parts, otherwise different compilers might interpret the and/or in different orders.

Looks like there's more new stuff to add to the docs, e.g. 2 new distributions and ICE_CONSTOPT

@apcraig apcraig assigned eclare108213 and unassigned apcraig Jun 30, 2018
@apcraig
Copy link
Contributor Author

apcraig commented Jun 30, 2018

The documentation is updated. Mostly, under the implementation performance section. The new documentation can be see here, https://apcraig-cice.readthedocs.io/en/rasma/user_guide/ug_implementation.html#performance. I have updated the decomposition description a bit.

If travis passes, which it should, I think this is ready. Just let me know if you have other changes.

@eclare108213
Copy link
Contributor

This looks okay to me. As I understand it, the values of the constants in ice_constants.F90 are now defaults, and other drivers like cesm or hadgem can reset them using the query/init interface, but they are currently not available in namelist. What is CESM/RASM's preferred method for entering 'SHR_CONST' parameters?

For the record, here are the differences between cice, cesm and hadgem3 constants files:

pn1614351:constants eclare$ diff cice/ice_constants.F90 hadgem3/ice_constants.F90
20,21c20,21
< omega = 7.292e-5_dbl_kind ,&! angular velocity of earth (rad/sec)
< radius = 6.37e6_dbl_kind ! earth radius (m)

     omega     = 7.292116e-5_dbl_kind,&! angular velocity of earth (rad/sec)
     radius    = 6.371229e6_dbl_kind   ! earth radius (m)

pn1614351:constants eclare$ diff cice/ice_constants.F90 cesm/ice_constants.F90
20,21c20,21
< omega = 7.292e-5_dbl_kind ,&! angular velocity of earth (rad/sec)
< radius = 6.37e6_dbl_kind ! earth radius (m)

     omega     = SHR_CONST_OMEGA ,&! angular velocity of earth (rad/sec)
     radius    = SHR_CONST_REARTH  ! earth radius (m)

24c24
< spval_dbl = 1.0e30_dbl_kind ! special value (double precision)

     spval_dbl = SHR_CONST_SPVAL    ! special value

120a121,125

#ifndef USE_ESMF
integer (kind=int_kind), parameter :: &
ESMF_SUCCESS = 0 ! otherwise ESMF defines this parameter
#endif

@eclare108213
Copy link
Contributor

@apcraig @dabail10, would one of you please answer this question? I'd like to know whether it makes sense to move these SHR constants into namelist, if they aren't already there.

the values of the constants in ice_constants.F90 are now defaults, and other drivers like cesm or hadgem can reset them using the query/init interface, but they are currently not available in namelist. What is CESM/RASM's preferred method for entering 'SHR_CONST' parameters?

@eclare108213 eclare108213 merged commit ad68918 into CICE-Consortium:master Jul 9, 2018
@apcraig
Copy link
Contributor Author

apcraig commented Jul 9, 2018

The constants do not have to be moved to namelist. It might be worth doing for internal testing, if we really care, but any coupling interface will have access to the share code in their model and the constants need to be set by calls to icepack/ice "init" interfaces. In other words, CESM would not use the namelist even if CICE had it because the constants have to come from share code. Adding namelist would just be a feature that might or might not be used by others.

When I looked at the constants diffs in the original code (noted above), they were so minimal that I saw no reason to worry about supporting a full set of constants via namelist at that time. But we could do that if it were desired.

@apcraig apcraig deleted the rasmA branch August 17, 2022 20:56
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.

2 participants