[
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: [email protected]
For additional commands, e-mail: [email protected]
