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

Clarify in convertLength documentation usage of degree and radian units #2203

Closed
stebogit opened this issue Sep 15, 2021 · 7 comments
Closed
Labels

Comments

@stebogit
Copy link
Collaborator

Working on a line grid module, copying from the @turf/rectangle-grid package, I stumbled upon this inconsistency. Is it possible convertLength is off or am I missing something obvious here?

const nw = turf.point([-120.20778894424438, 47.40590873190948]);
const ne = turf.destination(nw, 1, 90, { units: 'kilometers' });
const sw = turf.destination(nw, 1, 180, { units: 'kilometers' });
const se = turf.point([ne.geometry.coordinates[0], sw.geometry.coordinates[1]]);

const box = turf.bboxPolygon(turf.bbox(turf.featureCollection([nw, ne, sw, se])))

const dKm = turf.distance(nw, ne, {units: 'kilometers'});
const width = Math.abs(nw.geometry.coordinates[0] - ne.geometry.coordinates[0]);
const widthKm = turf.convertLength(width, 'degrees', 'kilometers');

console.log(dKm); // 1.0000000000004476
console.log(widthKm); // 1.4792675889975462

Screen Shot 2021-09-15 at 9 24 18 AM

The earth's curvature can't justify such big difference in a 1km distance.

The way the bbox's width is calculated above is the same logic used now (recently updated) in defining the rectangle/square grid features, but that should produce an incorrect grid result. Though I'm not sure those changes have been already published since the output of the square grid seems fine.
@rowanwins any thoughts?

@tunnelpuzzle
Copy link
Contributor

tunnelpuzzle commented Sep 18, 2021

I think the issue is subtracting the latitude won't give you the correct degree distance.

const width = Math.abs(nw.geometry.coordinates[0] - ne.geometry.coordinates[0]); // 0.013287829229710724
const widthDistance= turf.distance(nw, ne, {units: 'degrees'}); // 0.008982708286552414

If your starting point was at [0, 0] for example instead of [-120.0, 47.0] it would work.

@smallsaucepan
Copy link
Member

This sounds like it wasn't an issue in the end? Closing but please reopen if it was a problem with convertLength.

@smallsaucepan smallsaucepan closed this as not planned Won't fix, can't repro, duplicate, stale Jun 26, 2024
@twelch
Copy link
Collaborator

twelch commented Jun 28, 2024

@smallsaucepan looks related to the recent #2638 and possibly the changes made in #2106? And a difference in how turf.distance and turf.convertLength work. If the difference is expected, the goal of this issue might just be to clarify the behavior (and maybe underlying earth model) of each, which could make it's way into the API docs.

@twelch twelch reopened this Jun 28, 2024
@twelch twelch added the docs label Jun 28, 2024
@twelch
Copy link
Collaborator

twelch commented Jun 28, 2024

I wouldn't mind updating the docs if someone could explain the difference.

@smallsaucepan
Copy link
Member

This is working ok. The distance between two longitudes (nw.geometry.coordinates[0] - ne.geometry.coordinates[0]) varies with latitude though. So we can't just subtract them and use that as an "angle" to feed into convertLength().

I've had in mind to add a "Units" page to the website for a while. Let's leave this open to be resolved by the addition of that page - including gotchas when using degrees and radians as units.

@smallsaucepan smallsaucepan changed the title convertLength incorrect? Clarify in convertLength documentation usage of degree and radian units Jun 30, 2024
@smallsaucepan
Copy link
Member

@twelch can we consider this covered by PR #2748, which adds a cautionary note to the Units documentation about using degrees and radians? That page would eventually be linked to from many pages, including convertLength.

@twelch
Copy link
Collaborator

twelch commented Nov 22, 2024

Yes, @smallsaucepan the addition of Units docs is sufficient for me to close this.

@twelch twelch closed this as completed Nov 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants