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

Michael McCandless commented on LUCENE-6780:
--------------------------------------------

Can we fix {{closestPointOnBBox}} to return {{void}}, and requiring incoming 
arg is non-null?  I think it makes the API confusing to both take as an 
argument, and return, the result.

Can we expand this out to a full if statement?:

{noformat}
     @Override
+    protected short computeMaxShift() {
+      return (short)(GeoPointField.PRECISION_STEP * ((query.radius > 2000000) 
? 5 : 4));
+    }
{noformat}

Maybe add javadoc to the new {{closestPointOnBBox}} and {{computeMaxShift}}?

In {{closestPointOnBBox}} should you maybe use Double.NaN as the marker value 
instead of 0.0 since 0.0 can legitimately occur?

A general geo API question: why do we sometimes use x/y ({{rMinX}}, {{rMinY}}) 
and other times use lon/lat ({{centerLon}}, {{centerLat}})?

It looks like your new patch lost my original patch (that just made the test 
more evil) ... when I apply both, I still see test failures, e.g.:

{noformat}
   [junit4] Started J0 PID(390@localhost).
   [junit4] Suite: org.apache.lucene.search.TestGeoPointQuery
   [junit4]   1> T0: id=3454 docID=3389 lat=16.958122319434622 
lon=91.51666186085828 deleted?=false expected=true but got false 
query=GeoPointDistanceQuery: field=geoField: Center: 
[28.3906677508417,9.349277744701268] Distance: 6883636.144942287 m Lower Left: 
[-33.7590864185931,-52.77655846609843] Upper Right: 
[90.5404219202765,71.33132841950409]
   [junit4]   2> sep 10, 2015 4:07:41 PM 
com.carrotsearch.randomizedtesting.RandomizedRunner$QueueUncaughtExceptionsHandler
 uncaughtException
   [junit4]   2> WARNING: Uncaught exception in thread: 
Thread[T0,5,TGRP-TestGeoPointQuery]
   [junit4]   2> java.lang.AssertionError: wrong hit
   [junit4]   2>        at 
__randomizedtesting.SeedInfo.seed([94245EC72F3A129]:0)
   [junit4]   2>        at org.junit.Assert.fail(Assert.java:93)
   [junit4]   2>        at 
org.apache.lucene.search.TestGeoPointQuery$VerifyHits.test(TestGeoPointQuery.java:572)
   [junit4]   2>        at 
org.apache.lucene.search.TestGeoPointQuery$1._run(TestGeoPointQuery.java:513)
   [junit4]   2>        at 
org.apache.lucene.search.TestGeoPointQuery$1.run(TestGeoPointQuery.java:406)
   [junit4]   2> 
   [junit4]   2> NOTE: reproduce with: ant test  -Dtestcase=TestGeoPointQuery 
-Dtests.method=testRandom -Dtests.seed=94245EC72F3A129 -Dtests.slow=true 
-Dtests.linedocsfile=/lucenedata/hudson.enwiki.random.lines.txt.fixed 
-Dtests.locale=es_MX -Dtests.timezone=Canada/Atlantic -Dtests.asserts=true 
-Dtests.file.encoding=UTF-8
   [junit4] ERROR   0.78s | TestGeoPointQuery.testRandom <<<
{noformat}

I'll attach the combined patch ...

I think a simple random test could ferret out self-consistency bugs in these 
new geo APIs (similar to what we did on LUCENE-6699): make a random circle and 
random bbox, then ask the GeoUtil APIs what their relationship is, then 
randomly sample points from each and confirm no point ever violates the 
relationship ... I'll try to code this up.


> GeoPointDistanceQuery doesn't work with a large radius?
> -------------------------------------------------------
>
>                 Key: LUCENE-6780
>                 URL: https://issues.apache.org/jira/browse/LUCENE-6780
>             Project: Lucene - Core
>          Issue Type: Bug
>            Reporter: Michael McCandless
>         Attachments: LUCENE-6780.patch, LUCENE-6780.patch
>
>
> I'm working on LUCENE-6698 but struggling with test failures ...
> Then I noticed that TestGeoPointQuery's test never tests on large distances, 
> so I modified the test to sometimes do so (like TestBKDTree) and hit test 
> failures.



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

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

Reply via email to