EAMxx: water thermo fixer #7758

mahf708 · 2025-10-07T16:06:25Z

add "air sea surface water thermos fixer" (in EAM known as IEFLX) in the same general area as "energy fixer"

[NBFB] only for coupled eamxx cases such as WCYCLXX2010

mahf708 · 2025-10-07T16:33:30Z

the global mean addition is on the order of ~1 W/m2

> ncdump -v surf_sens_flux after.INSTANT.nsteps_x1.0001-01-01-34200.nc | tail -n5
    -10.04311, -7.123801, -8.322318, -9.511498, -8.958095, -8.3983, 
    -8.873889, -9.240752, -6.763078, -9.248022, -7.410153, -4.584584, 
    -3.634221, -3.310493, -5.005818, -3.691226, -3.295502, -4.835014, 
    -6.196012, -3.144907, -2.030783, -5.829425, -4.557174 ;
}
> ncdump -v surf_sens_flux before.INSTANT.nsteps_x1.0001-01-01-34200.nc | tail -n5
    -6.490936, -7.243723, -8.283883, -7.734644, -7.220661, -7.708688, 
    -8.046339, -5.529199, -8.085396, -6.212454, -3.280025, -2.231682, 
    -1.892398, -3.778452, -2.15747, -2.520945, -3.489566, -4.656339, 
    -2.323156, -1.165505, -4.645431, -3.324049 ;
}

jgfouca

LGTM, just a minor point

jgfouca · 2025-10-07T17:01:05Z

components/eamxx/src/share/property_checks/mass_and_energy_conservation_check.cpp

 }

-void MassAndEnergyConservationCheck::global_fixer(const bool & print_debug_info)
+void MassAndEnergyConservationCheck::global_fixer(const bool & print_debug_info, const bool & ieflx_fixer)


bools don't really need to be passed by ref. A const copy should be fine. Also, should ieflx_fixer be defaulted to false?

I followed the print_debug_info to keep the same style. Should I change both while at it?

whannah1 · 2025-10-07T17:41:42Z

components/eamxx/src/share/property_checks/mass_and_energy_conservation_check.cpp

+    });
+    Kokkos::fence();
+  } else {
+    m_h2otemp_global_mean = 0.0;


Shouldn't this return some invalid value like a negative?
Maybe I just haven't thought it through.

Well, this is the else-branch in applying the ieflx fixer. If we are not applying the fixer, then we are adding NOTHING (zero) to the sens flux. Does that sound reasonable to you? Btw, this only used for printing a debug statement in the logs (if the debug info is requested). Usually, this value is ~1 W/m2 in ne4 coupled cases (instant time step)

bartgol

Can we use a different name than ieflx_fixer? For someone that doesn't already know what it is, it conveys very little meaning. I assume flx stands for flux, but saving that one char doesn't really seem worth it. And maybe spell out what IE stands for too? I assume these are ported from existing pieces in EAM, but I think we can improve names.

mahf708 · 2025-10-07T23:38:31Z

Can we use a different name than ieflx_fixer? For someone that doesn't already know what it is, it conveys very little meaning. I assume flx stands for flux, but saving that one char doesn't really seem worth it. And maybe spell out what IE stands for too? I assume these are ported from existing pieces in EAM, but I think we can improve names.

Agree, ieflx stands for internal energy fixer and there's a lot of controversy about this --- in an ideal world, we won't have this stuff in the code. I will edit. Also, I am discussing with Mark and Oksana a potential change to the way we do this. Currently, it is carbon-copy from EAM, but this is a good time to consider improvements.

mahf708 · 2025-10-08T13:17:55Z

components/eamxx/cime_config/namelist_defaults_eamxx.xml


    <!-- Fields that we need to initialize, but are not in an initial condition file need to be inited here -->
    <surf_evap>0.0</surf_evap> <!-- TODO, Delete this when the IC fixes come in -->
+    <h2otemp>0.0</h2otemp> <!-- this is in W/m2, comes from ocean for water thermo fixer -->


@bartgol two questions

will adding this field here avoid fails in the standalone tests? I think they just need this field to be available somewhere, but I may have missed something

This is a weird name, so if you want to improve, let me know. This is what it is called in the coupler. It's the energy flux (W/m2) due to a mismatch in the temperature of water in the atmosphere and ocean, and so it is often thought of as a heat flux to compensate that; "h2o" is water and "temp" is temperature, I suppose, but I dislike the fact that the name implies that the term itself is a temperature in units of K

No: the xml defaults file is only used by CIME. You need to add a line with h20temp: 0.0 to all input.yaml files of tests that use this var.

If the name refers to a varname from the cpl, then i'm ok using it. My rant above was more for eamxx var/option names.

bartgol · 2025-10-08T17:07:03Z

components/eamxx/cime_config/namelist_defaults_eamxx.xml

           in-fields. <= 0 disables hashing. -->
      <enable_column_conservation_checks>false</enable_column_conservation_checks>
      <enable_energy_fixer type="logical" doc="Turn on energy fixer for dycore (fixes dycore, pressure adjustment, PD coupling)">true</enable_energy_fixer>
+      <enable_air_sea_surface_water_thermo_fixer type="logical" doc="Turn on energy fixer for inconsistent internal energy due to mismatch with ocean water temperature, works by adding to energy fixer above">true</enable_air_sea_surface_water_thermo_fixer>


Much better! Thanks!

due to mismatch with ocean water temperature should be due to mismatch with ocean water thermodynamics

mahf708 · 2025-10-08T20:18:25Z

components/eamxx/src/share/property_checks/mass_and_energy_conservation_check.cpp

+
+      // Add h2otemp contribution to energy change if air sea surface water thermo fixer is enabled
+      if (air_sea_surface_water_thermo_fixer && h2otemp_ptr != nullptr) {
+        energy_change(i) += h2otemp_ptr[i] * dt;


operating on the ptr was an artifact of an earlier impl. I will change this to view later --- thoughts @bartgol + @jgfouca?

Well, since the view will never be a subview of something else, using the raw ptr should yield the correct result (meaning there would be no funky offsets calc in a 1d view anyways).

That said, using a view may make it clearer. But you do need to decl the view before the if statement:

typename Field::view_dev<const Real*> h2otemp_v; if (...) { h2otemp_v = m_fields["h2otemp"].get_view<const Real*>(); }

It's not a big deal, but in case someone changes the code a bit, and messes with indices, the view approach may help with debugging, as we can turn on bounds checks.

mahf708 · 2025-10-09T15:36:17Z

This should only be merged after @oksanaguba approves. Please hold off merging. I will assign someone once Oksana reviews (and eventually approves).

oksanaguba · 2025-10-09T19:51:42Z

I made a minor change in your description's first line. For the second line, that this is nonbfb only for coupled runs -- I believe that IEFLX is used in old e3sm in AMIP runs, and i think its absence would be detected in long AMIP simulations in RESTOM-RESSURF value.

(not to delay this PR, maybe, for future tasks) Since there is no good test for IEFLX, it would be good to run at least diagnostics with its magnitude. For that one can disable energy fixer (just comment out in the code, because namelist change won't probably work) and run it. There is already some diagnostics (at least, T tendency) for the fixer, one should confirm that the magnitude of IEFLX is as expected (in range [-4,4] W/m2 at most, depending on time step, in 1 degree runs). The fixer without IEFLX (from old e3sm runs) is in range [-2,2] W/m2.

mahf708 · 2025-10-10T14:33:11Z

for @bartgol + @jgfouca, ready to merge, please feel free to merge (or request further edits) at your convenience

for @whannah1 + @crterai, if you're interested in the test Oksana mentions, we can conduct that later (I can write precise instructions on how to conduct it or I can do it myself). Up to you, but I view this as a secondary task to the PR itself, so we should integrate and then we can separately document the effect. For the most part this fixer shouldn't be active in in regular eamxx simulations we conduct nowadays, but will definitely be active in coupled cases

EAMxx: water thermo fixer add "air sea surface water thermos fixer" (in EAM known as IEFLX) in the same general area as "energy fixer" [NBFB] only for coupled eamxx cases such as WCYCLXX2010

mahf708 requested review from bartgol, crterai, jgfouca, mt5555, oksanaguba and whannah1 October 7, 2025 16:06

mahf708 added non-BFB PR makes roundoff changes to answers. EAMxx C++ based E3SM atmosphere model (aka SCREAM) labels Oct 7, 2025

jgfouca requested changes Oct 7, 2025

View reviewed changes

whannah1 reviewed Oct 7, 2025

View reviewed changes

bartgol requested changes Oct 7, 2025

View reviewed changes

mahf708 requested a review from amametjanov October 8, 2025 00:14

mahf708 force-pushed the mahf708/eamxx/ieflx branch from 3893fc0 to 621f0e2 Compare October 8, 2025 13:14

mahf708 changed the title ~~EAMxx: ieflx fixer~~ EAMxx: water thermo fixer Oct 8, 2025

mahf708 requested review from bartgol, jgfouca and whannah1 October 8, 2025 13:15

mahf708 commented Oct 8, 2025

View reviewed changes

EAMxx: expose h2otemp, add water thermo fixer

05f5ff9

mahf708 force-pushed the mahf708/eamxx/ieflx branch from 621f0e2 to 05f5ff9 Compare October 8, 2025 15:45

bartgol approved these changes Oct 8, 2025

View reviewed changes

mahf708 commented Oct 8, 2025

View reviewed changes

jgfouca approved these changes Oct 8, 2025

View reviewed changes

oksanaguba approved these changes Oct 9, 2025

View reviewed changes

EAMxx: slightly better docs for ieflx

72531cd

mahf708 assigned bartgol Oct 10, 2025

mahf708 assigned jgfouca Oct 10, 2025

jgfouca merged commit 339ba28 into master Oct 10, 2025
18 of 21 checks passed

jgfouca deleted the mahf708/eamxx/ieflx branch October 10, 2025 19:17

EAMxx: water thermo fixer #7758

EAMxx: water thermo fixer #7758

Conversation

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

Uh oh!

Uh oh!

mahf708 commented Oct 7, 2025

Uh oh!

jgfouca left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bartgol left a comment

Choose a reason for hiding this comment

Uh oh!

mahf708 commented Oct 7, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bartgol Oct 8, 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mahf708 commented Oct 9, 2025

Uh oh!

oksanaguba commented Oct 9, 2025

Uh oh!

mahf708 commented Oct 10, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

mahf708 commented Oct 7, 2025 •

edited by oksanaguba

Loading

bartgol Oct 8, 2025 •

edited

Loading