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

Invalid conversion from radians to yards #193

Closed
dimonkomaron opened this issue Oct 16, 2024 · 5 comments · Fixed by #194
Closed

Invalid conversion from radians to yards #193

dimonkomaron opened this issue Oct 16, 2024 · 5 comments · Fixed by #194
Labels
bug Something isn't working

Comments

@dimonkomaron
Copy link
Contributor

Seems like conversion from radians to length if using Unit.yards calculates wrong because 1 meter = 1.09361 yards, but in factors earthRadius is divided on 1.0936 but there we should multiply.

/// Convert a distance measurement (assuming a spherical Earth) from radians to a more friendly unit.
/// Valid units: miles, nauticalmiles, inches, yards, meters, metres, kilometers, centimeters, feet
num radiansToLength(num radians, [Unit unit = Unit.kilometers]) {
  var factor = factors[unit];
  if (factor == null) {
    throw Exception("$unit units is invalid");
  }
  return radians * factor;
}
/// Earth Radius used with the Harvesine formula and approximates using a spherical (non-ellipsoid) Earth.
const earthRadius = 6371008.8;

/// Unit of measurement factors using a spherical (non-ellipsoid) earth radius.
/// Keys are the name of the unit, values are the number of that unit in a single radian
const factors = <Unit, num>{
  Unit.centimeters: earthRadius * 100,
  Unit.degrees: earthRadius / 111325,
  Unit.feet: earthRadius * 3.28084,
  Unit.inches: earthRadius * 39.370,
  Unit.kilometers: earthRadius / 1000,
  Unit.meters: earthRadius,
  Unit.miles: earthRadius / 1609.344,
  Unit.millimeters: earthRadius * 1000,
  Unit.nauticalmiles: earthRadius / 1852,
  Unit.radians: 1,
  Unit.yards: earthRadius / 1.0936,
};
@lukas-h lukas-h added the bug Something isn't working label Oct 16, 2024
@lukas-h
Copy link
Member

lukas-h commented Oct 16, 2024

Hi @dimonkomaron, thank you for bringing this issue to our attention. 👍 👍

Would you be open to contribute and create a pull request to fix this problem?

Otherwise it might take longer until this bug is fixed.

@dimonkomaron
Copy link
Contributor Author

dimonkomaron commented Oct 17, 2024

@lukas-h Yes, ok, will try :)

Also have a question is there reason why spherical radius is used as it can loss precision near the equator or ellipsoidal radius has it's own flaws?

/// Earth Radius used with the Harvesine formula and approximates using a spherical (non-ellipsoid) Earth. const earthRadius = 6371008.8;

P.S. Also seems I have no rights to push in the repository

@lukas-h
Copy link
Member

lukas-h commented Oct 17, 2024

@dimonkomaron

P.S. Also seems I have no rights to push in the repository

just start by

  1. forking this repository
  2. committing onto the main branch of your fork
  3. create a pull request with a clear name so it is identifiable
  4. then I'll review and merge

Also have a question is there reason why spherical radius is used as it can loss precision near the equator or ellipsoidal radius has it's own flaws?

/// Earth Radius used with the Harvesine formula and approximates using a spherical (non-ellipsoid) Earth. const earthRadius = 6371008.8;

we didn't decide on that ourselves, rather it was a decision made by TurfJS, the original javascript library that turf_dart is very much influenced by.

Perplexity gave me this answer: https://www.perplexity.ai/search/in-turfjs-is-there-reason-why-DDd.UpGSRo6Awus4iot7iA

If you have an idea/concept for ellipsoidal calculations how to do it, let me know, I'd be happy to discuss and add it to the library 😄

@dimonkomaron
Copy link
Contributor Author

@lukas-h PR is done. You can find it here: #194

@lukas-h
Copy link
Member

lukas-h commented Oct 17, 2024

@dimonkomaron It is merged. Until the next turf release on pub.dev you can just use the package like this in your pubspec.yaml:

dependencies:
  turf:
    git:
      url: https://github.com/dartclub/turf_dart.git
      ref: main

I will mark this issue as done.

@lukas-h lukas-h closed this as completed Oct 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants