Investigate polyclip-ts with floats instead of bignumber#2990
Investigate polyclip-ts with floats instead of bignumber#2990
Conversation
These are the changes to the tests required to pick up an alternate version of polyclip-ts that has been migrated from the bignumber usage to native javascript floats for perf. It did seem to reintroduce some errors that were previously fixed, but I'm thinking these are down to the precision issues at the ~12th decimal place
|
I confirmed that migrating those two skipped tests to 8 decimal places fixes the errors. According to a stackexchange post that's 1.1mm of precision, which seems about as far as any geo system can be pushed. I don't think we should proactively truncate our inputs or outputs to a given precision (for speed purposes), but I think we should push back on anyone filing issues that only trigger past some relatively-arbitrary precision level. |
|
|
||
| if (process.env.REGEN) | ||
| writeJsonFileSync(directories.out + filename, result); | ||
| if (process.env.REGEN) writeJsonFileSync(directories.out + filename, fc); |
There was a problem hiding this comment.
It was writing the wrong thing to disk (pre modification above), and so a REGEN actually broke all the tests.
|
I reserve the right to drop this PR and move from polyclip-ts to clipper2-ts if the @turf/buffer PR goes well. |
|
The arrival of |
These are the changes to the tests required to pick up an alternate version of polyclip-ts that has been migrated from the bignumber usage to native javascript floats for perf.
It did seem to reintroduce some errors that were previously fixed, but I'm thinking these are down to the precision issues at the ~12th decimal place.
The test failures here are expected, as I merely built polyclip-ts locally and linked to it on my machine instead of going through the work to create a self-publishing npm package.
@turf/union benchmarks:
So there's a massive speed improvement, but the unable-to-complete-output-ring tests are failing (sort of as expected).