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

New ellipsoids #180

Merged
merged 30 commits into from
Apr 17, 2024
Merged

New ellipsoids #180

merged 30 commits into from
Apr 17, 2024

Conversation

MarkWieczorek
Copy link
Contributor

@MarkWieczorek MarkWieczorek commented Apr 2, 2024

The PR makes the following changes:

  • Added an optional comments attribute to all ellipsoid classes that is used when instantiating the ellipsoid classes. The default value is None. This attribute can provide additional information for understanding the origin of the numbers used to instantiate the class (for example, many papers provide several different versions of the ellipsoid, and you need to choose one).
  • Added semimajor_axis_longitude as an optional attribute that is used to instantiate triaxial ellipsoids. Previously, this parameter was found as an optional argument to the geocentric radius function. However, this parameter is an intrinsic property of the ellipsoid and the coordinate frame, and will be necessary for all routines that make use of longitude coordinates. The test_geocentric_radius_semimajor_axis_longitude was somewhat modified: I declared a new ellipsoid triaxialellipsoid_90 that set semimajor_axis_longitude to 90.
  • A large number of new ellipsoids were added, a few were renamed and the preexisting VESTA ellipsoid was removed (see comments below). Please note that all these ellipsoids are added to the main namespace. This might be too much of a mess for people, and I suggest that we instead put these all in a new module boule.ellipsoids. I will wait for your comments before doing so. Note that the ellipsoid year corresponds to the year when all the required data were published, or to the year that we assembled the ellipsoid ourselves using various sources.

Vesta Notes

I removed the original VESTA triaxial ellipsoid because there is not enough information in the paper to determine the GM, it is not stated if the ellipsoid is geocentric or offset from the center of mass, and the directions of the principal axes are not given. I think that keeping this ellipsoid as is will just cause problems for people who use it. I wasted a lot of time trying to figure this one out (and never did). Instead, two new ellipsoids have been added: The default (Vesta2017) is biaxial, as Vesta has no real tides. The Vesta2017_triaxial ellispoid, in contrast, is triaxial. The triaxial one, however, is not aligned with the topography and gravity coordinate axes, and this is specified by the new attribute semimajor_axis_longitude.

All Ellipsoids

Mercury

  • Mercury2015 (previously MERCURY, updated the last digit of GM that was incorrect)
  • Mercury2024 (new, updated R and GM)

Venus

  • Venus2015 (previously VENUS)

Earth

  • GRS80
  • WGS84
  • EGM96 (new)

Moon

  • Moon2015 (previously MOON)

Asteroids

  • Ceres2018 (new)
  • Vesta2017 (new)
  • Vesta2017_triaxial (new)
  • VESTA Removed (see comments)

Mars

  • Mars2009 (previously MARS)

Jupiter system

  • Io2024 (new)
  • Europa2024 (new)
  • Ganymede2024 (new)
  • Callisto2024 (new)

Saturn system

  • Enceladus2024 (new)
  • Titan2024 (new)

Pluto system

  • Pluto2024 (new)
  • Charon2024 (new)

Relevant issues/PRs:
Closes #176

Copy link

@VascoSch92 VascoSch92 left a comment

Choose a reason for hiding this comment

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

Nice :)

boule/_realizations.py Outdated Show resolved Hide resolved
@MarkWieczorek
Copy link
Contributor Author

Please merge #183 and #185 first. Afterwards, I will make minor changes to this PR that make use of the new comments and semimajor_axis_longitude atrributes.

boule/_realizations.py Outdated Show resolved Hide resolved
Copy link
Member

@leouieda leouieda left a comment

Choose a reason for hiding this comment

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

This is awesome! I pushed some minor fixes to the citations and renamed the Vesta triaxial to VestaTriaxial2017 to keep the CamelCase notation.

@leouieda
Copy link
Member

Please note that all these ellipsoids are added to the main namespace. This might be too much of a mess for people, and I suggest that we instead put these all in a new module boule.ellipsoids. I will wait for your comments before doing so.

I think this is fine. The API itself is only 3 classes so having all of these in the main namespace is OK.

@leouieda leouieda merged commit 16b4699 into fatiando:main Apr 17, 2024
15 checks passed
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.

New ellipsoids : Proposal
3 participants