@turf/nearestPointOnLine Coordinates must contain numbers

[SimplyPancake]

When asking for the nearest point on line with a valid point and a lineString, at some point it returns an error stating that "coordinates must contain numbers".
However, both the lineString and Point have data. After some debugging, it seems that somewhere in the code a variable intersectPos is set. The variable is checked to be null, however there should also be a check if the array of points does not contain Nan numbers.

I recommend that this check would be fixed with something along the lines of

if (
  intersectPos &&
  Array.isArray(intersectPos) &&
  intersectPos.length === 2 &&
  !Number.isNaN(intersectPos[0]) &&
  !Number.isNaN(intersectPos[1])
) {
  intersectPt = point(intersectPos, {
    dist: distance(pt, intersectPos, options),
    multiFeatureIndex: multiFeatureIndex,
    location: length + distance(start, intersectPos, options),
  });
}

[SimplyPancake]

If you need the lineString and point, please let me know and I can supply it.

[bratter]

Hi, the lineString and point would be useful I think to give the best advice on how to fix your specific issue. Also what version of turf are you using? This function has known issues in some versions, so that will help.

In general, I don't think that your solution is likely to be optimal - your proposition effectively skips consideration of any point in the lineString, under any condition, that contains a NaN. Given that NaNs are almost certainly an error condition, silently ignoring them doesn't seem like the right option. Propagating the NaN like the current behavior is likely better, or throwing an exception earlier would likely be better options.

From your debugger screencap, it looks like the issue lies in the input data itself (I'm assuming unlikely) or the nearestPointOnSegment function (for instance if you have degenerate line segments this function will produce the observed results like in #2938, #2909, #2807, etc). If you share the inputs I'll be able to tell you for sure. If its a new issue for some reason, I want to do a PR for this function anyway per #2934.

[HarelM]

I've been using turf 7.1 for a while, and after I upgraded to 7.2 I see that some of my tests started failing.
The following is the minimal reproduction I was able to achieve:

const p = turf.point(
  [35.0005, 32.0005],
  {'marker-color': '#F00'}
);

const line = turf.lineString([
    [35.000102519989014,32.00010141921075],
    [35.00010251998902,32.00010141921075],
    [35.000102519989014,32.00020150330438],
    [35.000102519989014,32.000301587288746]
]);

const p2 = turf.nearestPointOnLine(line, p);

I tried forking this repo and adding a test for it, but I think the exception in not thrown when using node and not the browser, but I'm not 100% sure I was running the right stuff as I'm not entirely familiar with this repo.

Here's a jsbin with the error:
https://jsbin.com/qaviqowuye/1/edit?html,output

I don't think the line contains duplicate points as opposed to other reported issues, but it might be a precision related issue.
Let me know if there's anything I can do to help out fix this issue, this is currently preventing me from upgrading to 7.2.

[bratter]

Hi, I think you are 100% spot on with your precision comment. The cause is here, where the very first pair of points on the line segment is returning a cross product of [0,0,0]. This is clearly invalid and then cascades through the calc until the exception is hit.

The input points themselves are close to the precision limit, and the multiply in the cross product for sure exceeds the available precision. The function was rewritten in 7.2 - the previous version according to the commit was using planar math rather than spherical.

The correct behavior here and for other degenerate cases is likely to be assume coincidence. As per the above I am aiming to contribute a PR on this function anyway and will try and incorporate, otherwise I think you are out of luck on 7.2 unfortunately.

[HarelM]

Thanks for your reply, not sure I fully understand it, but I wouldn't expect this method to throw on a valid input, even if it reaches the precision limit, and even if there are duplicate points (or semi-duplicate points from a precision perspective).
Please let me know if there's anything I can do to help and tag me on the relevant PR.
I'm ok with not using 7.2 if there are plans to solve it in a next release or so.

[HarelM]

There's also this comment that I don't know if helps, but might be relevant, maybe:
#2511 (comment)

[HarelM]

BTW, there's also this issue, which seems they are all duplicate of the same issue:

• Error: coordinates must contain numbers #2809

[bratter]

Sorry about the lack of clarity - late night musings. Yes, certainly looks like both of those issues are similar. Specifically regarding nearestPointOnLine, the fix you suggested in #2809 is currently in master (not 7.2), but even still only catches exact point matches rather than accounting for precision loss in the internal nearestPointOnSegment function.

I'm thinking that the best solution will be to check the cross products results in nearestPointOnSegment and in cases where they are [0,0, 0] doing an early out. I will try and get a PR done this week for you to check out.

[bratter]

@HarelM I've been digging into this and having the same challenge with the error not being thrown in node and worked out that for some reason the Math.cos call that happens on the longitudes produces a result that differs in a the last bit of the mantissa for the same input between node and chrome:

Math.cos(35.000102519989014 * Math.PI / 180);

// chrome: 0 01111111110 1010 00110110 01111100 00110010 00010110 11001111 10110000
//   node: 0 01111111110 1010 00110110 01111100 00110010 00010110 11001111 10110001

It just so happens that the browser version is exactly same as the value for the other longitude, but the node one is different by that single bit. Nonetheless it is a possibility. I've put a test in for it, but it currently doesn't catch anything on node and I couldn't quickly work out how to just tweak the numbers a little to make it work. I put the following in my fork. Suggestions welcome, and will be submitting as PR soon.

[HarelM]

Nice catch! I wouldn't know where to start looking for such a problem.
But you could maybe add some browser tests, or even run the tests in a real browser since this is supported by this library.
Might be too big of a change for this fix though.
If I can help in any way, do let me know...

[bratter]

The itch needed scratching. Any comments on #2940 would be welcome, especially the choice of throwing for antipodal points rather than a more permissive "let's assume the point is on the line" approach. Cheers!