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

Change geopotential <-> height formulas to use standard gravity #1174

Merged
merged 1 commit into from Dec 24, 2019
Merged

Change geopotential <-> height formulas to use standard gravity #1174

merged 1 commit into from Dec 24, 2019

Conversation

jthielen
Copy link
Collaborator

Description Of Changes

As referenced in #1144 and #1115, pint's change in the value of G has been problematic for the tests of the geopotential <-> height functions.

It was suggested to change these functions to utilize standard gravity. However, after doing a deep dive into the original implementation and the finer points of gravity, it appears that the original implementation was based on assumptions of a spherical Earth and no centrifugal force effects. By basing these calculations on standard gravity instead (along with the below approximation for altitude adjustment), the formulation now includes the average effects of centrifugal force (by way of standard gravity itself), but still neglects latitudinal variation (which I think is acceptable?).

(from https://en.wikipedia.org/wiki/Gravity_of_Earth#Altitude as suggested in #663 (comment)). Combining this with the definition of geopotential from Wallace and Hobbs:

gives the formulation used here. It also happens to be the same formulation that arises if one uses the current implementation and assumes

(which is admittedly a poor way of looking at it, because standard gravity isn't just gravitational acceleration from Newton's law of universal gravitation, but it is still what I tried first before investigating the nuances referred to above.)

Note: this significantly changes the expected output of these functions!

Checklist

  • Tests added modified
  • Fully documented

@dopplershift
Copy link
Member

Yikes. We'll have to think about this some more. I'm hesitant to change the set of assumptions being made unless we have a good basis for it. I'm especially hesitant because of this quote from the wikipedia article:

For the purpose of satellite orbital mechanics, the geopotential is typically described by a series expansion into spherical harmonics (spectral representation). In this context the geopotential is taken as the potential of the gravitational field of the Earth, that is, leaving out the centrifugal potential.

@jthielen
Copy link
Collaborator Author

jthielen commented Sep 21, 2019

Yikes. We'll have to think about this some more. I'm hesitant to change the set of assumptions being made unless we have a good basis for it. I'm especially hesitant because of this quote from the wikipedia article:

For the purpose of satellite orbital mechanics, the geopotential is typically described by a series expansion into spherical harmonics (spectral representation). In this context the geopotential is taken as the potential of the gravitational field of the Earth, that is, leaving out the centrifugal potential.

I found that too with regards to the prior implementation, but that also could very well be a reason in itself as to why these functions should be changed in MetPy. Unlike in orbital mechanics as Wikipedia references, I've almost always seen centrifugal force included with gravity in dynamical equations in meteorology (especially with geopotential)...so I'd almost argue for this being the "more correct" solution compared to what we had before. (Although, to be fully correct, we'd probably need the full altitude and latitude formula from the AMS glossary/Smithsonian tables?)

But, I agree that thinking about this more would be good.

@kgoebber
Copy link
Collaborator

Having just recently lectured on the Newtonian equations of motion for NWP at the beginning of the semester, it is true that for dynamical meteorology it is the combination of standard gravity and centrifugal that is inherent within them.

@dopplershift dopplershift added this to the 0.12 milestone Oct 2, 2019
@jthielen
Copy link
Collaborator Author

jthielen commented Dec 23, 2019

The link check for https://www.weather.gov/media/ffc/ta_htindx.PDF seems to be causing the doc builds to fail somewhat often, but otherwise all the required Travis builds seem to be passing here. Some additional xarray errors have popped up on the master branch build that I can try looking into, but the tests relevant to this PR still seem to be passing.

@dopplershift
Copy link
Member

(Just to capture my thoughts for posterity...)

  • I like that this is the same formulation as if we used the simplifying assumption of g = GM/Re^2, but I'm much happier having the more theoretical basis behind it.
  • Including centrifugal force in when calculating the geopotential seems to be a good thing for meteorological use cases
  • These functions are now isolated from any further changes to G (according to the pint code standard gravity was defined in 1901)
  • We're dependent now on our value of Re, which comes from GEMPAK--that's both a blessing and a curse based on things like what I learned about GEMPAK's conversion factor for knots. (It's using an old standard). Defining a good value for Re is an entirely different dragon-infested rabbit hole.
  • I would love to have some actual canonical values for testing, but alas...
  • These functions keep the precision improvements we did in BUG: Fixup geopotential<->height calculation (Fixes #1075) #1082, if not improve them even further. Values also round-trip really well.
  • It seems like, despite there being some rather significant changes to the values, that these functions are more correct now.

Copy link
Member

@dopplershift dopplershift left a comment

Choose a reason for hiding this comment

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

Thanks for the hard work in researching and trying to understand what the right answer is here.

@dopplershift dopplershift merged commit 5c71f96 into Unidata:master Dec 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants