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

Fixes computation of geohash neighbours #7247

Closed
wants to merge 0 commits into from
Closed

Fixes computation of geohash neighbours #7247

wants to merge 0 commits into from

Conversation

colings86
Copy link
Contributor

The geohash grid it 8 cells wide and 4 cells tall. GeoHashUtils.neighbor(String,int,int.int) set the limit of the number of cells in y to < 3 rather than <= 3 resulting in it either not finding all neighbours or incorrectly searching for a neighbour in a different parent cell.

Closes #7226

@colings86
Copy link
Contributor Author

I actually think this code should be changed to work out the neighbours from the complete geohash in one go rather than doing it level by level. This would be more efficient as there is a 20 in 32 chance of a random geohash being on the edge of a parent cell so we end up doing a lot of computation at different levels. Instead we should decode the full geohash into its odd and even components, add/subtract from the relevant component and then re-encode to get the final result. It would also make the code a lot easier to follow.

Thoughts?

@clintongormley
Copy link
Contributor

That sounds sensible.

@jpountz jpountz removed the review label Aug 13, 2014
@jpountz
Copy link
Contributor

jpountz commented Aug 13, 2014

LGTM

+1 also to improve the way that neighbors are computed, but I would be totally fine with doing it in another PR.

@colings86
Copy link
Contributor Author

Pushed to master, 1.x and 1.3

@colings86 colings86 closed this Aug 13, 2014
@colings86 colings86 self-assigned this Aug 21, 2014
@colings86 colings86 deleted the fix/7226 branch August 21, 2014 15:10
@clintongormley clintongormley added the :Analytics/Geo Indexing, search aggregations of geo points and shapes label Jun 7, 2015
@clintongormley clintongormley changed the title Geo: fixes computation of geohash neighbours Fixes computation of geohash neighbours Jun 7, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Analytics/Geo Indexing, search aggregations of geo points and shapes >bug v1.4.0.Beta1 v2.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Geo: Geohash_cell produces bad neighbors
3 participants