-
Notifications
You must be signed in to change notification settings - Fork 200
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
Rhumb destinations do not wrap angles after the first pole intersection #1152
Comments
@apendleton this is your feature. I don't think this is a bug, but I agree that clamping lon and lat would be preferable. WDYT, and do you have capacity to add it? |
Returning a latitude <-90 is pretty close to being a bug :) I will take a look if I have time myself. |
For reference, here's a quick port of this test program to Javascript so I could test it with Turf (which is what the Rust implementation was ported from): const fs = require('fs');
const turf = require('@turf/turf');
function main() {
const out = [];
const origin = [0.0, 0.0];
for (let i = 0; i < 1000000; i++) {
let dist = 100.0 * i;
let dest = turf.rhumbDestination(origin, dist, 45.0, {units: 'meters'}).geometry.coordinates;
out.push(`${dist},${dest[0]},${dest[1]}`);
}
fs.writeFileSync("./turf_rhumb.csv", out.join('\n'));
}
main(); And it produces broadly similar output with respect to latitude, but probably-saner output with respect to longitude: With respect to longitude, we do already attempt to ensure that we're wrapping correctly, but it looks like there's maybe some corner-case weirdness that results in us producing some occasional out-of-bounds values as evidenced by this graph. I think wrapping (not clamping) is the preferred behavior for longitude, so probably we need to figure out what's going on that results in us occasionally producing longitudes <-180 in the Rust version. For latitude, I'm actually not sure what we want the behavior to be for very long lines. I think clamping at +/- 90 is defensible, but it's not clear if we just allow the longitude number to continue to spin meaninglessly in that case or what. As I understand it, conceptually, for non-horizontal lines, we should be spiraling towards the pole for awhile and then eventually reach it at some fixed/calculable distance, after which further movement along the line is undefined. I can pitch in however is most useful, but it might be good to see if any other implementations exist out in the wild to see if there's a standard/expected behavior in this circumstance? I don't have a clear frame of reference here except for Turf (which seems to also be arguably wrong). |
Returning an |
An As to how to calculate: yeah, makes sense. My use of |
I would say API inconsistency isn't a big deal; it's accounting for a different detail after all. The breaking change is a concern. On the other hand, the API isn't in wide use, and the change is certainly justifiable. |
Makes sense. I'll probably have time to dig in in more detail either Friday or this weekend, but happy to cede this to someone else if there's interest in it being done sooner. |
Turns out to be a lot simpler than I expected. Early return is actually the faster/simpler approach here since the conditional is already being checked anyway. |
Given that a rhumb line has finite length to a given pole, and that
RhumbDestination
appears to wrap correctly in one direction, one would expect that latitude would be bounded to +/-90 and longitude to +/-180 as the path length increased. Instead, we see the following:Here is a rust program that dumps the data:
and some python code to load and plot:
The expected fix would be that latitude would be clamped, either by taking the modulo of the path length from pole to pole, or empirically.
The text was updated successfully, but these errors were encountered: