Skip to content

Emergency fix: exponentiation instead of multiplication caused bad 2m temperatures#940

Merged
SamuelTrahanNOAA merged 1 commit into
NCAR:mainfrom
SamuelTrahanNOAA:catastrophic-typo
Jun 14, 2022
Merged

Emergency fix: exponentiation instead of multiplication caused bad 2m temperatures#940
SamuelTrahanNOAA merged 1 commit into
NCAR:mainfrom
SamuelTrahanNOAA:catastrophic-typo

Conversation

@SamuelTrahanNOAA
Copy link
Copy Markdown
Contributor

Emergency bugfix for the MYNN surface layer scheme:

 TH1D(I)=T1D(I)**(100000./P1D(I))**ROVCP !(Theta, K)

The first ** should be a *

 TH1D(I)=T1D(I)*(100000./P1D(I))**ROVCP !(Theta, K)

@ligiabernardet
Copy link
Copy Markdown
Collaborator

Do we need this bug fix in the CCPP v6 and SRW App v2 release codes?

@SamuelTrahanNOAA
Copy link
Copy Markdown
Contributor Author

Yes.

@SamuelTrahanNOAA
Copy link
Copy Markdown
Contributor Author

That "Yes" referred to "Do we need this bug fix in the CCPP v6 and SRW App v2 release codes?"

Copy link
Copy Markdown
Collaborator

@grantfirl grantfirl left a comment

Choose a reason for hiding this comment

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

@SamuelTrahanNOAA Can you please make a PR into release/public-v6 too?

@SamuelTrahanNOAA
Copy link
Copy Markdown
Contributor Author

Good news: the release-v6 has an older calculation, which lacks this bug.

@SamuelTrahanNOAA
Copy link
Copy Markdown
Contributor Author

SamuelTrahanNOAA commented Jun 10, 2022

This is in the release:

         ! CONVERT LOWEST LAYER TEMPERATURE TO POTENTIAL TEMPERATURE:                                                                                                                                                                          
         TH1D(I)=T1D(I)*THCON(I)                !(Theta, K)                                                                                                                                                                                    
         TC1D(I)=T1D(I)-273.15                  !(T, Celsius)                                                                                                                                                                                  

which is correct.

EDIT: I copied the wrong lines.

@grantfirl
Copy link
Copy Markdown
Collaborator

@SamuelTrahanNOAA Awesome, thanks for checking.

@dustinswales
Copy link
Copy Markdown
Member

@SamuelTrahanNOAA
I saw that the release branch is a tad bit different than the main branch with respect to mynnedmf_wrapper.F90.
The argument aren't the same between the two.
Looking at their histories, they seem to have diverged at some point:
https://github.com/NCAR/ccpp-physics/commits/release/public-v6/physics/mynnedmf_wrapper.F90
https://github.com/NCAR/ccpp-physics/commits/main/physics/mynnedmf_wrapper.F90

@grantfirl
Copy link
Copy Markdown
Collaborator

grantfirl commented Jun 10, 2022

@SamuelTrahanNOAA I saw that the release branch is a tad bit different than the main branch with respect to mynnedmf_wrapper.F90. The argument aren't the same between the two. Looking at their histories, they seem to have diverged at some point: https://github.com/NCAR/ccpp-physics/commits/release/public-v6/physics/mynnedmf_wrapper.F90 https://github.com/NCAR/ccpp-physics/commits/main/physics/mynnedmf_wrapper.F90

@dustinswales This is by "design" since the release branch doesn't contain the GSL "big merge" changes which is where the differences in mynnedmf_wrapper come from. After the release, the two will be reconciled, bringing the release branch changes into main (which is mainly just documentation updates).

@ligiabernardet ligiabernardet added bug CCPP v6 Needed for CCPP v6 public release labels Jun 10, 2022
@SamuelTrahanNOAA
Copy link
Copy Markdown
Contributor Author

@dustinswales Generally, the releases branch off of a stable-looking point in the authoritative version, and add only bug fixes and critical features. The v6 release lacked the change set that added this bug since those were new features not yet stable enough for the release.

@SamuelTrahanNOAA SamuelTrahanNOAA removed the CCPP v6 Needed for CCPP v6 public release label Jun 10, 2022
@dustinswales
Copy link
Copy Markdown
Member

@SamuelTrahanNOAA I saw that the release branch is a tad bit different than the main branch with respect to mynnedmf_wrapper.F90. The argument aren't the same between the two. Looking at their histories, they seem to have diverged at some point: https://github.com/NCAR/ccpp-physics/commits/release/public-v6/physics/mynnedmf_wrapper.F90 https://github.com/NCAR/ccpp-physics/commits/main/physics/mynnedmf_wrapper.F90

@dustinswales This is by "design" since the release branch doesn't contain the GSL "big merge" changes which is where the differences in mynnedmf_wrapper come from.

@grantfirl
Thanks for the explanation. I wasn't aware that all of the GSL development was to be excluded from the v6 release. Admittedly I'm a little bit spun around on what is going where (Other than knowing where RRTMGP is not going...)

@SamuelTrahanNOAA
Copy link
Copy Markdown
Contributor Author

Thanks for the explanation. I wasn't aware that all of the GSL development was to be excluded from the v6 release. Admittedly I'm a little bit spun around on what is going where (Other than knowing where RRTMGP is not going...)

Recent developments aren't going into the release, except for bug fixes. The release branch started off of a certain point in main and only adds bug fixes after that. That's how stable releases work.

@dustinswales
Copy link
Copy Markdown
Member

Thanks for the explanation. I wasn't aware that all of the GSL development was to be excluded from the v6 release. Admittedly I'm a little bit spun around on what is going where (Other than knowing where RRTMGP is not going...)

Recent developments aren't going into the release, except for bug fixes. The release branch started off of a certain point in main and only adds bug fixes after that. That's how stable releases work.

I understand, just trying to follow along. I wasn't aware that the release branch and main diverged recently...

@SamuelTrahanNOAA SamuelTrahanNOAA merged commit 9cfe75e into NCAR:main Jun 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants