Add ellipsoid class #5

gmgunter · 2021-02-26T22:48:57Z

This PR demos a class for representing geodetic reference ellipsoids. In isce3, this is the role of the Ellipsoid class. The main differences between this implementation and the one in isce3 are:

Renamed "Ellipsoid" -> "Spheroid"
- The shape that we typically refer to as an "ellipsoid" is actually a particular type of ellipsoid called a "spheroid". While an ellipsoid has 3 axes of independent lengths, two of the axes of a spheroid are always of equal length.
Changed constructor arguments
- isce3's Ellipsoid is constructed by passing a semi-major axis length and eccentricity constant. My understanding is that it's more common in literature to describe geodetic reference ellipsoids by their semi-major axis length and flattening (or inverse flattening) coefficient, so I've changed the constructor to be consistent with that convention.
Removed default constructor
- In isce3, Ellipsoid can be default constructed to create a WGS 84 ellipsoid. This has been removed and replaced with a global constant called wgs84_ellipsoid.

Some of the higher-level, more complicated functionality of Ellipsoid has also been removed here to just focus on the basics. (Almost) all of the remaining interface is now constexpr.

r-burns

TIL longbois are called "prolate spheroids"

ellipsoid/v1/caesar/spheroid.hpp

r-burns

nits

Looks pretty great overall, gonna try to use this for geocoding later

r-burns · 2021-03-16T04:15:29Z

ellipsoid/v1/caesar/spheroid.hpp

+     * \see Spheroid::flattening
+     */
+    [[nodiscard]] constexpr double
+    inverse_flattening() const


Suggested change

inverse_flattening() const

inverse_flattening() const noexcept

r-burns · 2021-03-16T04:15:33Z

ellipsoid/v1/caesar/spheroid.hpp

+    [[nodiscard]] constexpr double
+    third_flattening() const noexcept
+    {
+        return f() / (2. - f());


Suggested change

return f() / (2. - f());

return squared_eccentricity();

right?

Oh wow you're right lol. I must've grabbed the formulas from two different places and not realized they were equivalent.

I think I'll probably drop the squared_eccentricity() method then and just keep third_flattening() since that name appears to be widely used in literature, whereas I only see "squared eccentricity" in isce. I'll add some documentation to make the relationship to eccentricity() clear, though.

r-burns · 2021-03-16T04:16:17Z

ellipsoid/v1/caesar/spheroid.hpp

+     * \f]
+     */
+    [[nodiscard]] double
+    eccentricity() const


Suggested change

eccentricity() const

eccentricity() const noexcept

std::sqrt itself isn't noexcept so I think there's some danger of nasal demons if we assert to the compiler that this function may not throw.

Now that you mention it, I think it would be possible for a prolate Spheroid to have negative squared_eccentricity, which would cause a domain_error here. It looks like we should be returning an Expected<double> instead of double here, and checking for the case where the argument to std::sqrt would be negative. Then it could be made noexcept.

Huh you're right, that's odd that it isn't noexcept. I assumed it was, after reading that even bad inputs will just return nan or rounded values. Does that mean that prolate spheroids have imaginary eccentricity then? Lol, weird

Does that mean that prolate spheroids have imaginary eccentricity then?

According to this, squared eccentricity is always positive, so I guess I was wrong. For a prolate spheroid, you flip the position of a and b in the equation. In other words, eccentricity is defined in terms of the semi-major and semi-minor axis lengths instead of a and b.

I think that also means that flattening should always be positive, in contradiction with what I wrote in the class docstring. Oops.

gmgunter

Thanks for the feedback @r-burns! Think I noticed a few other issues as well because of your comments

gmgunter · 2021-03-16T21:08:06Z

ellipsoid/v1/caesar/spheroid.hpp

+    [[nodiscard]] constexpr double
+    third_flattening() const noexcept
+    {
+        return f() / (2. - f());


Oh wow you're right lol. I must've grabbed the formulas from two different places and not realized they were equivalent.

I think I'll probably drop the squared_eccentricity() method then and just keep third_flattening() since that name appears to be widely used in literature, whereas I only see "squared eccentricity" in isce. I'll add some documentation to make the relationship to eccentricity() clear, though.

gmgunter · 2021-03-16T21:08:10Z

ellipsoid/v1/caesar/spheroid.hpp

+     * \f]
+     */
+    [[nodiscard]] double
+    eccentricity() const


std::sqrt itself isn't noexcept so I think there's some danger of nasal demons if we assert to the compiler that this function may not throw.

Now that you mention it, I think it would be possible for a prolate Spheroid to have negative squared_eccentricity, which would cause a domain_error here. It looks like we should be returning an Expected<double> instead of double here, and checking for the case where the argument to std::sqrt would be negative. Then it could be made noexcept.

gmgunter · 2021-03-17T05:49:50Z

ellipsoid/v1/caesar/spheroid.hpp

+     * \param[in] a semi-major axis length
+     * \param[in] f flattening constant
+     */
+    constexpr Spheroid(double a, double f) noexcept : a_(a), f_(f) {}


This is missing some error-checking. a must be > 0 and f must be < 1.

gmgunter · 2021-03-17T05:54:40Z

ellipsoid/v1/caesar/spheroid.hpp

+     * \see Spheroid::a
+     */
+    [[nodiscard]] constexpr double
+    semimajor_axis() const noexcept


Hmm, this isn't quite right either. By definition, semimajor_axis should be the larger of a or b. Setting it always equal to a only works for oblate spheroids.

Seems like some celestial objects are actually prolate spheroids, so it'd be worthwhile making this work correctly. I'll have to update the documentation, too.

r-burns

Testing this out on my geocoding branch, with a small addition for LLH to ECEF conversion: r-burns@976b09c
Works great!

Add Spheroid

60ed06b

r-burns reviewed Feb 26, 2021

View reviewed changes

ellipsoid/v1/caesar/spheroid.hpp Outdated Show resolved Hide resolved

Liberally sprinkle [[nodiscard]]

7871d03

r-burns reviewed Mar 16, 2021

View reviewed changes

gmgunter commented Mar 17, 2021

View reviewed changes

r-burns approved these changes Apr 11, 2021

View reviewed changes

	inverse_flattening() const
	inverse_flattening() const noexcept

Add ellipsoid class #5

Are you sure you want to change the base?

Add ellipsoid class #5

Uh oh!

Conversation

gmgunter commented Feb 26, 2021

Uh oh!

r-burns left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

r-burns left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

r-burns Mar 17, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gmgunter left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

r-burns left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

r-burns Mar 17, 2021 •

edited

Loading