update turf modules & .001 meter tolerance to area check

mapbox / tile-cover

Generate the minimum number of tiles to cover a geojson geometry

MIT License

189 stars 40 forks source link

update turf modules & .001 meter tolerance to area check #63

Closed morganherlocker closed 9 years ago

morganherlocker commented 9 years ago

Fixes an upstream nested dependency fail in jsts. The latest turf dependencies now shrinkwrap the failed lib inside jsts.
Slight behavior changes in jsts upstream also caused intersections at edges to be treated slightly differently. This led to near-zero area polygons slivers in the tile-cover tests. I added a small tolerance (0.001 meters) to this check.

cc @ingalls @karenzshea @yhahn @kelvinabrokwa

morganherlocker commented 9 years ago

travis builds are failing, but all tests pass when i run locally from a scratch-cloned repo. :/

morganherlocker commented 9 years ago

this failure happens on node 0.8.x and 0.10.x but not 0.12.x or io.js O_o https://travis-ci.org/mapbox/tile-cover/jobs/55696154

morganherlocker commented 9 years ago

0.11, 0.12 & io.js 1.6:

0.8 & 0.10:

morganherlocker commented 9 years ago

I found the discrepency buried deep in the callstack, inside tilebelt. There is a call to Math.exp, which returns ever so slightly different results in the latest version of V8. I believe that this is actually fixing a bug in previous versions, and the results now more closely match the intent of the code.

Math.exp(0.3650874275167828)

// node v0.10
// 1.4406399542173363
// node v0.12
// 1.440639954217336

This affected only one cover out of hundreds of tests and the results are still good (all geometries were still covered), so I feel good about overwriting this fixture to get tests to pass.

@yhahn I am considering modifying the .travis.yml file to target node 0.12 only. What do you think? Should we have version specific assertions, or is that overkill?

mourner commented 9 years ago

@morganherlocker what did you decide here? Also discovered that failing test today... Maybe we could try making the math in tilebelt more reliable somehow.

mourner commented 9 years ago

Looks like we have a good fix (see https://github.com/mapbox/tilebelt/issues/22).