Realistic input coordinate precision

[smallsaucepan]

There have been a few bugs recently that seem to relate to extremely small distances being represented as geojson. One recent case involved a line segment 0.6 nanometers long. Which seems to leave the underlying Javascript math functions no room to move, and degenerate elements creep in.

I'm wondering if we should start pushing back a little on error cases that go beyond six or seven decimals of precision (approx 10cm) at least in the first instance. Start taking the "geo" in geojson at face value. An expectation has possibly developed that Turf should be fast and robust and handle an arbitrary number of decimal places, and that's probably not something we can deliver on with JS.

Tagging a few people recently involved with issues along those lines. Please weigh in with your thoughts 🙏

@SimplyPancake @HarelM @bratter @mfedderly @JamesLMilner

[HarelM]

From my point of view, since I'm dealing with real world data (OSM) I'm totally fine with some "rounding".
The important part from my point of view is that this library will produce a good enough results and won't throw if the input is sightly problematic (duplicate like points due to precision limit etc).
While I am using real data, when converting it to different formats is when I get these odd precision issues, but I'm not sure the conversion tool are to blame here.
Another approach would be to split this into a method that will clean the data and then only use other methods and not guarantee that the methods will work without the cleaning first, but I don't think that's a great DX...

[smallsaucepan]

Maybe there is a place for turf-lint? Though whether it's an advisory debugging tool only, or a runtime input data fixer, I'm not sure. The spec is probably vague enough in a few areas that it might be tough to nail down the latter.

There is @placemarkio/check-geojson though it specifically doesn't look at precision, or ring winding which is a pretty common issue.

[mfedderly]

For ring winding we do have @turf/rewind at least as a reference

[mfedderly]

The RFC suggests 6 digits of precision, which conveniently also fits in a float32 which doesn't help us in javascript runtimes but would be meaningful for people interoperating with another language.

[bratter]

I might be missing something but my read of that comment in the spec was that it was 6 decimal places in decimal degrees (which aligns with the 10cm they quoted). Wouldn't this mean that its 9 significant figures total give the lng domain of [-180, 180], and therefore not fit into a float 32?

[mfedderly]

You may be right, I'm not an expert here but saw some references to this being the case when I was looking at it before. In any case single vs double precision floats is a bit of a weak argument anyhow. I think we can still choose some arbitrary number of decimal places (probably motivated by distance at a given precision) and push back on anyone having issues on scales smaller than we claim to support.

I'm pretty skeptical that any of our math is going to be close enough to correct once the scale gets small enough, given the errors coming from the underlying WGS84 constants, and positions coming from a GPS. There's a reason that people use local datums, adjust for continental drift over time, and hire proper surveyors.

[bratter]

Please correct me if I'm wrong, but I think it is fair to assume that the main use cases for a library like turf are going to be for visualizing real-world data and running lightweight analytics. I could very easily be wrong about the level of analytics that people use mind you.

I'm also working on the assumption that real world data is messy, and while its 100% reasonable to assume spec compliance, assuming anything beyond that (like no repeat points or certain spacing between points) is not reasonable.

So with those assumptions, my 2c is:

1. From a precision perspective, would consider a non-issue beyond that level suggested in the spec, so (a) for testing and therefore what the library promises, and (b) pushing back per @smallsaucepan's comment, seeking precision beyond the 9 significant figures (or maybe a slight buffer) should not be a goal of the library. This is not a scientific computing package :-).
2. Robustness to real world data however, as @HarelM said, I think is important. If there is nothing illegal about inputs, then the algorithms should be able to accept it and not fall over, even if the points are beyond the precision limit. I think it is totally fine for algorithms to assume that coords are coincident if within a precision limit if it makes it easier, but they should be able to take the input and process it whatever the choose to do in that processing. I don't feel confident suggesting whether functions should clean / check themselves or just suggest that cleansing / rewinding / quantiazation should be done where appropriate in the docs.
3. Degenerate cases, where possible, could have sane defaults rather than throwing. But when throwing is the right answer, try to do it close to the root cause with decent messages.
4. Speed is useful (certainly relative to precision) but a lower priority than cleanliness and ease of use.

As a testing aside, there is a mixture of exact precision tests (which definitely shouldn't exist) and approximate precision tests. To @mfedeerly's point about precision, I wonder if it would be a good idea to create a small set of helper assertion teset wrappers to use as approximately equal to an agreed number of significant figures then roll them in over time.

[bratter]

Yeah this makes total sense to me at least (primarily as a user of the package). I assume by your point 2 above you are referring to when there is an issue report?

FWIW I totally agree on the polyclip-ts topic and I am pondering your other discussion topic from earlier in the year. There shouldn't be any reason to need BigNumber for this sort of work other than the precision issues and I can't imagine anything else causing the slowdown.

[smallsaucepan]

Ah yeah, point 2 is talking about issue reports.

On the legal / reasonable topic #1946 is an old linebuffer bug, part of which is passing in a line with length 0. Legal geojson per the spec, but not possible for us to do anything with. It's not like we can even offset a point either side because what would be the correct perpendicular?

Would this be an occasion where we introduce and start throwing a "degenerate geometry" exception?

[HarelM]

I would always prefer an inaccurate response over an exception.
It's not always possible, if you request an offset to a line with only two points at the same location then it's not defined, but it only means you can return anything and it will still be valid in theory.
We are handling the same issues in line offset here for example:
maplibre/maplibre-gl-js#6625
And the way we choose to solve it is to clean the geometry before doing the calculations to avoid NaN or invalid input, but still produce valid output, even if not the best or the most accurate.
This is at least my approach, something valid but not 100% accurate is better than nothing/exception.

[smallsaucepan]

I would always prefer an inaccurate response over an exception.

My thinking was that with no changes to client code, at least we're throwing something deliberately and providing more useful context for troubleshooting. As opposed to what happens now where the code instead chokes six levels deeper with a vague JS arithmetic exception.

Totally understand not wanting to ask people to wrap try / catches around everything though. I like the idea of internally pre-sanitising like you suggest (collapsing consecutive coincident points for example) though that has its own quirks. What if two points differ only an elevation or properties level? Deciding which to use or keep track of elements to add back in later would be a nightmare.

Then even with sanitisation it's possible we're passed something we can't rationalise e.g. #2478. We can pick a sensible default (North) and keep on going, but then there's no feedback to the user that their result might not be meaningful.

For each package, would taking a "garbage in -> documented default out + a console.warn()" approach be an option?

[smallsaucepan]

Also reminded we throw when asked to generate an antipodean great circle: #2884

Just an observation, not implying it's any sort of binding precedent.