Repository navigation
Improve handling of FMS coupler_bc_type fluxes in the NUOPC cap - #97
Merged
Merged
Conversation
3 tasks
dougiesquire
force-pushed
the
wombat-bc-restarts
branch
3 times, most recently
from
October 5, 2026 06:09
6786a88 to
8e085be
Compare
dougiesquire
force-pushed
the
wombat-bc-restarts
branch
5 times, most recently
from
October 8, 2026 02:16
8c9d5fc to
71988cd
Compare
dougiesquire
marked this pull request as ready for review
October 8, 2026 02:45
Collaborator
Author
|
This has already been through a few rounds of review with Claude Opus. An ACCESS-OM3 prerelease build with these changes is here: ACCESS-NRI/ACCESS-OM3#281 ACCESS-OM3 config repro tests can be found linked in the same PR. All pass, except panan |
5 tasks done
dougiesquire
requested review from
minghangli-uni
and removed request for
minghangli-uni
October 8, 2026 06:37
Collaborator
Author
|
@angus-g any chance you have some time to look at this? |
angus-g
reviewed
Oct 9, 2026
get_checksum_loop_ranges sets the index range for a field's restart checksum from G%isd and G%jsd, without regard to the field's staggering. Fields are registered as assumed shape arrays, so the stored array is indexed from 1 whatever bounds the caller declared, and with symmetric memory a field at velocity or corner points starts an extra column or row to the west or south. The range for such a field is therefore erroneously shifted one point to the west or south, and with RESTART_SYMMETRIC_CHECKSUMS it is shifted a further point, into the halo on the western or southern PEs. The range is now derived from the first element of the registered field, which accounts for the staggering. The previous behaviour can be recovered with the new RESTART_STAGGERED_CHECKSUMS_BUG parameter, which defaults to ENABLE_BUGS_BY_DEFAULT. All answers are bitwise identical. With the parameter set false the checksums written into restart files change for fields at velocity or corner points in symmetric memory builds, so restarts written before this change will not validate unless RESTART_CHECKSUMS_REQUIRED is false.
The restart checksum bounds assume that every registered field is declared with halos. Fields that span only the computational domain, such as those held in an FMS coupler type, have no halos, and the resulting range is incorrect. This adds a new optional comp_domain_only argument to register_restart_field, which is recorded against the field. get_checksum_loop_ranges then takes such a field to start at the first computational point rather than the first point of the data domain. Staggering is accounted for in the same way as for any other field, although nothing registers a staggered field this way at present. comp_domain_only defaults to false, so all answers are bitwise identical.
The fields held in an FMS coupler_2d_bc_type cannot currently be registered with the MOM6 restart infrastructure. This adds coupler_type_register_restart_fields, which registers every field of a coupler type with a MOM_restart_CS. All of them are registered as mandatory, so that a field missing from a restart file is a fatal error rather than being silently left at its initialized value. The arrays in a coupler type span only the computational domain, so the fields are registered with comp_domain_only. They are also never rotated, so restart_init takes a new optional turns argument that lets a caller override INDEX_TURNS for the control structure it creates; nothing passes it until the NUOPC cap does. So that the framework does not reach into the FMS types directly, the infrastructure layer gains CT_num_bcs, CT_get_bc, CT_get_field and CT_is_double_precision. The last of these is needed because a coupler type holds its data in one of two arrays depending on its precision, and only the double precision one can be handed to the restart registration interfaces. There are no callers yet, so all answers are bitwise identical.
dougiesquire
force-pushed
the
wombat-bc-restarts
branch
2 times, most recently
from
October 13, 2026 05:29
90728a8 to
8929e37
Compare
angus-g
previously approved these changes
Oct 13, 2026
angus-g
left a comment
angus-g
left a comment
Collaborator
There was a problem hiding this comment.
Excellent, thanks @dougiesquire. I'm happy if you're happy :)
The air-sea tracer flux machinery added in #8 was written around the generic tracer packages specifically, was selected at compile time, and restarted its fields through a bespoke FMS2 interface writing to a directory of its own. This generalises and moves it onto MOM6's own infrastructure. mom_cap_gtracer_flux is renamed MOM_cap_fms_coupler_bcs and now owns a coupler_bcs_CS control structure and a coupler_bcs_* family of routines covering the whole lifetime of the boundary conditions. The FMS coupler calls that were scattered through mom_cap are collected there, so the cap no longer manipulates coupler types directly. The ocean surface fields that the flux calculation consumes are registered with MOM_restart through coupler_type_register_restart_fields and written beside the ocean restarts, listed in the CESM restart pointer file. The additional_restart_dir mechanism is gone. The fields are mandatory, so a missing restart file is a fatal error rather than a silent fall back to the values set by convert_state_to_ocean_type. Existing cases will need the new file added to their pointer file. The #ifdef _USE_GENERIC_TRACER blocks in the cap are replaced by runtime tests, so one executable serves both cases. data_override_init is only called when there are coupler boundary conditions to override, and mom_import now aborts if an import field is neither connected to the coupler nor supplied by data_override, instead of carrying on with whatever the array held. Direct FMS imports are reduced to atmos_ocean_fluxes_mod, which has no MOM6 wrapper. MOM_couplertype_infra gains CT_set_field and a copy_param option on CT_spawn, MOM_coupler_types re-exports the accessors the cap needs, and MOM_constants gains RDGAS, VONKARM and WTMAIR for the flux calculation. All answers are bitwise identical. Co-authored-by: Angus Gibson <angus@agibson.me>
MOM_generic_tracer is always compiled, either against the real GFDL generic tracers package or the stubs under config_src/external, so the macro never controlled whether anything was built. It only guarded the MOM_generic_tracer_fluxes_accumulate calls in the FMS, MCT and NUOPC caps, deciding at compile time something that can be decided at run time. All answers are bitwise identical for any configuration that defined the macro. A configuration that did not define it but did register a generic tracer coupler flux previously skipped the tracer flux accumulation but now performs it.
The call to ocean_model_flux_init in mom_cap is redundant since call_tracer_flux_init is now called earlier via ocean_model_init -> coupler_bcs_init. All answers are bitwise identical.
dougiesquire
force-pushed
the
wombat-bc-restarts
branch
from
October 13, 2026 09:54
8929e37 to
957a2db
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR improves the handling of FMS
coupler_bc_typefluxes in the NUOPC cap to support running with generic tracers.The changes are pretty widespread, but key improvements are:
Cap changes
MOM_restartinfrastructure so they're written and read like any other restart field, and listed in the CESM restart pointer file alongside the ocean restarts. Theadditional_restart_dirhack is gone.mom_importnow aborts if an import field is neither connected to the coupler nor supplied bydata_override.#ifdef _USE_GENERIC_TRACERblocks removed. The path is selected at runtime instead of compile time.MOM_cap_gtracer_fluxmodule has been generalised and renamedMOM_cap_fms_coupler_bcsMOM_cap_fms_coupler_bcsandatm_fieldsrelocated intoice_ocean_boundary_type.MOM_cap_fms_coupler_bcs. EDIT: not quite all.atmos_ocean_fluxes_modis left as a direct import. The solo drivers supply a stub of that module which provides onlyaof_set_coupler_flux. Adding stubs foratmos_ocean_type_fluxes_init/atmos_ocean_fluxes_initto the solo drivers for the purpose of the NUOPC cap seems backwards.src changes
MOM_coupler_typesto register a coupler type's fields for restarts.MOM_constants_USE_GENERIC_TRACERmacroAlso
I also fixed
twoone tangentially-related bugs noticed along the way:Fix a small bug left from NUOPC/MOM_restart - read decomposed restarts when PARALLEL_RESTARTFILES #23This has been moved into it's own PRClaude Opus 5 was used in preparing this contribution