[jira] [Commented] (LUCENE-7127) remove epsilon-based testing from lucene/spatial

["Robert Muir (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/LUCENE-7127?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15205872#comment-15205872 ] 

Robert Muir commented on LUCENE-7127:
-------------------------------------

The "fix" for that failure can look like this, the optimization is just not working:
{noformat}
--- a/lucene/spatial/src/java/org/apache/lucene/spatial/geopoint/search/GeoPointDistanceQueryImpl.java
+++ b/lucene/spatial/src/java/org/apache/lucene/spatial/geopoint/search/GeoPointDistanceQueryImpl.java
@@ -60,7 +60,7 @@ final class GeoPointDistanceQueryImpl extends GeoPointInBBoxQueryImpl {
 
     @Override
     protected boolean cellWithin(final double minLon, final double minLat, final double maxLon, final double maxLat) {
-      return GeoRelationUtils.rectWithinCircle(minLon, minLat, maxLon, maxLat,
+      return false && GeoRelationUtils.rectWithinCircle(minLon, minLat, maxLon, maxLat,
           centerLon, distanceQuery.centerLat, distanceQuery.radiusMeters, true);
     }
{noformat}

I also noticed if i just unconditionally returned {{true}} for {{cellCrosses}} the perf issues with testing go away and its just as fast as points. So I think these problems boil down to the same struggles i encountered when i tried to use them to add distance query to LatLonPoint?

A real/viable/alernative fix (which i can look at tomorrow), would be to just port the logic over from LatLonPointQuery for these? We can also port over its optimizations too to compensate. It might even end out faster.

> remove epsilon-based testing from lucene/spatial
> ------------------------------------------------
>
>                 Key: LUCENE-7127
>                 URL: https://issues.apache.org/jira/browse/LUCENE-7127
>             Project: Lucene - Core
>          Issue Type: Test
>            Reporter: Robert Muir
>         Attachments: LUCENE-7127.patch, LUCENE-7127.patch, LUCENE-7127.patch
>
>
> Currently, the random tests here allow a TOLERANCE and will fail if the error exceeds. But this is not fun to debug! It also keeps the door wide open for bugs to creep in.
> Alternatively, we can rework the tests like we did for sandbox/ points. This means the test is aware of the index-time quantization and so it can demand exact answers.
> Its more difficult at first, because even floating point error can cause a failure. It requires us to maybe work through corner cases/rework optimizations. If any epsilons must be added, they can be added to the optimizations themselves (e.g. bounding box) instead of the user's result.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org