Adding support geodesic area and perimeter calculations from geographlib-rs

[phayes]

• [X] I agree to follow the project's code of conduct.

• [X] I added an entry to CHANGES.md if knowledge of this change could be valuable to users.

This is a PR for adding a GeodesicArea trait for highly accurate geodesic area and perimeter calculations.

See https://github.com/georust/geographiclib-rs/pull/49

[phayes]

This is now ready for merging / review.

An open question is whether we like what I've done with having a bunch of different methods for getting area, perimeter or both. Maybe we want to ignore perimeter and only focus on area? Maybe we always want to get both together since they're computed together?

[phayes]

I've added a bunch more tests to confirm that my "winding correction logic" is good and adjusted the epsilons. I would appreciate another test run on your local to confirm that my epsilons are good (I still think it's really weird that there's a difference in behavior between our machines!).

[michaelkirk]

I've added a bunch more tests to confirm that my "winding correction logic" is good and adjusted the epsilons.

Without the "winding correction logic" you added, the mis-wound holes would add to rather than subtract from the area, right?

That logic seems like it would be helpful for most people, and the tests give me confidence that you've implemented it correctly.

As devil's advocate, I'm trying to imagine if there's a world where people would intentionally wind their polygons this way, where the existing computation would be meaningful, and now we've inadvertently broken some special use case that these people might have... but I'm having a hard time imagining such a thing.

On balance, I think what you've done makes sense, and is a nice guard rail, so we should merge it.

I would appreciate another test run on your local to confirm that my epsilons are good

Passing now — Thanks!

(I still think it's really weird that there's a difference in behavior between our machines!).

I'm no expert on floating point implementations, but it's my understanding that there are small discrepancies across architectures and compilers. Some operations are encompassed by the IEEE754 spec, and should perform uniformly, but some operations (e.g. trigonometric functions) are not and might diverge.

I don't know if that's what's going on here, but it's common enough that if the divergence is very small, I've chosen to add a small epsilon for testing and spend my time elsewhere.

---

Overall, this LGTM, but I'll wait a couple days before merging in case anyone else wants to chime in.

(Though I think it needs a cargo fmt before we can merge)

bors try

[bors[bot]]

try

Build failed:

• ci result

[phayes]

Without the "winding correction logic" you added, the mis-wound holes would add to rather than subtract from the area, right?

Correct

As devil's advocate, I'm trying to imagine if there's a world where people would intentionally wind their polygons this way, where the existing computation would be meaningful, and now we've inadvertently broken some special use case that these people might have... but I'm having a hard time imagining such a thing.

There is prior art for always considering the interior rings to reduce the area. I've been using QGIS to double-check my numbers in my tests, and QGIS always calculates interior-rings as reducing the magnitude of the area. So what we have here matches QGIS at the very least.

On balance, I think what you've done makes sense, and is a nice guard rail, so we should merge it.

Yay!

I've now run cargo fmt

bors try

[bors[bot]]

:lock: Permission denied

Existing reviewers: click here to make phayes a reviewer

[michaelkirk]

bors try

[bors[bot]]

try

Build succeeded:

• ci result

[urschrei]

I think the area calculation you've got now is the correct approach – I don't think we have to cater to what would be a fundamental divergence from the spec and convention on the part of a user.

[michaelkirk]

bors r+

[bors[bot]]

Build succeeded:

• ci result