georust / geo

Geospatial primitives and algorithms for Rust
https://crates.io/crates/geo
Other
1.48k stars 191 forks source link

Clarify that current cross-track distance uses spherical (Haversine) earth model, add Ellipsoidal (Vincenty) version #1140

Open nrhill1 opened 5 months ago

nrhill1 commented 5 months ago

Issue #1128

In response to Issue #1128 , I moved the old cross_track_distance.rs into a new cross_track_distance_haversine.rs file and added an ellipsoid cross track distance function in cross_track_distance_geodesic.rs that uses the Karney 2023 improvements to the BML algorithm on geodesic intersections.

nrhill1 commented 4 months ago

@busstoptaktik -- Interesting take, Thomas. I am, however, struggling to find the Geodesic struct in this crate so that I can understand your point a little better.

busstoptaktik commented 4 months ago

struggling to find the Geodesic struct in this crate so

I'm talking about the Geodesic struct from geographiclib_rs, which you use in your code

nrhill1 commented 4 months ago

So @busstoptaktik; are you suggesting we put these as a trait implementations in geographiclib_rs?

busstoptaktik commented 4 months ago

are you suggesting we put these as a trait implementations in geographiclib_rs?

Since geographiclib-rs is a hard dependency anyway, and since it extends it with material based on additional work by the original author of GeographicLib, I think it would be a reasonable place to put it.

Also note that it would be meaningful (e.g. for roundtripping), and probably almost free, to return a tuple consisting of the across track distance in combination with the along track distance for the intersection.

michaelkirk commented 4 months ago

Thank you for the code and sorry that this sat here for so long!

ses the Karney 2023 improvements to the BML algorithm on geodesic intersections.

Is this based on reading a paper or on existing code somewhere? Can you provide a link to either or both?

it would eliminate the (rather heavy) recomputation of all the numerical factors for Geodesic at every new call.

Note that Geodesic::wg84() is memoized, so it only incurs that setup cost one time.

@busstoptaktik's larger point about being able to use the method on other geodesics is reasonable, but in any case, I think we'll still want this existing API as well, because it mirrors our Haversine implementation, so I think it makes sense to merge this.

(I still do want to update the docs and compare to any reference implementation you can link before merging)

nrhill1 commented 4 months ago

Hi @michaelkirk, thanks for joining us!

I've implemented the ellipsoidal formula based on the following Python code which is related to the changes in this Karney 2023 paper, as well as an existing Rust implementation from Catenary.