-
Notifications
You must be signed in to change notification settings - Fork 25k
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
Documentation for geohex_grid over geo_shape #92999
Conversation
The feature to add support for geohex_grid aggregations over geo_shape fields was added in elastic#91956. This is the associated documentation for that.
Documentation preview: |
Pinging @elastic/es-analytics-geo (Team:Analytics) |
Pinging @elastic/es-docs (Team:Docs) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is looking great. I just add a comment on why shapes are done in cartesian which I think it is more accurate, wdyt?
within the edges defined by great circles. In other words the calculation is done using spherical coordinates. | ||
However, when aggregating over `geo_shape` data, the shapes are considered within a hexagon if they lie | ||
within the edges defined as straight lines on an equirectangular projection. The reason for this is that | ||
visualizing aggregation results in a map application will show surprising results when zoomed out. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the reason is that Elasticsearch (more in particular, lucene) treats edges using the equirectangular projection at search time, therefore the mismatch between the query result and the aggregation might provided surprising results.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this case we should do this with points too, right? I know we cannot change points due to backwards compatibility, but it might be nice to have an explanation for doing the two differently.
Of course with points we have less risk due to only the cells having edges, while with shapes we have edges for both the shapes and the H3 cells, increasing the likelihood of something looking weird. But that does not seem like sufficient reason to use spherical for points.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The main issue is that we accept edges (polygons and lines) that cannot be represented in spherical coordinates (edges > 180 degrees). This alone makes impossible to resolve geo_shape aggregations using spherical geometry.
Note that we use equirectangular projection but maps are normally using mercator projection, so there is already a mismatch there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK. Understood. I used your initial explanation, and decided not to involve the visual artefact discussion at all. Also, the question of edges > 180 degrees could, presumably, be solved with sidedness (and orientation), but I understand from previous discussions we have a backwards compatibility issue there. So I simplified and did not bring that up here either.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks! that's makes more sense to me.
docs/reference/aggregations/bucket/geohexgrid-aggregation.asciidoc
Outdated
Show resolved
Hide resolved
…idoc Co-authored-by: Abdon Pijpelink <abdon.pijpelink@elastic.co>
When aggregating geohex over geoshape we use requirectangular because underlying lucene index indexes and searches the polygons in that way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
I think there is a typo, otherwise thanks for the iteration!
However, when aggregating over `geo_shape` data, the shapes are considered within a hexagon if they lie | ||
within the edges defined as straight lines on an equirectangular projection. | ||
The reason is that Elasticsearch and Lucene treat edges using the equirectangular projection at index and search time. | ||
In order to ensure that search results and aggregation results are aligned, we therefor also use equirectangular |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/therefor/therefore
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow, I thought it was an alternative spelling (US vs British), but according to grammarly, "therefor" is not an alternative spelling of "therefore". We should use the conjunctive form here.
within the edges defined by great circles. In other words the calculation is done using spherical coordinates. | ||
However, when aggregating over `geo_shape` data, the shapes are considered within a hexagon if they lie | ||
within the edges defined as straight lines on an equirectangular projection. The reason for this is that | ||
visualizing aggregation results in a map application will show surprising results when zoomed out. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks! that's makes more sense to me.
According to grammarly, "therefor" is not an alternative spelling of "therefore". We should use the conjunctive form here. See https://www.grammarly.com/blog/therefore-vs-therefor/
The feature to add support for geohex_grid aggregations over geo_shape fields was added in #91956. This is the associated documentation for that.
Also made a few small geotile_grid docs fixes to make it more up-to-date (mostly copied from the newer geohex_grid aggregation which was written at least two to three years later).