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

Fix for rectangle and square grid dimensions #2106

Merged
merged 13 commits into from
Aug 5, 2021
Merged

Conversation

rowanwins
Copy link
Member

@rowanwins rowanwins commented Jun 18, 2021

So in digging into #2079 I found that we using the haversine distance formula for calculating how big to make the grid cells.

Where this falls down is where you bbox if > 180 degrees because it then finds the shortest distance, and the wider your bbox of interest the worse the effects would be.

// A straight line 
turf.distance([-180, -90], [180, -90], {units: 'degrees'})
==> 0

Swapping to a flat earth earth calc we do something like

180 - -180
==> 360

There doesn't seem to be an easy way to hack our distance calc to produce something similar (at least in my initial play) so adjusting the approach seems reasonable and gives more expected results when generating large grids (eg see https://github.com/Turfjs/turf/blob/master/packages/turf-rectangle-grid/test/out/big-bbox-500x100-miles.geojson)

Resolves #2079
Resolves #1948
Resolves #1598

The same lines of code possibly need to be ported to point-grid and triangle-grid. See #1886

@rowanwins rowanwins mentioned this pull request Jun 18, 2021
@rowanwins rowanwins changed the title Use non-haversine distance calc Fix for rectangle and square grid dimensions Jun 18, 2021
@rowanwins
Copy link
Member Author

Alternatively I need to look into how calculate a harversine distance going the long way around (when the bbox is >180), that seems achievable, although a bit fiddlier

Copy link
Collaborator

@mfedderly mfedderly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like this broke the tests in @turf/square-grid, they probably just need to be re-updated. Github also isn't letting me do the rich diff view for these geojsons, but I'm assuming you looked at the output and think its better?

@rowanwins
Copy link
Member Author

I've checked the results of the proposed PR against QGIS and we now generate almost the exact same results.

Copy link
Collaborator

@mfedderly mfedderly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great to me

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

Successfully merging this pull request may close these issues.

Global squaregrid squareGrid returns rectangles? Grids generation problems on the whole sphere
2 participants