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

LatLonSpherical.intersection() - rounding errors lead to φ3=NaN #71

Closed
henrythasler opened this issue Nov 29, 2019 · 3 comments
Closed

Comments

@henrythasler
Copy link

Action

Add the following test case to the intersection group in latlon-spherical-tests.js:

test('φ3=NaN', () => should.equal(LatLon.intersection(new LatLon(-77.6966041375563, 18.28125000000003), 179.99999999999994, new LatLon(89, 180), 180), null));

Expected behavior

I don't know... something around the south pole or no solution at all?

Observed behavior

φ3=NaN

  1 failing

  1) latlon-spherical
       intersection
         rounding errors, φ3=NaN:
     TypeError: invalid lat ‘NaN’
      at new LatLonSpherical (latlon-spherical.js:49:31)
      at Function.intersection (latlon-spherical.js:442:16)
      at Context.test (test/latlon-spherical-tests.js:183:81)
      at process.topLevelDomainCallback (domain.js:126:23)

Workaround

(...)
const φ3 = Math.asin(Math.sin(φ1) * Math.cos(δ13) + Math.cos(φ1) * Math.sin(δ13) * Math.cos(θ13));
if (isNaN(φ3)) {
    return null;
}
(...)
@chrisveness
Copy link
Owner

I think a better option would be to clamp the value supplied to Math.asin() in the calculation of φ3:

const φ3 = Math.asin(Math.min(Math.max(Math.sin(φ1)*Math.cos(δ13) + Math.cos(φ1)*Math.sin(δ13)*Math.cos(θ13), -1), 1));

Test:

test('rounding: φ3 requires clamp', () => LatLon.intersection(new LatLon(-77.6966041375563, 18.2812500000000), 179.99999999999995, new LatLon(89, 180), 180).toString().should.equal('90.0000°S, 163.9902°W'));

As far as I can see, this gives correct values.

henrythasler added a commit to henrythasler/Leaflet.Geodesic that referenced this issue Dec 1, 2019
@henrythasler
Copy link
Author

Ok, that works for me. Thanks!

@chrisveness
Copy link
Owner

Fixed in v2.2.1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants