-
Notifications
You must be signed in to change notification settings - Fork 17
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
Fix normal gravity equation of Sphere #52
Conversation
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.
@santisoler maybe we should merge #51 first (with the failing test) and then use it here to make sure we're doing the right thing this time. What do you think?
Sure, I want to have the Somigliana test before merging this, so we can be sure that our normal gravity fix works fine. |
Bad news: Somigliana test fails 😞 |
Defines three different methods for computing normal gravity: - normal_gravity computes the radial component of the gradient of the combined potential (gravitational + centrifugal). - normal_gravity_norm computes the norm of the gradient of the combined potential. - normal_gravity_no_rotation computes the norm (equal to the radial component) of the gradient of the gravitational potential only (no rotating sphere). Add warning about using rotation spheres: no self equipotential surface and no hydrostatic equilibrium.
@MarkWieczorek I implemented some of the things we discussed on #50.
I don't have any use case for the last two, so I'm not entirely sure if they are needed, but I would do like |
Just some quick comments without looking at the code. As a reminder, this was what my suggestion was
If you want normal_gravity to agree with Somliagno's formula (in the limit of zero rotation and flattening), then you have to define normal gravity as in (1) above. I don't think that there is anyway to avoid this. I don't think that there is much use for returning only the radial component as in your first definition above. If you really need this, I would instead probably return the full acceleration vector. |
Thanks @MarkWieczorek for the quick response.
I respectfully disagree. Somigliana equation does take into account the rotation of the ellipsoid. On #50 I referenced Somigliana equation and the expression I got after replacing the parameters with the ones of a sphere: the gravitational attraction minus the radial component of the centrifugal acceleration. So if we want
I have never worked with spherical ellipsoids, maybe that's why I'm facing this issues now. I don't know which is the common usage of such ellipsoids, and how normal gravity is usually computed. What I do want is our Because other users might need other gravity computations, I though that would be clever to add them, like the norm of the gradient or the normal gravity without considering rotation. |
Here are some more comments on the way I see this. Its possible that we are using different definitions, so it probably best to start from the basics and to see at which point we disagree. Conceptual framework
Proposal 1 for Sphere class:
Proposal 2 for Sphere class:
|
Thanks @MarkWieczorek for the knowledge and the advices. I completely agree with the items of the conceptual framework. On this PR I will fix the |
Hi Mark, thanks for the input! A few thoughts: We seem to have different definitions of normal gravity here. We've been going with the one from physical geodesy "the magnitude of the gravity potential of a reference ellipsoid". This is also what is used in Geophysics for the Earth at least. There is no restriction on the computation point (e.g. surface) and it's gravity = gravitation + centrifugal. I'd like to keep things consistent and avoid having the ellipsoid return gravity and the sphere return gravitation. That is bound to be the source of bugs in the future. So this rules out the first part of proposal 1.
Sure, but the moon is rotating and the reference surface for it is a sphere. So in practice this doesn't seem to apply.
Again, we seem to be using conflicting definitions here. I like the idea of providing other methods for calculating the magnitude and vectors of gravitation and centrifugal. But we also need a normal_gravity method that is consistent with the ellipsoid definition. Otherwise, we can't calculate gravity anomalies and disturbances from observed gravity (gravitational + centrifugal). Unless the data for the Moon etc don't include the centrifugal components. In that case, there would be issues with the ellipsoids is an angular velocity is provided. |
Could you let me know the first numbered point that you disagree with? If it is number 1, could you let me know which part you disagree with?
I don't think that there is any disagreement here. The only possible difference is that my definition in (1) requires the reference ellipsoid to be hydrostatic (i.e., to correpsond to a constant gravitational potential, as in Somigliana's derivation). |
Hi @MarkWieczorek and @leouieda . After reading more and doing some math I have new thoughts about this. Firstly, I think we should stick with the gravity = gravitational + centrifugal definition. Therefore the normal gravity of a sphere must include a centrifugal component. Secondly, we can define the normal gravity as the norm of the gradient of the gravity potential (gravitational + centrifugal) of an equipotential ellipsoid of revolution. In the case of a rotating sphere is impossible to define normal gravity because a rotating sphere cannot be its own equipotential surface. So, in order to define a
Following my idea that we must consider the centrifugal component, I would go with the first option: ignoring the fact that the sphere is not its own equipotential surface. This opens another question. If we choose the first option, the My opinion
I think we should go with the first option and return the norm of the gradient because that's in agreement with the definition of normal gravity. I would like to hear your opinions on these @MarkWieczorek and @leouieda . Also would be nice to hear what @birocoles thinks of this. I'm open to suggestions, and remember that if any of you need a method that computes anything else (like the gravitational acceleration, for example) we can add it on a future PR as a separated method. |
Hi @santisoler , @leouieda and @MarkWieczorek , sorry for my delayed response. There is a lot of discussion here and I would like to give you my humble opinion. For me, the hydrostatic equilibrium is not relevant for geophysicists in this case. Actually, the geophysical normal Earth should be defined with the purposes of (1) keeping the difference from the actual gravity field as small as possible and (2) incorporate the internal layered structure of the Earth. It means that, for geophysical purposes, it is not necessary a normal Earth having a limiting surface defined by an equipotential of its own gravity field. There is a discussion about this topic in : Roman Pašteka, Ján Mikuška, and Bruno Meurers (2017). "Understanding the Bouguer Anomaly: A Gravimetry Puzzle". Elsevier. ISBN 978-0-12-812913-5. Based on these ideas, I think we should ignore that the sphere is not its own equipotential surface. |
Thanks @birocoles for your opinion! I ended up reverting the commits so If you all agree, I think we could merge this. As I said before, if any of us wants to add any other method that returns some other computation, we can do it on a separate PR. |
I agree with @birocoles and @santisoler on the definitions here. That makes the normal gravity of ellipsoids and spheres have the same definition and avoids confusion when switching from one to the other (users shouldn't have to care which one they use). For reference, we might want to add a note about this to the Normal Gravity documentation page and reference the Bouguer anomaly book. @MarkWieczorek we will make another issue to add a @santisoler I made a few changes to the docstrings but nothing major. Will merge as soon as CIs are happy. 👍🏽 |
Change how normal gravity of a sphere is computed.
It now computes it as the norm of the gradient of the combined potential (gravitational potential + centrifugal potential).
Fixes #50
Reminders:
make format
andmake check
to make sure the code follows the style guide.doc/api/index.rst
and the base__init__.py
file for the package.AUTHORS.md
file (if you haven't already) in case you'd like to be listed as an author on the Zenodo archive of the next release.