Add computation of horizontal components of gravitational acceleration

[santisoler]

Description of the desired feature

Point mass and tesseroid forward models are currently capable of computing only the potential and the vertical (or radial) component of the gravitational acceleration.
Would be nice to add the ability to compute the horizontal componentes, i.e. the easting and northing ones.

Are you willing to help implement and maintain this feature? Yes

[birocoles]

Hi @santisoler ,

I would like to add the other gravity components for point masses. I will start with point masses and learn the new features of Fatiando with you and @leouieda.

[santisoler]

Hi @birocoles. That sounds really nice! I'm glad to help you if you need so!

I agree we should start implementing only the horizontal components for the point masses, and leave the tesseroid case for the future.

I think you could start with the horizontal components of point masses in Cartesian coordinates. I think it's easier to code and test than the spherical coordinate case. Just remember that we are trying to avoid names like g_x and g_y, and more into naming them as g_northing and g_easting, or any names that make very clear which specific component we are computing. The only exception is for the g_z that always correspond to the downward component of the acceleration vector (both in Cartesian and spherical coordinates). See #89 for more information about why we chose this convention.

Feel free to open a PR anytime you want. You can add a WIP label on the PR title to flag it as a Work in Progress, so you'll make it clear that the branch is not ready to be merged.

[leouieda]

After #119, I started thinking about these forward functions. The size of point_mass_gravity is getting big and there aren't even any gradient components in there. I'm wondering how useful it is to have Cartesian and spherical in the same function. The point of being in the same function is that kernels can reuse some computations (which we don't do now).

What would you think of separating Cartesian and spherical into separate functions? Repeated checks can be moved to helper functions. That opens the way for ellipsoidal coordinate functions as well.

[birocoles]

Hi @leouieda and @santisoler

Sorry for closing the issue. It was a mistake.

I think separating the Cartesian and spherical stuff for point masses is a good idea. What do you mean by "Repeated checks can be moved to helper functions"? What methods will compose each function?

[leouieda]

What do you mean by "Repeated checks can be moved to helper functions"?

Things like checking inputs, broadcasting output arrays, etc, can be moved to a separate function and called from point_mass_gravity_spherical, etc.

What methods will compose each function?

What do you mean?

[birocoles]

I understood the following:

1. The point_mass_gravity in the point_mass.py would be moved to a new .py file. It contains the repeated checks.
2. There would be a new file called point_mass_gravity_cartesian.py containing the jit_point_mass_cartesian and all cartesian kernels (e.g., kernel_potential_cartesian).
3. There would be a new file called point_mass_gravity_spherical.py containing the jit_point_mass_spherical and all spherical kernels (e.g., kernel_g_z_spherical).

Is it right?

[leouieda]

No new files are needed. My argument was:

1. Separate the spherical and cartesian computations in different functions
2. Code that is repeated between these 2 functions should go into helper functions instead of being duplicated

[birocoles]

So, there will be a single file point_mass.py containing the two functions point_mass_gravity_cartesian and point_mass_gravity_spherical. Ok?

[leouieda]

That is what I had in mind. @santisoler what do you think about this?

[santisoler]

I think it's very annoying to pass the desired coordinate system as argument, in fact it could lead to major mistakes if we suddenly forget to pass it (and I think there's a big change to forget that argument). I'm not so sure about the proposed function names (seems too long), but maybe we could give it a try and then change it later.

I don't think we should define submodules for forward models on different coordinate systems because only point masses can be computed on Cartesian and spherical coordinates (prisms are only used on Cartesian, while tesseroids only on spherical). So maybe we can live with that function names (they are kind of ugly, but they serve a purpose).

[birocoles]

@leouieda and @santisoler , I will start creating the two functions point_mass_gravity_cartesian and point_mass_gravity_spherical in the file point_mass.py, ok?

[santisoler]

Hi @birocoles! For sure, feel free to open a PR whenever you're ready.

[birocoles]

I think this issue should be closed. Actually, the horizontal components of gravitational acceleration have already been implemented. I will create another issue for adding second derivatives of gravitational acceleration, ok?

[leouieda]

Agreed. No need to open an issue, though. Go straight for the PR instead. 👍