Dont assume that MSLP and theta ref pressure are the same

[trontrytel]

We were assuming that mean sea level pressure (MSLP) and the reference pressure used in potential temperature are the same. This is technically not a mistake. It is causing problems in ClimaAtmos where we were overwriting MSLP=101325 value to be 100000 hPa for some of our cases but not for others.

I'm hoping that differentiating between the two would help us get rid of those overwrites

I kept the MSLP in temperature profiles and in the specific_entropy_ reference pressure. Lucky for us entropy is not used in ClimaAtmos, so it doesn't really matter.

[trontrytel]

How set in stone is this number in the test?

  Expression: count(air_temperature.(param_set, ts_mid) .== Ref(_T_freeze)) ≥ 1373
   Evaluated: 1296 ≥ 1373

[charleskawczynski]

How set in stone is this number in the test?

Yeah, that was a bit of a kludge. We should really be testing this: https://github.com/CliMA/Thermodynamics.jl/blob/main/test/relations.jl#L837. I forget exactly why that wasn’t working.

[tapios]

This looks good. It's good to separate the different reference pressures playing different roles in thermodynamic functions.

Given that MSLP is only used as the reference pressure for entropies now, could you please rename this to p_ref_entropy or something like it, to make clear what role it plays?

[charleskawczynski]

My only concern is, is it possible that this opens the door for inconsistencies? Otherwise it seems good.

[charleskawczynski]

Also, I agree that this is a breaking change, so will need to bump the minor version

[trontrytel]

Given that MSLP is only used as the reference pressure for entropies now, could you please rename this to p_ref_entropy or something like it, to make clear what role it plays?

@tapios - I changed the name in Thermodynamics but kept the MSLP parameter in CLIMAParameters. Or would you rather I also change the name in CLIMAParameters?

We use MSLP in a couple of places in ClimaAtmos. In some of those places it should now be replaced with p_ref_theta. I wanted to do that after I update Thermodynamics.

We use MSLP in ClimaAtmos to:

• to compute the reference density: ᶜρ_ref = @. MSLP * exp(-grav * ᶜcoord.z / (R_d * T_ref)) / (R_d * T_ref) (don't have to be changed)
• in initial profiles (I think in some of those it should be changed to ref_p_theta, but not all)
• in HeldSuarez forcing (I think it should be changed for theta forcing, but not needed for energy)
• in implicit tendency wfact for theta (should be changed to p_ref_theta)

[tapios]

Given that MSLP is only used as the reference pressure for entropies now, could you please rename this to p_ref_entropy or something like it, to make clear what role it plays?

@tapios - I changed the name in Thermodynamics but kept the MSLP parameter in CLIMAParameters. Or would you rather I also change the name in CLIMAParameters?

We use MSLP in a couple of places in ClimaAtmos. In some of those places it should now be replaced with p_ref_theta. I wanted to do that after I update Thermodynamics.

We use MSLP in ClimaAtmos to:

• to compute the reference density: ᶜρ_ref = @. MSLP * exp(-grav * ᶜcoord.z / (R_d * T_ref)) / (R_d * T_ref) (don't have to be changed)
• in initial profiles (I think in some of those it should be changed to ref_p_theta, but not all)
• in HeldSuarez forcing (I think it should be changed for theta forcing, but not needed for energy)
• in implicit tendency wfact for theta (should be changed to p_ref_theta)

Of course, please do leave MSLP as a parameter if it's used for initial conditions etc. in ClimaAtmos. Please make the changes to p_ref_theta in ClimaAtmos where needed to disambiguate the parameters.

[codecov]

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (f7d5775) 93.35% compared to head (7056443) 93.35%.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #151   +/-   ##
=======================================
  Coverage   93.35%   93.35%           
=======================================
  Files           9        9           
  Lines        1114     1114           
=======================================
  Hits         1040     1040           
  Misses         74       74           

Files Changed	Coverage Δ
src/Parameters.jl	93.33% <ø> (ø)
src/isentropic.jl	100.00% <100.00%> (ø)
src/relations.jl	96.57% <100.00%> (ø)

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

[trontrytel]

bors r+

[bors]

Configuration problem:
bors.toml: not found

[trontrytel]

bors try

[bors]

try

Configuration problem:
bors.toml: not found

[charleskawczynski]

Ah, right, we switched to the GH merge queue

[charleskawczynski]

Let's try