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

NearFilter doesn't account for coordinates on a sphere and conversion for distance in meters #1079

Open
leoger opened this issue Nov 25, 2024 · 3 comments

Comments

@leoger
Copy link
Contributor

leoger commented Nov 25, 2024

While trying to integrate the spatial index and Near Filter into my application based on the published documentation, I was unable to produce the expected results. The filter is returning many records far outside the specified radius.

The documentation shows a sample query that takes lat/long coordinate on Earth as the center point and floating point quantity in meters as the distance argument. However, the two unit tests covering NearFilter use a location value of "POINT (490 490)", which is outside the possible range that can correspond to a lat/long pair on a globe.

Summary

The NearFilter behavior for a range of inputs is consistent with what one would expect if neither the code in Nitrite nor the code in the JTS library are doing anything to (1) convert meters to/from degrees of latitude/longitude; or (2) account for the curvature of the Earth.

I found this root cause hypothesis was supported by examination of the existing unit tests in nitrite-spatial/src/test/, the implementation in nitrite-spatial/src/main/, and the JTS library's code and documentation.

Repro steps

I have opened a PR on my own personal fork of nitrite where you can see simple unit tests cases based on real-world lat-long data designed to check whether the Spatial distance search is properly. (See this map for an illustration of the unit test data.)

Here is a simplified repro at the equator:

  1. Choose a center point $P_C$ at $( 0 \degree, 0 \degree )$ in the Atlantic Ocean.
  2. Let $P_{E1}$ be a point approximately 111km due east of $P_C$, at $( 0 \degree, +1 \degree )$
  3. Let $P_{E2}$ be a point approximately 1km due east of $P_C$, at $( 0 \degree , +0.01 \degree)$
  4. Use NearFilter to find the subset of $\{ P_{E1} , P_{E2} \}$ within 2km and within 20cm of $P_C$.
    within2kmFilter = where("geometry").near(centerPoint, 2000.0 /* meters */);
    within20cmFilter = where("geometry").near(centerPoint, 0.02 /* meters */);

Observed vs. Expected results

Observed Expected
within2kmFilter returns $\{ P_{E1} , P_{E2} \}$ returns $\{ P_{E2} \}$
within20cmFilter returns $\{ P_{E2} \}$ empty result set
@leoger leoger changed the title NearFilter doesn't account correctly for coordinates on a sphere and distances in meters NearFilter doesn't account for coordinates on a sphere and conversion for distance in meters Nov 25, 2024
@anidotnet
Copy link
Contributor

Hi @leoger thank you for reporting this issue. I am still quite busy with other urgent engagement. Meantime if you have a fix ready, I'll be happy to take a PR.

@leoger
Copy link
Contributor Author

leoger commented Nov 30, 2024

@anidotnet, I do have a proposal that would either pull in another dependency or duplicate some detailed logic from a library. My reading on the topic suggests that what we'd want here is some code that can solve the "direct geodesic problem". Please let me know what your preference would be between these two as a starting point.1

I believe we could keep the same basic approach of generating a boundary, but instead of it being a circle/ellipse, it would be end up being a sort of a pinched/stretched ellipse. Here's some pseudo-code.

# in python-ish pseudo-code:
for i in range(0, NUM_POINTS):
    bearing = (i * 2 * Math.PI) / NUM_POINTS
    # NOTE: I can't tell whether this is properly called a "bearing" or an "azimuth" in this usage. 🤷‍♂
    perimeter_point = geodesic_direct(center.lat, center.lon, bearing, distance_in_meters)
    perimeter_poly.append(perimeter_point)
end

I will try to share a PR for this in ~2-3 weeks.

Footnotes

  1. If I were in your shoes, I'd probably just want a local copy of just the needed math library function that could be replaced later with a dependency of your choice. If the tests still pass, you can reasonably be confident it hasn't broken anything.

@anidotnet
Copy link
Contributor

I would vote for adding a dependency in the nitrite spatial module for required math logic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants