EAMxx: upgrade/simplify step tendency calculation in atm procs #7824

bartgol · 2025-10-22T18:29:03Z

Allow calculation also of tendency of groups monolithic/individual fields.

[non-BFB] Only for newly added var to eamxx-prod output.

A few more details:

Allow to call get_field_in/out for groups individual/monolithic fields.
Avoid considering internal fields automatically as restart fields (this should e.g. remove omega_dyn and phys_dyn from restart file, which were not needed (omega_dyn) or duplicate (phys_dyn)). Instead, atm procs should specify groups to which the internal field should be added to.
Setup tendencies AFTER all regular fields/groups have been created and set in the atm proc. Since NO OTHER atm proc should update this proc's tendencies, we can create them as internal fields (adding them to the "ACCUMULATED" and "DIVIDE_BY_DT" groups, so that the driver will a) zero them out at the beginning of the atm step, and b) divide by atm_dt at the end of the time step)

Do not add ALL fields to the restart group. Instead, atm procs should specify what groups the internal fields belong to (if any), and the driver will add to all those groups in the field manager

For both input and output fields. And for both individual and monolithic field (if present).

- Setup tends as internal fields (they should not be computed by other procs anyways) - Setup tends in atm procs AFTER individual fields/groups have been set in the process by the driver - Allow to compute tend of groups or group fields

Must also process its own tendencies requests

There are 2 copies of qv, so we must be explicit on which one we want

Do not finalize if it was never inited. Avoids throwing while unwinding stack after uncaught exception

github-actions · 2025-10-29T14:38:26Z

PR Preview Action v1.6.2
🚀 View preview at https://E3SM-Project.github.io/E3SM/pr-preview/pr-7824/
Built to branch `gh-pages` at 2025-10-30 16:57 UTC. Preview will be ready when the GitHub Pages deployment is complete.

bartgol · 2025-10-29T19:44:14Z

All failures are expected:

eamxx-sa: the tend fields have been moved from being a computed field to be an internal field, hence they show up also in the process pre-subcycle hash calculation.
eamxx-prod: I added a field to the output (the qc tend), to verify that we could compute the tendency of a field that dyn was not explicitly requesting (it only gets it via the tracers group).

Copilot

Pull Request Overview

This PR refactors the tendencies computation system in EAMxx to simplify the implementation and fix several issues related to field grouping and restart functionality.

Replaced shortcut notation [all] with explicit field lists in tendency configuration
Moved tendency setup from create_grids() to create_fields() phase
Enhanced internal field handling to support proper group membership and restart tagging

Reviewed Changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
`components/eamxx/tests/single-process/shoc/input.yaml`	Updated tendency configuration from `[all]` shortcut to explicit field list
`components/eamxx/src/share/field/field_group_info.hpp`	Added default initializers for member variables to prevent uninitialized state
`components/eamxx/src/share/data_managers/field_manager.cpp`	Enhanced `add_field` to handle group info updates and monolithic allocation checks
`components/eamxx/src/share/atm_process/atmosphere_process_group.hpp`	Renamed method from `setup_tendencies_requests` to `setup_step_tendencies`
`components/eamxx/src/share/atm_process/atmosphere_process_group.cpp`	Simplified group tendency setup by removing redundant field re-registration logic
`components/eamxx/src/share/atm_process/atmosphere_process.hpp`	Refactored tendency data structures and method signatures for cleaner design
`components/eamxx/src/share/atm_process/atmosphere_process.cpp`	Simplified tendency computation logic and added group support to internal fields
`components/eamxx/src/physics/rrtmgp/eamxx_rrtmgp_process_interface.cpp`	Added guard to prevent finalize errors when initialize was not called
`components/eamxx/src/dynamics/homme/eamxx_homme_process_interface.cpp`	Tagged internal fields with RESTART group and fixed grid name in field access
`components/eamxx/src/control/atmosphere_driver.cpp`	Moved tendency setup after field creation and improved restart group handling
`components/eamxx/cime_config/testdefs/testmods_dirs/eamxx/prod/yaml_outs/eamxx_output.decadal.4hourlyAVG_coarse.yaml`	Added new tendency output field
`components/eamxx/cime_config/testdefs/testmods_dirs/eamxx/prod/shell_commands`	Updated tendency configuration with grid-specific field requests
`components/eamxx/cime_config/namelist_defaults_eamxx.xml`	Updated documentation to clarify groups support

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

components/eamxx/src/share/atm_process/atmosphere_process.cpp

Co-authored-by: Copilot <[email protected]>

tcclevenger · 2025-10-29T20:39:22Z

components/eamxx/cime_config/testdefs/testmods_dirs/eamxx/prod/shell_commands

 $atmchange -b physics::mac_aero_mic::p3::compute_tendencies=T_mid,qv
 $atmchange -b physics::rrtmgp::compute_tendencies=T_mid
-$atmchange -b homme::compute_tendencies=T_mid,qv
+$atmchange -b homme::compute_tendencies=T_mid,qv#physics_pg2,qc#physics_pg2


Is this #... the syntax, or a comment? Seems like syntax, but I would think physics pg2 would be default grid.

It is syntax. I can replace with a different character (e.g., qv@physics_pg2), to avoid confusing with a bash comment.

The issue is that homme has 2 copies of qv and qc, one on dyn grid, and one on pg2 grid (actually 3, if we count also the physics_gll grid). When requesting to compute tendencies, we must help atm proc to disambiguate. While pg2 is the "default" grid, the atm proc class has no knowledge about what the default grid is (or even that there is a "default" grid).

Gotcha. I would prefer @ but don't have a strong preference.

is this going to be needed? can we make the physics (pg2) one the default?

As of now, there is no way for an atm proc to know a priori the grids names, or that one name is "the default". Although all procs do communicate via physics grid fields, so I suppose we could assume "physics" is the default. I need to consider whether or not there may be hidden ramifications. If not, I can add it to this PR.

Gotcha. I would prefer @ but don't have a strong preference.

That's my preference too. I almost added a commit changing # to @, but refrained from doing it, thinking about bwd compatibility. Alas, I think nobody is using (or even has used) this feature, so maybe it's still a good time to change notation.

components/eamxx/src/dynamics/homme/eamxx_homme_process_interface.cpp

bartgol · 2025-10-30T16:08:10Z

@tcclevenger @mahf708 I made the modification you requested. Defaulting to the physics grid was harder than I thought, as the base class has no knowledge of the physics grid, and the actual name of the grid (physics_gll or physics_pg2) is what is used when field IDs are created. I ended up having the driver passing the name of the "default" grid to the tend setup method. I'm not fond of this, but the alternative was to make the base class store a handle to the physics grid, which would require making set_grids similar to init/run/finalize (method non virtual, calling the virtual xyz_impl inside). Tbh, I don't think the name "set_grids" is apt for what the derived classes do. Perhaps it should be called "create_fields_requests" or something like that.

Requires a bit more work to provide the ACTUAL name of the physics grid to the atm process

Copilot

Pull Request Overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (5)

components/eamxx/src/share/atm_process/atmosphere_process.cpp:935

Corrected spelling of 'proces' to 'process'.

        "Error! Could not locate input field in this atm proces.\n"

components/eamxx/src/share/atm_process/atmosphere_process.cpp:959

Corrected spelling of 'proces' to 'process'.

        "Error! Could not locate input field in this atm proces.\n"

components/eamxx/src/share/atm_process/atmosphere_process.cpp:975

Corrected spelling of 'proces' to 'process'.

        "Error! Could not locate output field in this atm proces.\n"

components/eamxx/src/share/atm_process/atmosphere_process.cpp:999

Corrected spelling of 'proces' to 'process'.

        "Error! Could not locate output field in this atm proces.\n"

components/eamxx/src/share/atm_process/atmosphere_process.cpp:1015

Corrected spelling of 'proces' to 'process'.

        "Error! Could not locate input group in this atm proces.\n"

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

components/eamxx/src/dynamics/homme/eamxx_homme_process_interface.cpp

tcclevenger

Yeah, this is ok. Also, completely agree on the set_grids name, should be something to do with requesting fields.

Co-authored-by: Copilot <[email protected]>

mahf708

Thank you!!!

bartgol · 2025-10-30T18:08:56Z

All DIFFs are expected. Merging.

Allow calculation also of tendency of groups monolithic/individual fields. [BFB] except for newly added var to eamxx-prod output.

bartgol added 2 commits October 22, 2025 12:25

EAMxx: minor upgrade to driver related to internal field groups

875f7c8

Do not add ALL fields to the restart group. Instead, atm procs should specify what groups the internal fields belong to (if any), and the driver will add to all those groups in the field manager

EAMxx: allow to retrieve groups fields via get_field calls in atm proc

f3cad12

For both input and output fields. And for both individual and monolithic field (if present).

bartgol self-assigned this Oct 22, 2025

bartgol added BFB PR leaves answers BFB EAMxx C++ based E3SM atmosphere model (aka SCREAM) labels Oct 22, 2025

bartgol force-pushed the bartgol/eamxx/step-tends-refactor branch from 4f27762 to f61b705 Compare October 22, 2025 20:17

bartgol added 2 commits October 22, 2025 16:31

EAMxx: fix handling of groups for fields added to FM

11a1596

bartgol force-pushed the bartgol/eamxx/step-tends-refactor branch from f61b705 to 11a1596 Compare October 22, 2025 22:38

bartgol added 6 commits October 23, 2025 12:11

EAMxx: fix step-tendencies setup for atm proc group

1ee7624

Must also process its own tendencies requests

EAMxx: fix homme tendency request in eamxx-prod testmod

50b8cfc

EAMxx: fix usage of qv field in homme interface

a040c0a

There are 2 copies of qv, so we must be explicit on which one we want

EAMxx: fix a couple of RESTART-related issues in homme process

b5d0c7b

EAMxx: fix rrtmgp finalization

b7588ce

Do not finalize if it was never inited. Avoids throwing while unwinding stack after uncaught exception

EAMxx: fix wrong check in driver restart_model fcn

be5278b

EAMxx: do not add grid name to tend output field name

0f7bd84

bartgol force-pushed the bartgol/eamxx/step-tends-refactor branch from 00673e8 to 0f7bd84 Compare October 29, 2025 18:44

bartgol requested review from mahf708 and tcclevenger and removed request for tcclevenger October 29, 2025 19:44

mahf708 requested a review from Copilot October 29, 2025 20:23

Copilot AI reviewed Oct 29, 2025

View reviewed changes

components/eamxx/src/share/atm_process/atmosphere_process.cpp Outdated Show resolved Hide resolved

EAMxx: fix bug in atm process internals setup

f377f07

Co-authored-by: Copilot <[email protected]>

tcclevenger reviewed Oct 29, 2025

View reviewed changes

components/eamxx/src/dynamics/homme/eamxx_homme_process_interface.cpp Show resolved Hide resolved

tcclevenger approved these changes Oct 29, 2025

View reviewed changes

EAMxx: change syntax to request atm proc tendencies on specific grid

7974d3d

EAMxx: default atm proc tendencies calculation to physics grid

669b528

Requires a bit more work to provide the ACTUAL name of the physics grid to the atm process

bartgol force-pushed the bartgol/eamxx/step-tends-refactor branch from b7d91c1 to 669b528 Compare October 30, 2025 16:20

bartgol requested a review from tcclevenger October 30, 2025 16:26

mahf708 requested a review from Copilot October 30, 2025 16:41

Copilot AI reviewed Oct 30, 2025

View reviewed changes

components/eamxx/src/dynamics/homme/eamxx_homme_process_interface.cpp Outdated Show resolved Hide resolved

tcclevenger approved these changes Oct 30, 2025

View reviewed changes

EAMxx: minor code cleanup

2e9b7fa

Co-authored-by: Copilot <[email protected]>

mahf708 approved these changes Oct 30, 2025

View reviewed changes

tcclevenger approved these changes Oct 30, 2025

View reviewed changes

bartgol added a commit that referenced this pull request Oct 30, 2025

Merge branch 'bartgol/eamxx/step-tends-refactor' into next (PR #7824)

61d4399

Allow calculation also of tendency of groups monolithic/individual fields. [BFB] except for newly added var to eamxx-prod output.

bartgol merged commit c578206 into master Oct 30, 2025
13 of 20 checks passed

bartgol deleted the bartgol/eamxx/step-tends-refactor branch October 30, 2025 18:11

mahf708 mentioned this pull request Nov 4, 2025

EAMxx: 'computable or allow_invalid_fields' error for shared instance #7859

Closed

EAMxx: upgrade/simplify step tendency calculation in atm procs #7824

EAMxx: upgrade/simplify step tendency calculation in atm procs #7824

Uh oh!

Conversation

bartgol commented Oct 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Oct 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Built to branch gh-pages at 2025-10-30 16:57 UTC. Preview will be ready when the GitHub Pages deployment is complete.

Uh oh!

bartgol commented Oct 29, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

tcclevenger Oct 29, 2025

Choose a reason for hiding this comment

Uh oh!

bartgol Oct 29, 2025

Choose a reason for hiding this comment

Uh oh!

tcclevenger Oct 29, 2025

Choose a reason for hiding this comment

Uh oh!

mahf708 Oct 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bartgol Oct 30, 2025

Choose a reason for hiding this comment

Uh oh!

bartgol Oct 30, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

bartgol commented Oct 30, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

tcclevenger left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mahf708 left a comment

Choose a reason for hiding this comment

Uh oh!

bartgol commented Oct 30, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

bartgol commented Oct 22, 2025 •

edited

Loading

github-actions bot commented Oct 29, 2025 •

edited

Loading

Built to branch `gh-pages` at 2025-10-30 16:57 UTC.
Preview will be ready when the GitHub Pages deployment is complete.

mahf708 Oct 29, 2025 •

edited

Loading

tcclevenger left a comment •

edited

Loading