Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update gw_convect #6749

Merged
merged 8 commits into from
Nov 20, 2024
Merged

Update gw_convect #6749

merged 8 commits into from
Nov 20, 2024

Conversation

whannah1
Copy link
Contributor

@whannah1 whannah1 commented Nov 15, 2024

The PR makes several changes to the convective gravity wave methods. The primary motivation was to address a problem in gw_beres_src() where the diagnosed heating max/depth values were erroneously too low whenever the ZM heating profile included negative values (or zero) in the middle of the heating layer. Additionally, previously hard coded values were exposed as namelist values for a planned QBO tuning exercise for the QBO SciDAC project.

[BFB]
[NML] new nml variables for eam tests with default physics.

@whannah1 whannah1 added Atmosphere BFB PR leaves answers BFB labels Nov 15, 2024
@whannah1 whannah1 requested a review from jjbenedict November 15, 2024 20:37
Copy link

github-actions bot commented Nov 15, 2024

PR Preview Action v1.4.8
🚀 Deployed preview to https://E3SM-Project.github.io/E3SM/pr-preview/pr-6749/
on branch gh-pages at 2024-11-18 15:49 UTC

@whannah1 whannah1 force-pushed the whannah/update-gw-convect-src branch from dbc44a1 to 5c37d1b Compare November 15, 2024 20:43

subroutine gw_convect_init( plev_src_wind, mfcc_in, errstring)
use ref_pres, only: pref_edge
real(r8), intent(in) :: plev_src_wind ! previously hardcoded to 70000._r8
Copy link

@jjbenedict jjbenedict Nov 15, 2024

Choose a reason for hiding this comment

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

I'd add here in the comments that plev_src_wind should be in units Pa to match pref_edge. Maybe also want to mention that actual steering flow pressure level can differ from what the user requests wherever the reference pressure values differ from the actual pressure values (i.e. over mountainous terrain).

Copy link

@jjbenedict jjbenedict left a comment

Choose a reason for hiding this comment

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

Just a some minor comments, mainly to clarify units. I like the new loop scheme.

<entry id="gw_convect_hdepth_min" type="real" category="gw_drag"
group="gw_drag_nl" valid_values="" >
minimum hdepth for for convective GWD spectrum lookup table
Default: 2.5

Choose a reason for hiding this comment

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

Add units: km (presumably)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

real(r8), parameter :: AL = 1.0e5_r8
! fixed parameters (we may want to expose these in the namelist for tuning)
real(r8), parameter :: tau_avg_length = 1.0e5_r8 ! spectrum averaging length
real(r8), parameter :: heating_altitude_max = 20e3 ! max altitude to check heating (probably don't need this)

Choose a reason for hiding this comment

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

Add units to heating_altitude_max description. Also, 20000._r8?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed - the change in how this value is specified won't cause a BFB change.

@@ -118,6 +118,11 @@ module gw_drag
! namelist
logical :: history_amwg ! output the variables used by the AMWG diag package

logical :: use_gw_convect_old ! switch to enable legacy behavior
real(r8) :: gw_convect_plev_src_wind ! reference pressure level for source wind for convective GWD

Choose a reason for hiding this comment

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

Add that gw_convect_plev_src_wind must be in units Pa.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

Copy link
Contributor

Choose a reason for hiding this comment

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

@whannah1 , do we have simulations that documents the behavior with the new Beres scheme for heating depth etc. (i.e., use_gw_convect_old = .false.).

@@ -118,6 +118,11 @@ module gw_drag
! namelist
logical :: history_amwg ! output the variables used by the AMWG diag package

logical :: use_gw_convect_old ! switch to enable legacy behavior
real(r8) :: gw_convect_plev_src_wind ! reference pressure level for source wind for convective GWD
real(r8) :: gw_convect_hdepth_min ! minimum hdepth for for convective GWD spectrum lookup table

Choose a reason for hiding this comment

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

Add units for gw_convect_hdepth_min.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

logical :: use_gw_convect_old ! switch to enable legacy behavior
real(r8) :: gw_convect_plev_src_wind ! reference pressure level for source wind for convective GWD
real(r8) :: gw_convect_hdepth_min ! minimum hdepth for for convective GWD spectrum lookup table
real(r8) :: gw_convect_storm_speed_min ! minimum convective storm speed for convective GWD

Choose a reason for hiding this comment

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

Add expected units for gw_convect_storm_speed_min.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@whannah1
Copy link
Contributor Author

e3sm_atm_developer passes on Chrysalis, but it didn't do the baseline comparison (my mistake). I ran a single "SMS.ne4pg2_oQU480.F2010.chrysalis_intel.eam-cosplite" test with baseline comparison enabled and it passes - so my expectation of these changes being BFB holds up.

Copy link

@jjbenedict jjbenedict left a comment

Choose a reason for hiding this comment

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

The updates look good to me.

@wlin7 wlin7 added the NML label Nov 20, 2024
wlin7 added a commit that referenced this pull request Nov 20, 2024
Update gw_convect

The PR makes several changes to the convective gravity wave methods.
The primary motivation was to address a problem in gw_beres_src() where
the diagnosed heating max/depth values were erroneously too low whenever
the ZM heating profile included negative values (or zero) in the middle
of the heating layer. Additionally, previously hard coded values were exposed
as namelist values for a planned QBO tuning exercise for the QBO SciDAC project.

[BFB]
[NML] new nml variables for eam tests with default physics.
@wlin7
Copy link
Contributor

wlin7 commented Nov 20, 2024

Merged to next.

@wlin7 wlin7 merged commit de2142e into master Nov 20, 2024
13 of 24 checks passed
@wlin7 wlin7 deleted the whannah/update-gw-convect-src branch November 20, 2024 15:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Atmosphere BFB PR leaves answers BFB NML
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants