Skip to content

Conversation

@herrwang0
Copy link

@herrwang0 herrwang0 commented Mar 16, 2025

This PR adds a series of new diagnostics in both barotropic solver and pressure force modules. There is no answer change.

Issues with old KE budget

Currently, the kinetic energy budget is as follows:
dKE_dt = KE_Coradv + PE_to_KE + KE_BT + KE_visc + KE_horvisc

While this expression is algebraically correct, the physical meaning of some individual terms can be obscure.

  1. KE_BT is from the total acceleration from barotropic solver that includes anomalous pressure force, anomalous Coriolis force and when (tidal) linear wave drag is used, a barotropic wave drag term. In other words, this term has a well-defined numerical meaning but not a physical one.
  2. PE_to_KE is from PF[uv] in PressureForce module, which also includes the acceleration from tidal forcing and SAL.

New KE budget

New diagnostics are added to

  1. decompose and rearrange KE_BT.
  • KE_BT = KE_BTPF + KE_BTCF [+ KE_BTWD]
  • PE_to_KE_btbc = PE_to_KE + KE_BTPF
  • KE_Coradv_btbc = KE_Coradv + KE_BTCF
  1. single out tidal forcing and SAL contributions from PE_to_KE with terms KE_tides and KE_SAL.

With these terms, the new KE budget is (terms in the bracket are related to SAL, tides and linear wave drag, which may not also present)
dKE_dt = KE_Coradv_btbc + (PE_to_KE_btbc [-KE_SAL-KE_tides]) [+ KE_SAL] [+ KE_tides] + KE_visc + KE_horvisc [+ KE_BTWD]


This PR includes two commits:

11c5db4: Add decomposed KE contribution from BT solver, including from anomalous pressure force, anomalous Coriolis force and linear wave drag.

cb55280: Add in FV pressure force tides and SAL diagnostics (momentum acceleration and KE contribution)

@herrwang0 herrwang0 marked this pull request as draft March 16, 2025 06:45
@herrwang0 herrwang0 force-pushed the add_bt_energy_diag branch from cbe2304 to 953b20c Compare March 16, 2025 07:33
@codecov
Copy link

codecov bot commented Mar 16, 2025

Codecov Report

Attention: Patch coverage is 58.09129% with 101 lines in your changes missing coverage. Please review.

Project coverage is 38.23%. Comparing base (b232aba) to head (9b155a8).
Report is 2 commits behind head on dev/gfdl.

Files with missing lines Patch % Lines
src/core/MOM_PressureForce_FV.F90 29.85% 27 Missing and 20 partials ⚠️
src/core/MOM_barotropic.F90 40.90% 12 Missing and 14 partials ⚠️
src/diagnostics/MOM_diagnostics.F90 79.48% 7 Missing and 17 partials ⚠️
src/core/MOM_dynamics_split_RK2b.F90 0.00% 2 Missing ⚠️
src/core/MOM_PressureForce.F90 80.00% 1 Missing ⚠️
src/core/MOM_dynamics_split_RK2.F90 50.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##           dev/gfdl     #855      +/-   ##
============================================
+ Coverage     38.19%   38.23%   +0.04%     
============================================
  Files           300      300              
  Lines         88820    89022     +202     
  Branches      16757    16833      +76     
============================================
+ Hits          33921    34037     +116     
- Misses        48658    48702      +44     
- Partials       6241     6283      +42     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@herrwang0 herrwang0 marked this pull request as ready for review March 16, 2025 07:45
@herrwang0
Copy link
Author

herrwang0 commented Mar 16, 2025

It looks like the regression tests failed because the order of the diagnostics are changed in this PR. e_tidal, e_sal, e_tide_eq and e_tide_sal are moved after MassWt_[uv] in the PR.

Copy link
Member

@Hallberg-NOAA Hallberg-NOAA left a comment

Choose a reason for hiding this comment

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

I have examined the code changes in this PR, and they look correct to me. Thank you for these added diagnostic capabilities.

The red-X from the TC testing is because the order of some post_data() calls have been changed for reasons that make perfect sense. This is a false warning, and we have been discussing how to revise the TC testing diagnostics checks to use an appropriate sort to avoid false warnings such as this, but that is not available yet.

Unfortunately, because of the red-X I did not review this PR as quickly as I might have done, and in accepting PR #861 we have created some (minor) code conflicts in MOM_barotropic.F90 that will have to be resolved manually (sorry about that). Please rebase this PR on top of the latest dev/gfdl version of MOM6, @herrwang0, and we should be able to accept this PR and merge it in before creating the next set of conflicts with PR #863!

@herrwang0
Copy link
Author

@Hallberg-NOAA Thanks for the review! Alternatively, would it make sense to wait for PR #863 to be merged instead? As opposed to manually resolve the conflicts twice (once here and the other in PR #863), we can do it once for all. The changes to MOM_barotropic in this PR is quite moderate. It would be essentially the same amount of work for me and also allow adopting the new naming convention _sum -> _avg for diagnostics introduced here.

@Hallberg-NOAA
Copy link
Member

If this PR were to be rebased upon dev/gfdl now, I suspect that there would not be any further conflicts introduced by any of the subsequent expected commits.

@herrwang0 herrwang0 force-pushed the add_bt_energy_diag branch from 953b20c to cb55280 Compare March 26, 2025 21:32
@herrwang0
Copy link
Author

Tagging @Hallberg-NOAA, the PR is rebased for the newly refactored subroutine btstep().

@Hallberg-NOAA
Copy link
Member

We are working on revising the automated testing to avoid the false indication of changing diagnostics due to a simple change in the order with which diagnostics are posted, as in this case. We will retest and accept this PR once that capability is in place.

* In barotropic solver, add two diagnostics WaveDraguBT and WaveDragvBT
for 2D linear wave drag accelerations, which includes both traditional
and frequency-filtered.

* Add diagnostics of KE sources from decomposed barotropic solver
contribution, which are the anomalous pressure gradient term (KE_BTPF,
including the offset for center differencing in time), the anomalous
Coriolis term (KE_BTCF) and linear wave drag (KE_BTWD, when calculated).

* Add KE source diagnostics of combined PF and CA terms with both
barotropic and baroclinic contributions (PE_to_KE_btbc and
KE_Coradv_btbc)
* Add diagnostics "tides_[uv]" and "sal_[uv]" for momentum acceleration
due to tides and self-attraction and loading (SAL). These terms are
recalculated by taking the gradient of height anomalies.

* Accordingly, two new diagnostics for KE contributions from tidal
forcing ("KE_tides") and SAL ("KE_SAL") are added. These two terms are
components of PE_to_KE.

* The new diagnostics are only added to finite volume PGF and not
available in Montgomery PGF.
Copy link
Member

@Hallberg-NOAA Hallberg-NOAA left a comment

Choose a reason for hiding this comment

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

With our updated testing strategies, we can now confirm that this PR is passing the automated testing well enough to be able to accept it. Thank you for your patience.

@Hallberg-NOAA
Copy link
Member

This PR has passed pipeline testing at https://gitlab.gfdl.noaa.gov/ogrp/mom6ci/MOM6/-/pipelines/27137 with the expected warnings about new diagnostics.

@Hallberg-NOAA Hallberg-NOAA merged commit c23aad6 into NOAA-GFDL:dev/gfdl Apr 16, 2025
53 checks passed
@herrwang0 herrwang0 deleted the add_bt_energy_diag branch May 30, 2025 19:44
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