Skip to content

CM3 change rc cmeps1.1.31 x#12

Open
kieranricardo wants to merge 44 commits intocmeps1.1.35-xfrom
cm3-change-rc-cmeps1.1.31-x
Open

CM3 change rc cmeps1.1.31 x#12
kieranricardo wants to merge 44 commits intocmeps1.1.35-xfrom
cm3-change-rc-cmeps1.1.31-x

Conversation

@kieranricardo
Copy link
Copy Markdown

@kieranricardo kieranricardo commented Feb 2, 2026

Description of changes

CM3 related changes to CMEPS. This adds an "access" coupling mode and associate field exchange module. Also adds in a custom access coupling routine to "mep_phases_post_atm_mod.F90".

Specific notes

Contributors other than yourself, if any: @blimlim

Are changes expected to change answers? No, this shouldn't affect OM3.

@kieranricardo kieranricardo requested a review from a team February 2, 2026 23:43
Copy link
Copy Markdown
Collaborator

@anton-seaice anton-seaice left a comment

Choose a reason for hiding this comment

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

I didn't go through everything, but my main comment is - can we harmonise field names and code style with the rest of CMEPS

Comment thread mediator/esmFldsExchange_access_mod.F90 Outdated
Comment thread mediator/esmFldsExchange_access_mod.F90 Outdated
Comment thread mediator/med.F90 Outdated
Comment thread mediator/med_phases_post_atm_mod.F90 Outdated
Comment thread mediator/med_phases_post_atm_mod.F90 Outdated
nullify(is_local%wrap)
call ESMF_GridCompGetInternalState(gcomp, is_local, rc)

call ESMF_FieldBundleGet(is_local%wrap%FBImp(compice, compatm), fieldName='ia_aicen', field=ice_frac_cat, rc=rc)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Presumable we should be running the ChkErr error, most of the rest of the code does that

Comment thread mediator/esmFldsExchange_access_mod.F90 Outdated
Comment thread mediator/esmFldsExchange_access_mod.F90 Outdated
! to atm: from ocn
! ---------------------------------------------------------------------
allocate(S_flds(3))
S_flds = (/'So_t', 'So_u', 'So_v'/) ! sea_surface_temperature
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
S_flds = (/'So_t', 'So_u', 'So_v'/) ! sea_surface_temperature
S_flds = (/'So_t', 'So_u', 'So_v'/)

There's lots of comments like this which are wrong or only partially complete

Comment thread mediator/esmFldsExchange_access_mod.F90 Outdated
! ---------------------------------------------------------------------
allocate(S_flds(9))
S_flds = (/'Si_t', &
'ia_aicen', &
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
'ia_aicen', &
'Si_ifrac_n', &

Should we try and reuse existing names for these ?

Copy link
Copy Markdown
Collaborator

@anton-seaice anton-seaice Feb 3, 2026

Choose a reason for hiding this comment

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

from

call addfld_from(compice, 'Si_ifrac_n')
call addfld_to(compocn, 'Si_ifrac_n')

Comment thread mediator/esmFldsExchange_access_mod.F90 Outdated
Comment thread mediator/esmFldsExchange_access_mod.F90 Outdated
kieranricardo and others added 4 commits February 16, 2026 10:09
Co-authored-by: Anton Steketee <79179784+anton-seaice@users.noreply.github.com>
Co-authored-by: Anton Steketee <79179784+anton-seaice@users.noreply.github.com>
Co-authored-by: Anton Steketee <79179784+anton-seaice@users.noreply.github.com>
- rename tti routine and add description
- rename coupling mode to "access-esm"
- delete unfinished comments
- delete unused fields
Comment thread mediator/CMakeLists.txt Outdated
project(cmeps Fortran)

set(SRCFILES esmFldsExchange_cesm_mod.F90 med_fraction_mod.F90
set(SRCFILES esmFldsExchange_access_mod.F90 esmFldsExchange_cesm_mod.F90 med_fraction_mod.F90
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We don't use this file, so possibly this is confusing ([esmFldsExchange_hafs_mod.F90](https://github.com/ACCESS-NRI/CMEPS/blob/upstream-main/mediator/esmFldsExchange_hafs_mod.F90) isn't included, for example)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

It'd be nice to be able to use the same version of CMEPS in CM3 and OM3, but this CMakeList.txt is overridden in access3-share anyway, so I could put the change there. What do you think? Then we have a mediator that can compute both CM3 and OM3 fluxes.

Comment thread mediator/med_phases_post_atm_mod.F90 Outdated

do j = 1,lsize2
do i = 1,lsize1
if (ice_frac_cat_ptr(i, j) > 0.0) then
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

if ice_frac_cat_ptr is very small, won't there be a resulting overflow in ice_flux_cat_ptr ?

@anton-seaice anton-seaice changed the base branch from rc-cmeps1.1.31-x to cmeps1.1.35-x February 26, 2026 01:20
@anton-seaice
Copy link
Copy Markdown
Collaborator

Changed base to cmeps1.1.35-x, looks like it will merge fine

Copy link
Copy Markdown
Collaborator

@anton-seaice anton-seaice left a comment

Choose a reason for hiding this comment

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

Thanks @kieranricardo

I made it as far through this as I had time for.

There are some features in esmFldsExchange_cesm which are missing here and are quite useful, including the configuration option for rof->ocn mapping files and the detection of which meshes are the same. There's also lots of duplication with the addfld/addmap/addmrg with those in cesm FldsExchange.

Would it work to call esmFldsExchange_cesm first, and then in this file, only add any extra fields needed? All the fields in esmFldsExchange_cesm are wrapped in if (fldchk(...) statements, so it should just pass over any fields which are not needed.

Comment thread mediator/esmFldsExchange_access_mod.F90 Outdated
Comment thread mediator/esmFldsExchange_accessesm_mod.F90
!=====================================================================

call addfld_to(compatm, 'So_ofrac')
call addfld_to(compatm, 'Si_ifrac')
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Needs an if comp_present ?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I'm not 100% on this, but I think doing the fieldchecks in esmFldsExchange_accessesm_init is sufficient (which I've now added). We can advertise whatever fields we want (which is what this does), as long as we don't try to map between fields that don't exist.

Also I can't see any comp_present checks in esmFldsExchange_cesm, so I think this should work :)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Yep agreed

Comment thread mediator/esmFldsExchange_accessesm_mod.F90
Comment thread mediator/esmFldsExchange_access_mod.F90 Outdated
Comment thread mediator/esmFldsExchange_accessesm_mod.F90 Outdated
Comment thread mediator/esmFldsExchange_access_mod.F90 Outdated
Comment thread mediator/esmFldsExchange_access_mod.F90 Outdated
Comment thread mediator/esmFldsExchange_access_mod.F90 Outdated
Comment thread mediator/esmFldsExchange_access_mod.F90 Outdated
call addmrg_to(compatm, 'sstfrz', mrg_from=compice, mrg_fld='sstfrz', mrg_type='copy')

allocate(S_flds(7))
S_flds = (/'Si_ifrac_n', &
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We should avoid redefining these lists twice and just reuse, them or set them as Parameters.

Comment thread mediator/esmFldsExchange_access_mod.F90 Outdated
kieranricardo and others added 6 commits March 16, 2026 12:06
Co-authored-by: Anton Steketee <79179784+anton-seaice@users.noreply.github.com>
Co-authored-by: Anton Steketee <79179784+anton-seaice@users.noreply.github.com>
@kieranricardo
Copy link
Copy Markdown
Author

kieranricardo commented Mar 16, 2026

@anton-seaice and @blimlim thanks for you reviews!

Re. using the esmFldsExchange_cesm, there's a few potential issues:

  • If any of the CESM fields use a different re-mapping to us this won't work (I think it will either crash, or the mappings will override each other without a guarentee of which mapping is actually used)
  • I'm fairly sure that this require's aofluxes to run (and crashes it is isn't run), which could potentially override the UM fluxes.
  • There's a potential additional maintainence burden - we'd have to keep track of which fields of ours they define, and what mappings are being used.

So I think we're better off just using our own self-contained module.

Re. the field names, most of these names were chosen to reuse existing fields in fd.yaml. Aside from rain, there aren’t many fields with the Faxa_ prefix there. Also, esmFldsExchange_cesm.F90 sometimes plays a bit fast and loose with the prefixes - e.g. Faxa_rain is exported directly to the ocean, whereas it would probably be more consistent for this to enter the mediator as Faxa_rain and then be mapped to Foxx_rain. Given that, I’m inclined to avoid introducing additional Faxa_... fields into fd.yaml, especially since the other esmFldsExchange... modules don’t fully adhere to that convention either. But I don’t feel strongly about it, so happy to go with whatever approach people think is best.

Comment on lines +505 to +506
'Sa_u', &
'Sa_v', &
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

                  'Sa_v', &

Sounds like these two are not needed? But otherwise should be mappatch_uv3d ?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Yep, I was able to delete this

Comment thread mediator/esmFldsExchange_accessesm_mod.F90
Copy link
Copy Markdown
Collaborator

@anton-seaice anton-seaice left a comment

Choose a reason for hiding this comment

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

Shall we make an issue/issues for missing features, mostly:

-configurable rof2ocn_ice_rmap and rof2ocn_liq_rmap

I went through the remapping methods and added a few questions

! This is a mediator specific routine that determines ALL possible
! fields exchanged between components and their associated routing,
! mapping and merging
!---------------------------------------------------------------------
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Shall we add a note here on the general approach to remapping, e.g. which method is used for which remapping ?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

That might be a little complicated to capture here, but we should definitely document this.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I was just thinking, we use:

  • conservative remapping for states and volume fluxes (temp, area fractions, runoff etc)
  • patch remapping for stresses and wind
    ?

fldchk(is_local%wrap%FBImp(compocn, compocn), trim(fldname), rc=rc) &
) then
call addmap_from(compocn, trim(fldname), compatm, mapconsf, 'ofrac', 'unset')
call addmrg_to(compatm, trim(fldname), mrg_from=compocn, mrg_fld=trim(fldname), mrg_type='copy')
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I guess So_u and So_v should be mappatch_uv3d

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I think we should add an issue for these, if mappatch_uv3d works better we should use this for the stresses as well. But I think we should properly evaluate this before implementing.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We use patch for tau, so surely this should be tau too ?

the _uv3d part does seem contentious, so how about just mappatch

Comment thread mediator/esmFldsExchange_accessesm_mod.F90
'Sa_u', &
'Sa_v', &
'Sa_shum', &
'Sa_tbot', &
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Sa_tbot from atm uses mapbilnr and So_t from ocn to atm uses consf - is that correct ?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I was able to remove Sa_tbot, so now all our state fields are using conservative remapping. Conservative remapping is useful for ice->atm, bc it gives us a good option of weighting by ice fraction (weight by ice fraction, conservatively remap, divide by ice fraction). Also, because the atmosphere is a lot coarser that the ocean/ice grid it doesn't matter too much which re-mamping we use.

@kieranricardo
Copy link
Copy Markdown
Author

@anton-seaice @blimlim I've changed the coupling names around. Some choices still seem ambiguous to me, but at least we're no longer labelling fluxes from the atmosphere with Foxx!

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