Lucene - Core
  1. Lucene - Core
  2. LUCENE-1781

Large distances in Spatial go beyond Prime MEridian

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.9
    • Fix Version/s: 3.0
    • Component/s: modules/spatial
    • Labels:
      None
    • Environment:

      All

    • Lucene Fields:
      New

      Description

      http://amidev.kaango.com/solr/core0/select?fl=*&json.nl=map&wt=json&radius=5000&rows=20&lat=39.5500507&q=honda&qt=geo&long=-105.7820674

      Get an error when using Solr when distance is calculated for the boundary box past 90 degrees.

      Aug 4, 2009 1:54:00 PM org.apache.solr.common.SolrException log
      SEVERE: java.lang.IllegalArgumentException: Illegal lattitude value 93.1558669413734
      at org.apache.lucene.spatial.geometry.FloatLatLng.<init>(FloatLatLng.java:26)
      at org.apache.lucene.spatial.geometry.shape.LLRect.createBox(LLRect.java:93)
      at org.apache.lucene.spatial.tier.DistanceUtils.getBoundary(DistanceUtils.java:50)
      at org.apache.lucene.spatial.tier.CartesianPolyFilterBuilder.getBoxShape(CartesianPolyFilterBuilder.java:47)
      at org.apache.lucene.spatial.tier.CartesianPolyFilterBuilder.getBoundingArea(CartesianPolyFilterBuilder.java:109)
      at org.apache.lucene.spatial.tier.DistanceQueryBuilder.<init>(DistanceQueryBuilder.java:61)
      at com.pjaol.search.solr.component.LocalSolrQueryComponent.prepare(LocalSolrQueryComponent.java:151)
      at org.apache.solr.handler.component.SearchHandler.handleRequestBody(SearchHandler.java:174)
      at org.apache.solr.handler.RequestHandlerBase.handleRequest(RequestHandlerBase.java:131)
      at org.apache.solr.core.SolrCore.execute(SolrCore.java:1328)
      at org.apache.solr.servlet.SolrDispatchFilter.execute(SolrDispatchFilter.java:341)
      at org.apache.solr.servlet.SolrDispatchFilter.doFilter(SolrDispatchFilter.java:244)
      at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:235)
      at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:206)
      at org.apache.catalina.core.StandardWrapperValve.invoke(StandardWrapperValve.java:233)
      at org.apache.catalina.core.StandardContextValve.invoke(StandardContextValve.java:191)
      at org.apache.catalina.core.StandardHostValve.invoke(StandardHostValve.java:128)
      at org.apache.catalina.valves.ErrorReportValve.invoke(ErrorReportValve.java:102)
      at org.apache.catalina.core.StandardEngineValve.invoke(StandardEngineValve.java:109)
      at org.apache.catalina.connector.CoyoteAdapter.service(CoyoteAdapter.java:286)
      at org.apache.coyote.http11.Http11AprProcessor.process(Http11AprProcessor.java:857)
      at org.apache.coyote.http11.Http11AprProtocol$Http11ConnectionHandler.process(Http11AprProtocol.java:565)
      at org.apache.tomcat.util.net.AprEndpoint$Worker.run(AprEndpoint.java:1509)
      at java.lang.Thread.run(Thread.java:619)

      1. LUCENE-1781.patch
        18 kB
        Bill Bell
      2. LUCENE-1781.patch
        3 kB
        Michael McCandless

        Activity

        Hide
        Michael McCandless added a comment -

        No, this is fixed in 3.0 (to be released shortly).

        Show
        Michael McCandless added a comment - No, this is fixed in 3.0 (to be released shortly).
        Hide
        Bill Bell added a comment -

        Has this been applied to 2.9.1 ? Or do I need to apply it manually? I tested it on 2.9.1 and it does not appear to be in there.

        Show
        Bill Bell added a comment - Has this been applied to 2.9.1 ? Or do I need to apply it manually? I tested it on 2.9.1 and it does not appear to be in there.
        Hide
        Michael McCandless added a comment -

        Correcting fix version.

        Show
        Michael McCandless added a comment - Correcting fix version.
        Hide
        Michael McCandless added a comment -

        Thanks Bill!

        Show
        Michael McCandless added a comment - Thanks Bill!
        Hide
        Michael McCandless added a comment -

        Mark is it OK to commit this now? I realize it won't make 2.9 if the current RC is it...

        Show
        Michael McCandless added a comment - Mark is it OK to commit this now? I realize it won't make 2.9 if the current RC is it...
        Hide
        Michael McCandless added a comment -

        I opened LUCENE-1921.

        Show
        Michael McCandless added a comment - I opened LUCENE-1921 .
        Hide
        Michael McCandless added a comment -

        Can we go with my patch, and address this new large mile issue as a new LUCENE issue?

        I agree, let's break this out as its own issue. I'll open it and then commit this one. Thanks Bill!

        Show
        Michael McCandless added a comment - Can we go with my patch, and address this new large mile issue as a new LUCENE issue? I agree, let's break this out as its own issue. I'll open it and then commit this one. Thanks Bill!
        Hide
        Bill Bell added a comment -

        Can we go with my patch, and address this new large mile issue as a new LUCENE issue? I can probably assist, I just want this committed if possible.

        Show
        Bill Bell added a comment - Can we go with my patch, and address this new large mile issue as a new LUCENE issue? I can probably assist, I just want this committed if possible.
        Hide
        Bill Bell added a comment -

        Probably, do we have someone else to tackle that one? I would go further than 10,000 miles as well.

        There should be a switch to return all points, and just keep sorting and distance from lat/long, but I am not sure where to put that one...

        Bill

        Show
        Bill Bell added a comment - Probably, do we have someone else to tackle that one? I would go further than 10,000 miles as well. There should be a switch to return all points, and just keep sorting and distance from lat/long, but I am not sure where to put that one... Bill
        Hide
        Michael McCandless added a comment -

        Looks like great progress! I now see all tests pass with your patch.

        So you now explicitly handle 2 different rects in CartesianPolyFilterBuilder.

        However, I added a "miles too big" test, by inserting a 10000.0 miles test case in testRange & testGeoHashRange, which ought to return all points (20) but instead only hits 1. I think to fix this we just have to detect when the rect wraps around back onto itself, and somehow return "whole world" as the degenerate filter?

        Show
        Michael McCandless added a comment - Looks like great progress! I now see all tests pass with your patch. So you now explicitly handle 2 different rects in CartesianPolyFilterBuilder. However, I added a "miles too big" test, by inserting a 10000.0 miles test case in testRange & testGeoHashRange, which ought to return all points (20) but instead only hits 1. I think to fix this we just have to detect when the rect wraps around back onto itself, and somehow return "whole world" as the degenerate filter?
        Hide
        Bill Bell added a comment -

        Patch that fixed prime meridian and antimeridian issues. Also pole flipping.

        Show
        Bill Bell added a comment - Patch that fixed prime meridian and antimeridian issues. Also pole flipping.
        Hide
        Bill Bell added a comment -

        Combined patch. This does not fix Anti Meridien yet. It does test for it.

        Show
        Bill Bell added a comment - Combined patch. This does not fix Anti Meridien yet. It does test for it.
        Hide
        Michael McCandless added a comment -

        Bill, could you please post a single patch that includes all of your changes (fixing LLRect & TestCartesian)? Also, please first "svn up" to the latest svn trunk so that the patch applies cleanly.

        testPrimeM() should be fixed by the new LLRect.java. It goes over 90 degrees, and LLRect.java pole flips. If this is not working with the new LLRect, there is something wrong with that.

        Hmm I'm seeing testPrimeM fail – maybe once I sync to your new patch I'll see it pass. Maybe name that test "testPoleFlipping" or something instead?

        So testPrimeM isn't testing the "miles is too large" issue? If not, can you add a new test that explicitly tests this? We can use something ridiculous like 100000 miles.

        LLRect does calculate the right box across anti-meridien.

        I agree that it calculates the correct "upper right" and "lower left", but when those cross the anti-meridian, it's not possible to represent that with a single rect (you need two). This is where I'm waaaay out of my depth w/ GIS search... in general, how is this "normally" handled? It seems like something higher up must accept two rects and OR them together during the searching?

        Show
        Michael McCandless added a comment - Bill, could you please post a single patch that includes all of your changes (fixing LLRect & TestCartesian)? Also, please first "svn up" to the latest svn trunk so that the patch applies cleanly. testPrimeM() should be fixed by the new LLRect.java. It goes over 90 degrees, and LLRect.java pole flips. If this is not working with the new LLRect, there is something wrong with that. Hmm I'm seeing testPrimeM fail – maybe once I sync to your new patch I'll see it pass. Maybe name that test "testPoleFlipping" or something instead? So testPrimeM isn't testing the "miles is too large" issue? If not, can you add a new test that explicitly tests this? We can use something ridiculous like 100000 miles. LLRect does calculate the right box across anti-meridien. I agree that it calculates the correct "upper right" and "lower left", but when those cross the anti-meridian, it's not possible to represent that with a single rect (you need two). This is where I'm waaaay out of my depth w/ GIS search... in general, how is this "normally" handled? It seems like something higher up must accept two rects and OR them together during the searching?
        Hide
        Bill Bell added a comment - - edited

        Posted new Test Junit tests. The AntiM will still fail, until we get a fix.

        Show
        Bill Bell added a comment - - edited Posted new Test Junit tests. The AntiM will still fail, until we get a fix.
        Hide
        Bill Bell added a comment - - edited

        Not exactly.

        testPrimeM() should be fixed by the new LLRect.java. It goes over 90 degrees, and LLRect.java pole flips. If this is not working with the new LLRect, there is something wrong with that.

        testAntiM() creates a case that should fail. This condition needs to be fixed. The case is: You are located in Hawaii and you should see:

        Marshall Island Airfield",7.06, 171.2
        Midway Island",25.7, -171.7

        It only shows the Midway Island, since it is still in the same hemisphere (it did not go from -171 to 171 (the Anti-Meridien), This case also errors since the upper right is also going over the pole. So it also is testing testPrimeM(). If we reduce the miles to 3500 it tests only these 2 points. Since it does not show Marshall Island, the Local Lucene is not working right. Once this case works people we can release it.

        Here are my thoughts on a fix:

        LLRect does calculate the right box across anti-meridien. The issue is elsewhere. Either we have multiple boxes, or check the Point2D since the whole thing is not right crossing boundaries... box.getMaxPoint().getY() is not the max when going to and vice versa.

        [junit] boxCorners: ur 43.242262719615994,-123.073340742054
        [junit] boxCorners: cnt 21.6032207,-158.0
        [junit] boxCorners: ll -5.189678558944157,177.24228397256368

        Show
        Bill Bell added a comment - - edited Not exactly. testPrimeM() should be fixed by the new LLRect.java. It goes over 90 degrees, and LLRect.java pole flips. If this is not working with the new LLRect, there is something wrong with that. testAntiM() creates a case that should fail. This condition needs to be fixed. The case is: You are located in Hawaii and you should see: Marshall Island Airfield",7.06, 171.2 Midway Island",25.7, -171.7 It only shows the Midway Island, since it is still in the same hemisphere (it did not go from -171 to 171 (the Anti-Meridien), This case also errors since the upper right is also going over the pole. So it also is testing testPrimeM(). If we reduce the miles to 3500 it tests only these 2 points. Since it does not show Marshall Island, the Local Lucene is not working right. Once this case works people we can release it. Here are my thoughts on a fix: LLRect does calculate the right box across anti-meridien. The issue is elsewhere. Either we have multiple boxes, or check the Point2D since the whole thing is not right crossing boundaries... box.getMaxPoint().getY() is not the max when going to and vice versa. [junit] boxCorners: ur 43.242262719615994,-123.073340742054 [junit] boxCorners: cnt 21.6032207,-158.0 [junit] boxCorners: ll -5.189678558944157,177.24228397256368
        Hide
        Michael McCandless added a comment -

        So the anti-meridian test is expected to fail, right? Ie, LLRect is only able to represent a single rect, which is insufficient (I think?). Not sure what LLRect should do when it detects that the anti-meridian is crossed. Would be best if we could return 2 rects somehow...

        The prime-meridan test is testing the "distance too large" issue, right? It fails now, but we should be able to fix this, I think, by capping the corners somehow?

        The last issue is the pole-flipping issue, which we think we fixed but we don't yet have a test case for, right?

        Show
        Michael McCandless added a comment - So the anti-meridian test is expected to fail, right? Ie, LLRect is only able to represent a single rect, which is insufficient (I think?). Not sure what LLRect should do when it detects that the anti-meridian is crossed. Would be best if we could return 2 rects somehow... The prime-meridan test is testing the "distance too large" issue, right? It fails now, but we should be able to fix this, I think, by capping the corners somehow? The last issue is the pole-flipping issue, which we think we fixed but we don't yet have a test case for, right?
        Hide
        Bill Bell added a comment -

        I added the test cases that both fail.

        Show
        Bill Bell added a comment - I added the test cases that both fail.
        Hide
        Bill Bell added a comment -

        In case you cannot figure out the SVN diff.

        Show
        Bill Bell added a comment - In case you cannot figure out the SVN diff.
        Hide
        Bill Bell added a comment -

        New test cases to test Prime and Anti Meridien. Both fail

        Show
        Bill Bell added a comment - New test cases to test Prime and Anti Meridien. Both fail
        Hide
        Bill Bell added a comment -

        SVN diff patch

        Show
        Bill Bell added a comment - SVN diff patch
        Hide
        Michael McCandless added a comment -

        OK the new normLat looks like it handles crossing the pole now, by flipping the lng; thanks.

        Can you add a unit test that confirms this was a problem before, and that the new lng-flipping resolves it? Probably if you just modify TestCartesian, to add a new point, and then add a new test cast that starts from a lat/lng that's near the pole, you could get the issue to happen & be resolved.

        Also, can use "svn diff" and post the resulting output (instead of the whole copy of each source file)? Thanks.

        The current Spatial ONLY works for one hemisphere at a time.

        Actually, shouldn't LLRect work fine if a rect crosses the equator? Or, a pole? What I think it cannot handle is crossing the anti-meridian? (Because, when that projects out, you'll need 2 rectangles, on opposite sides, to handle it?). I'm not sure what LLRect should even do if it finds it "needs" to cross the anti-meridian. If it had the ability to return 2 rectangles I think it could be fixed.

        Or are you saying there are further limitations in contrib/spatial (ie, not caused by LLRect) that cause it to only work within one hemisphere?

        As for the "we get no results when radius is very large" bug, I like the theory that the corners need to be flipped. Or, it could be the rect tried to cross the anti-meridian, and once we fix that, this case will also be fixed. Finally, it could also be we simply have to floor the the lat after normalizing. Ie before normalizing we get a ridiculous lat, say 1000.0, which normLat cannot handle since it can only "undo" at most "off by 90". I think normLat may need to be changed so that if it still sees lat/lng out of bounds after doing it's normalization, it simply floors the resulting lat? Can you add a test case for this case as well? We may as well get test cases for all the problems we're uncovering... then as we iterate on the patches we can see if they are fixed.

        Show
        Michael McCandless added a comment - OK the new normLat looks like it handles crossing the pole now, by flipping the lng; thanks. Can you add a unit test that confirms this was a problem before, and that the new lng-flipping resolves it? Probably if you just modify TestCartesian, to add a new point, and then add a new test cast that starts from a lat/lng that's near the pole, you could get the issue to happen & be resolved. Also, can use "svn diff" and post the resulting output (instead of the whole copy of each source file)? Thanks. The current Spatial ONLY works for one hemisphere at a time. Actually, shouldn't LLRect work fine if a rect crosses the equator? Or, a pole? What I think it cannot handle is crossing the anti-meridian? (Because, when that projects out, you'll need 2 rectangles, on opposite sides, to handle it?). I'm not sure what LLRect should even do if it finds it "needs" to cross the anti-meridian. If it had the ability to return 2 rectangles I think it could be fixed. Or are you saying there are further limitations in contrib/spatial (ie, not caused by LLRect) that cause it to only work within one hemisphere? As for the "we get no results when radius is very large" bug, I like the theory that the corners need to be flipped. Or, it could be the rect tried to cross the anti-meridian, and once we fix that, this case will also be fixed. Finally, it could also be we simply have to floor the the lat after normalizing. Ie before normalizing we get a ridiculous lat, say 1000.0, which normLat cannot handle since it can only "undo" at most "off by 90". I think normLat may need to be changed so that if it still sees lat/lng out of bounds after doing it's normalization, it simply floors the resulting lat? Can you add a test case for this case as well? We may as well get test cases for all the problems we're uncovering... then as we iterate on the patches we can see if they are fixed.
        Hide
        Bill Bell added a comment -

        I did some additional research. The current Spatial ONLY works for one hemisphere at a time. It does a simple min/max for lat/long measurements. This makes the whole solution not useful between one hemisphere and another. Specifically Rectangle.java, getBoundary, etc needs to work on a circle. The first step is to build a rectangle when lat goes from -90 to +89, and long goes from -180 to +179, etc.

        new Rectangle(ll.getLng(), ll.getLat(), ur.getLng(), ur.getLat())

        At least LLRect appears correct now... Next step is to fix the CartesianPolyFilterBuilder.

        Show
        Bill Bell added a comment - I did some additional research. The current Spatial ONLY works for one hemisphere at a time. It does a simple min/max for lat/long measurements. This makes the whole solution not useful between one hemisphere and another. Specifically Rectangle.java, getBoundary, etc needs to work on a circle. The first step is to build a rectangle when lat goes from -90 to +89, and long goes from -180 to +179, etc. new Rectangle(ll.getLng(), ll.getLat(), ur.getLng(), ur.getLat()) At least LLRect appears correct now... Next step is to fix the CartesianPolyFilterBuilder.
        Hide
        Bill Bell added a comment -

        Added flipping for > 90 degrees if needed. See comment.

        Show
        Bill Bell added a comment - Added flipping for > 90 degrees if needed. See comment.
        Hide
        Bill Bell added a comment - - edited

        Everything is working except when you use a large area like 10000 miles. I get no results at this distance when crossing the anti-meridian (180 degrees).

        Most of the time this is fine, but specifically when -181 becomes 178 there appears to be an issue somewhere else in the code and nothing is returned. I believe this code is good, the issue is somewhere else. Maybe lower left is no longer lower left, and upper right is no longer upper right? The box is probably too big for the other algorithms. Not sure what else to check. How it is being used? Regardless this section appears right.

        Start here: ctr 39.3209801,-111.0937311
        Distance: 7200

        boxCorners: before norm 22.100623434197477,21.15746490712925
        boxCorners: normLng 22.100623434197477,21.15746490712925
        boxCorners: distance: d 7200.0
        boxCorners: ctr 39.3209801,-111.0937311
        boxCorners: normLat 22.100623434197477,21.15746490712925
        boxCorners: before norm -43.22565169384456,-181.34791600031286 note -181
        boxCorners: normLng -43.22565169384456,178.65208399968714 Note 178
        boxCorners: distance: d 7200.0
        boxCorners: ctr 39.3209801,-111.0937311
        boxCorners: normLat -43.22565169384456,178.65208399968714
        corner 1054.4155877284288

        I do get results from Hawaii crossing this at 10,000 miles. This works:

        boxCorners: before norm 6.201324582593365,-0.012709669713800501
        boxCorners: normLng 6.201324582593365,-0.012709669713800501
        boxCorners: distance: d 10000.0
        boxCorners: ctr 19.8986819,-155.6658568
        boxCorners: normLat 6.201324582593365,-0.012709669713800501
        boxCorners: before norm -41.508634930577436,-302.4840293070323 note -302
        boxCorners: normLng -41.508634930577436,57.5159706929677 note 57
        boxCorners: distance: d 10000.0
        boxCorners: ctr 19.8986819,-155.6658568
        boxCorners: normLat -41.508634930577436,57.5159706929677
        corner 1464.4660940672625

        Note: This does not get any results. Note the 4.815339955430126 difference. Very weird.

        boxCorners: distance: d 10500.0
        boxCorners: ctr 19.8986819,-155.6658568
        boxCorners: normLat 0.8114618951495843,4.815339955430126
        boxCorners: before norm -37.88735182208723,-310.6222696081052
        boxCorners: normLng -37.88735182208723,49.37773039189477
        boxCorners: distance: d 10500.0
        boxCorners: ctr 19.8986819,-155.6658568
        boxCorners: normLat -37.88735182208723,49.37773039189477
        corner 1537.6893987706253

        Show
        Bill Bell added a comment - - edited Everything is working except when you use a large area like 10000 miles. I get no results at this distance when crossing the anti-meridian (180 degrees). Most of the time this is fine, but specifically when -181 becomes 178 there appears to be an issue somewhere else in the code and nothing is returned. I believe this code is good, the issue is somewhere else. Maybe lower left is no longer lower left, and upper right is no longer upper right? The box is probably too big for the other algorithms. Not sure what else to check. How it is being used? Regardless this section appears right. Start here: ctr 39.3209801,-111.0937311 Distance: 7200 boxCorners: before norm 22.100623434197477,21.15746490712925 boxCorners: normLng 22.100623434197477,21.15746490712925 boxCorners: distance: d 7200.0 boxCorners: ctr 39.3209801,-111.0937311 boxCorners: normLat 22.100623434197477,21.15746490712925 boxCorners: before norm -43.22565169384456,-181.34791600031286 note -181 boxCorners: normLng -43.22565169384456,178.65208399968714 Note 178 boxCorners: distance: d 7200.0 boxCorners: ctr 39.3209801,-111.0937311 boxCorners: normLat -43.22565169384456,178.65208399968714 corner 1054.4155877284288 I do get results from Hawaii crossing this at 10,000 miles. This works: boxCorners: before norm 6.201324582593365,-0.012709669713800501 boxCorners: normLng 6.201324582593365,-0.012709669713800501 boxCorners: distance: d 10000.0 boxCorners: ctr 19.8986819,-155.6658568 boxCorners: normLat 6.201324582593365,-0.012709669713800501 boxCorners: before norm -41.508634930577436,-302.4840293070323 note -302 boxCorners: normLng -41.508634930577436,57.5159706929677 note 57 boxCorners: distance: d 10000.0 boxCorners: ctr 19.8986819,-155.6658568 boxCorners: normLat -41.508634930577436,57.5159706929677 corner 1464.4660940672625 Note: This does not get any results. Note the 4.815339955430126 difference. Very weird. boxCorners: distance: d 10500.0 boxCorners: ctr 19.8986819,-155.6658568 boxCorners: normLat 0.8114618951495843,4.815339955430126 boxCorners: before norm -37.88735182208723,-310.6222696081052 boxCorners: normLng -37.88735182208723,49.37773039189477 boxCorners: distance: d 10500.0 boxCorners: ctr 19.8986819,-155.6658568 boxCorners: normLat -37.88735182208723,49.37773039189477 corner 1537.6893987706253
        Hide
        Michael McCandless added a comment -

        Thanks for the updated patch Bill!

        That's a good improvement (taking into account the varying miles per lng, depending on lat), but isn't that fix orthogonal to the normalization issue? Ie, one could still easily overflow lat or lng with a large enough miles. EG, I added 6000 miles as a testcase in TestCartesian, and if I turn off the normalization, it hits the same exception (Illegal lattitude value 94.77745787739758).

        And I'm still concerned that the normalization fails to properly "cross" the north (or south) pole, by flipping the lng whenever the lat is too high; instead it seems to incorrectly "bounce" the point back? Am I missing something?

        Show
        Michael McCandless added a comment - Thanks for the updated patch Bill! That's a good improvement (taking into account the varying miles per lng, depending on lat), but isn't that fix orthogonal to the normalization issue? Ie, one could still easily overflow lat or lng with a large enough miles. EG, I added 6000 miles as a testcase in TestCartesian, and if I turn off the normalization, it hits the same exception (Illegal lattitude value 94.77745787739758). And I'm still concerned that the normalization fails to properly "cross" the north (or south) pole, by flipping the lng whenever the lat is too high; instead it seems to incorrectly "bounce" the point back? Am I missing something?
        Hide
        Bill Bell added a comment - - edited

        Michael - Please rerun your tests.

        The 2 normalization functions probably are now not needed, but they are there "as an added check"...

        This algorithm is standard, several web sites use it from Haversine. One example is at "Destination point given distance and bearing from start point" at http://www.movable-type.co.uk/scripts/latlong.html

        Thanks.

        Show
        Bill Bell added a comment - - edited Michael - Please rerun your tests. The 2 normalization functions probably are now not needed, but they are there "as an added check"... This algorithm is standard, several web sites use it from Haversine. One example is at "Destination point given distance and bearing from start point" at http://www.movable-type.co.uk/scripts/latlong.html Thanks.
        Hide
        Bill Bell added a comment -

        Large distance fixer

        Show
        Bill Bell added a comment - Large distance fixer
        Hide
        Bill Bell added a comment -

        I did some additional testing, and here is the new fix that works. The issue is the number of miles per Long is dependent on Latitude. The code checks it but only uses this value from the center point. I converted to the standard way to calculate distances for the Earth, assume 45 and 225 degree right angles. This is working using very large distances.

        See new code.

        Not sure if you want the prints.

        Show
        Bill Bell added a comment - I did some additional testing, and here is the new fix that works. The issue is the number of miles per Long is dependent on Latitude. The code checks it but only uses this value from the center point. I converted to the standard way to calculate distances for the Earth, assume 45 and 225 degree right angles. This is working using very large distances. See new code. Not sure if you want the prints.
        Hide
        Michael McCandless added a comment -

        So, here's one thing that worries me about this change... note that my grasp of geographic math is very tenuous at best, so what follows could be pure folly:

        Say the latitude must wrap around, because it's 110. This means (I think) that the this point has crossed over the north pole.

        With this patch, we'll simply map that 110 to 90 - (110 - 90) = 70.

        But shouldn't we also be flipping the longitude 180 degrees whenever the latitude crosses the north pole? Ie, in crossing the north pole, the point has moved to the other side of the earth. We aren't, with the current patch, which I think means the normalization simply bounces the point back where it came from, which doesn't seem right?

        Also, the code has this spooky TODO which looks relevant to this discussion:

            // TODO: Prob only works in northern hemisphere?
        

        I'm a little too nervous to commit this patch as is... I'm going to clear it from 2.9 for now. I'm hoping someone with a strong grasp of lat/long math can step in and bring some clarity here, in time for 2.9...

        Show
        Michael McCandless added a comment - So, here's one thing that worries me about this change... note that my grasp of geographic math is very tenuous at best, so what follows could be pure folly: Say the latitude must wrap around, because it's 110. This means (I think) that the this point has crossed over the north pole. With this patch, we'll simply map that 110 to 90 - (110 - 90) = 70. But shouldn't we also be flipping the longitude 180 degrees whenever the latitude crosses the north pole? Ie, in crossing the north pole, the point has moved to the other side of the earth. We aren't, with the current patch, which I think means the normalization simply bounces the point back where it came from, which doesn't seem right? Also, the code has this spooky TODO which looks relevant to this discussion: // TODO: Prob only works in northern hemisphere? I'm a little too nervous to commit this patch as is... I'm going to clear it from 2.9 for now. I'm hoping someone with a strong grasp of lat/long math can step in and bring some clarity here, in time for 2.9...
        Hide
        Michael McCandless added a comment -

        I was able to get TestCartesian to hit the issue, but adding 6000.0 radius into its tests, and confirmed that the patch fixed it.

        I also changed the two if's in each of the new norm methods to be if / else if instead (they are exclusive, I think).

        But unfortunately my lat/lng arithmetic is rather rusty – I don't fully understand the normalization that's being added, here. Can someone (Patrick?) check this?

        Also, what happens if radius is absurdly large and the resulting lat/lngs are ridiculous (say 1000.0)?

        Show
        Michael McCandless added a comment - I was able to get TestCartesian to hit the issue, but adding 6000.0 radius into its tests, and confirmed that the patch fixed it. I also changed the two if's in each of the new norm methods to be if / else if instead (they are exclusive, I think). But unfortunately my lat/lng arithmetic is rather rusty – I don't fully understand the normalization that's being added, here. Can someone (Patrick?) check this? Also, what happens if radius is absurdly large and the resulting lat/lngs are ridiculous (say 1000.0)?
        Hide
        Bill Bell added a comment -

        Fixed code - needs to be put in a release (trunk)

        Show
        Bill Bell added a comment - Fixed code - needs to be put in a release (trunk)

          People

          • Assignee:
            Michael McCandless
            Reporter:
            Bill Bell
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development