[ 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