-
Notifications
You must be signed in to change notification settings - Fork 446
Fix indexing for tau in gw_front subroutine #7817
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
Conversation
375bd11 to
3bed466
Compare
jgfouca
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for taking a deeper look at this to see if the climate changes made sense! I rebased and updated the C++ to match the fixed fortran.
jjbenedict
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I only checked the sign change in the Fortran code. The change seems warranted, and clearly gives a different answer in the test simulation. Variable fav is set in gw_front_init and is defined as "Average value of gaussian over gravity wave spectrum bins, multiplied by background source strength (taubgnd)". Based on my limited search, fav is only a function of wavenumber. I wondered if fav is symmetric, such that the sign in front wouldn't matter... but apparently it is not symmetric. In any case, the code modifications seems justified.
|
I'm going to continue for another 4 years and run the E3SM diags |
|
I ran a pair of 5-year runs to examine the impact of this bug on the long-term solution, and there are some small, but systematic changes, especially in polar regions. The sea-level pressure map is one area that this crops up - the various zonal mean plots are another good one to look at. Despite those changes, the TOA energy fluxes don't seem to be qualitatively affected - so while this is an "answer changing" fix, it's not the sort of change that would necessitate a major retuning. E3SM Diags
|
|
Adding @crterai @xuezhengllnl and @wlin7 for review of this "slightly answer changing" bug fix. In a recent discussion of this the issue how this might impact the HR runs came up. It seems that we might want to wait to merge this until after a tag has been created that reflects the state of the model that was used for the HR simulation campaign. Although, I'm a bit confused about whether that has already happened yet or not. |
|
The freeze on science changes for production configs is in effect. So yes it will be up to the v3HR, BlueTip and HES leads on if they want this in 3.1. |
bpbond
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the clear documentation and assessment.
|
Jack Chen at NCAR is now insisting that this is NOT a bug... but his reasoning would imply that changing the code should not lead to any real change in the model.. but this doesn't make sense with how things are coded. So I need to take some time to examine the NCAR code and check a few things. |
|
After further consideration I'm leaning towards concluding that this was not actually a bug. I'm still unclear on several details, but I'm going to assume that all the differences I found in my tests were just a fluke, and that the systematic differences just showed up by chance. Therefore, I'm going close the PR and potentially revisit in the future. |
|
I'm confused. I thought someone said yesterday that this was a bug that someone at NCAR fixed in their version of cam? |


This fixes a simple bug identified by Jim Foucar while porting the GW schemes to C++.
Fixes #7845
[non-BFB]