search

NCAR / ccpp-physics

GFS physics for CCPP
Other
58 stars 145 forks source link

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

Closed SamuelTrahanNOAA closed 2 years ago

SamuelTrahanNOAA commented 2 years ago

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 commented 2 years ago

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

SamuelTrahanNOAA commented 2 years ago

Yes.

SamuelTrahanNOAA commented 2 years ago

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

SamuelTrahanNOAA commented 2 years ago

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

SamuelTrahanNOAA commented 2 years ago

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 commented 2 years ago

@SamuelTrahanNOAA Awesome, thanks for checking.

dustinswales commented 2 years ago

@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 commented 2 years ago

@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).

SamuelTrahanNOAA commented 2 years ago

@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.

dustinswales commented 2 years ago

@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 commented 2 years ago

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 commented 2 years ago

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...