Lucene - Core
  1. Lucene - Core
  2. LUCENE-3795

Replace spatial contrib module with LSP's spatial-lucene module

    Details

    • Type: New Feature New Feature
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 4.0-ALPHA
    • Component/s: modules/spatial
    • Labels:
      None
    • Lucene Fields:
      New

      Description

      I propose that Lucene's spatial contrib module be replaced with the spatial-lucene module within Lucene Spatial Playground (LSP). LSP has been in development for approximately 1 year by David Smiley, Ryan McKinley, and Chris Male and we feel it is ready. LSP is here: http://code.google.com/p/lucene-spatial-playground/ and the spatial-lucene module is intuitively in svn/trunk/spatial-lucene/.

      I'll add more comments to prevent the issue description from being too long.

        Issue Links

          Activity

          Hide
          David Smiley added a comment -

          LSP is comprised of several modules:

          • spatial-lucene: The heart of the project.
          • spatial-solr: Solr support, notably field types using spatial-lucene.
          • spatial-extras: An extension of spatial-lucene that uses JTS (LGPL licensed) for polygon support.
          • spatial-demo: A demonstration web UI using OpenLayers, Solr, Wicket, and the other LSP modules.

          The spatial-solr module of LSP can be considered in another issue following the conclusion of this one. The other modules aren't being considered for incorporation into Lucene/Solr.

          LSP is largely new code although some of it originated using chunks of the existing Lucene spatial contrib module and SOLR-2155 (A recursive PrefixTree/Trie algorithm using geohashes). It's fair to say this is a superset and descendent of SOLR-2155 but with a real framework around it and plenty of refactorings and tests.

          I ran Atlassian's Clover code coverage to get some statistics of this spatial-lucene module of LSP:

          • LOC: 6,605, NCLOC: 3,959
          • Packages: 18, Classes: 70
          • Code coverage: 53%

          The code coverage surprises me a little... perhaps the number is higher when the spatial-solr module gets involved which uses more of the classes then the tests do here alone.

          Show
          David Smiley added a comment - LSP is comprised of several modules: spatial-lucene: The heart of the project. spatial-solr: Solr support, notably field types using spatial-lucene. spatial-extras: An extension of spatial-lucene that uses JTS (LGPL licensed) for polygon support. spatial-demo: A demonstration web UI using OpenLayers, Solr, Wicket, and the other LSP modules. The spatial-solr module of LSP can be considered in another issue following the conclusion of this one. The other modules aren't being considered for incorporation into Lucene/Solr. LSP is largely new code although some of it originated using chunks of the existing Lucene spatial contrib module and SOLR-2155 (A recursive PrefixTree/Trie algorithm using geohashes). It's fair to say this is a superset and descendent of SOLR-2155 but with a real framework around it and plenty of refactorings and tests. I ran Atlassian's Clover code coverage to get some statistics of this spatial-lucene module of LSP: LOC: 6,605, NCLOC: 3,959 Packages: 18, Classes: 70 Code coverage: 53% The code coverage surprises me a little... perhaps the number is higher when the spatial-solr module gets involved which uses more of the classes then the tests do here alone.
          Hide
          David Smiley added a comment -

          The spatial-lucene module of LSP has 3 main packages: 'base', 'strategies', and 'benchmark'. It also has a fair amount of tests.

          Base

          Major pieces in 'base':

          • SpatialContext interface and simple implementation
          • Distance math code
          • Shapes interface and implementations
          • PrefixTree/Trie (grid) interface and implementations (e.g. geohash)

          Strategies

          The "strategies" portion of this module contains spatial indexing/search implementations using Lucene.
          Major interfaces (just one):

          • SpatialStrategy (including abstract PrefixGridStrategy)
            Major implementations:
            • RecursivePrefixTreeStrategy
              • This is the main strategy, based on SOLR-2155. It uses an abstract class SpatialPrefixTree which has geohash and quadtree implementations.
            • TermQueryPrefixTreeStrategy
            • TwoDoubleStrategy
              • Akin to Solr's LatLonType, although some edge cases not yet implemented.

          Benchmarking

          Benchmarking is a TBD; there's the start of some code there but nothing real. 1 year ago I did benchmark SOLR-2155 (with great results) and posted my benchmark code here LUCENE-2844 in the interest of transparency.

          Testing

          Testing so far hasn't been aimed directly at increasing code coverage, it's been aimed at finding nasty corner cases in spatial. Spatial code is highly prone to edge cases.

          Show
          David Smiley added a comment - The spatial-lucene module of LSP has 3 main packages: 'base', 'strategies', and 'benchmark'. It also has a fair amount of tests. Base Major pieces in 'base': SpatialContext interface and simple implementation Distance math code Shapes interface and implementations PrefixTree/Trie (grid) interface and implementations (e.g. geohash) Strategies The "strategies" portion of this module contains spatial indexing/search implementations using Lucene. Major interfaces (just one): SpatialStrategy (including abstract PrefixGridStrategy) Major implementations: RecursivePrefixTreeStrategy This is the main strategy, based on SOLR-2155 . It uses an abstract class SpatialPrefixTree which has geohash and quadtree implementations. TermQueryPrefixTreeStrategy TwoDoubleStrategy Akin to Solr's LatLonType, although some edge cases not yet implemented. Benchmarking Benchmarking is a TBD; there's the start of some code there but nothing real. 1 year ago I did benchmark SOLR-2155 (with great results) and posted my benchmark code here LUCENE-2844 in the interest of transparency. Testing Testing so far hasn't been aimed directly at increasing code coverage, it's been aimed at finding nasty corner cases in spatial. Spatial code is highly prone to edge cases.
          Hide
          David Smiley added a comment - - edited

          Features

          The main goals of LSP is to be a great framework to plug in spatial search algorithms and shape implementations. It of course includes good implementations of these key abstractions. Here are some key features, most of which related to using RecursivePrefixTreeStrategy with geohashes:

          • Multi-valued fields
          • Index shapes that have area (e.g. not just points)
            Tests have yet to be added for this.
          • No special RAM caches for filtering, just standard term index
            Unlike Solr's LatLonType which needs to cache all points in RAM if the query shape is a circle
          • Fast filtering
            Although SOLR-2155 has been proven, technically LSP hasn't. 3rd party anecodes re-inforce this claim.
          • Multi-value sort
            Based on closest index point to center of query shape. Distances are returned via the score of an LSP query.
          • Specify precision of query shape and index shape
            Thereby allowing for faster filtering tunable precision
          • Multiple distance algorithms:
            • Spherical: Law of Cosines, Haversine, Vincenty
            • Cartesian: Pythagorean Theorem
          • Cartesian (2d flat) & Geospatial sphere models

          Todo

          There are many things I want to improve and add but in my view there isn't anything truly making this non-committable. Chris has raised concerns that the other committers will want to see benchmark results before accepting this. I'll leave that for you (the other committers) to decide.

          And I also heard that some committers are unsure wether Lucene should have a spatial module at all. However there is certainly demand for it, at least at the Solr level. Furthermore, there are some non-spatial use cases of the spatial module. One interesting use-case is RecursivePrefixTreeStrategy's (RPTS) unique ability to index shapes with area. If you had a requirement to index a variable number of time durations, then unlike Lucene's trie numeric support in which only discrete numbers are supported, RPTS could be used with x being time and y being unused. Buy the way, PrefixTree and Trie are synonymous words.

          Show
          David Smiley added a comment - - edited Features The main goals of LSP is to be a great framework to plug in spatial search algorithms and shape implementations. It of course includes good implementations of these key abstractions. Here are some key features, most of which related to using RecursivePrefixTreeStrategy with geohashes: Multi-valued fields Index shapes that have area (e.g. not just points) Tests have yet to be added for this. No special RAM caches for filtering, just standard term index Unlike Solr's LatLonType which needs to cache all points in RAM if the query shape is a circle Fast filtering Although SOLR-2155 has been proven, technically LSP hasn't. 3rd party anecodes re-inforce this claim. Multi-value sort Based on closest index point to center of query shape. Distances are returned via the score of an LSP query. Specify precision of query shape and index shape Thereby allowing for faster filtering tunable precision Multiple distance algorithms: Spherical: Law of Cosines, Haversine, Vincenty Cartesian: Pythagorean Theorem Cartesian (2d flat) & Geospatial sphere models Todo There are many things I want to improve and add but in my view there isn't anything truly making this non-committable. Chris has raised concerns that the other committers will want to see benchmark results before accepting this. I'll leave that for you (the other committers) to decide. And I also heard that some committers are unsure wether Lucene should have a spatial module at all. However there is certainly demand for it, at least at the Solr level. Furthermore, there are some non-spatial use cases of the spatial module. One interesting use-case is RecursivePrefixTreeStrategy's (RPTS) unique ability to index shapes with area. If you had a requirement to index a variable number of time durations, then unlike Lucene's trie numeric support in which only discrete numbers are supported, RPTS could be used with x being time and y being unused. Buy the way, PrefixTree and Trie are synonymous words.
          Hide
          Simon Willnauer added a comment -

          wow this is a lot of stuff. we certainly need a code donation for this. without getting into details +1 from my side. I think lucene desperatly needs spatial support... it should be a module IMO. we should drop the stuff we have an get this in shape ie. into a module. I am not sure about the LGPL stuff I guess we should try to integrate everything else and if we really want or if there is a way to integrate the LGPL stuff we can take care of this later!

          Show
          Simon Willnauer added a comment - wow this is a lot of stuff. we certainly need a code donation for this. without getting into details +1 from my side. I think lucene desperatly needs spatial support... it should be a module IMO. we should drop the stuff we have an get this in shape ie. into a module. I am not sure about the LGPL stuff I guess we should try to integrate everything else and if we really want or if there is a way to integrate the LGPL stuff we can take care of this later!
          Hide
          David Smiley added a comment -

          What constitutes a "code donation"? By the way, I've gone through the proper channels with my employer with regard to SOLR-2155 and LSP. MITRE has no copyright on this code; I've marked it all as ASF.

          Show
          David Smiley added a comment - What constitutes a "code donation"? By the way, I've gone through the proper channels with my employer with regard to SOLR-2155 and LSP. MITRE has no copyright on this code; I've marked it all as ASF.
          Hide
          Robert Muir added a comment -

          Simon do we really need a code grant here?

          Its my understanding (correct me if i am wrong): the developers involved (David, Ryan, Chris) are all committers
          with iCLA on file, so is it really any different than any other patch from that perspective?

          As far as LGPL, according to David's description and the title of this jira issue (possible i did not interpret it correctly,
          correct me if so), the he wants to replace lucene/contrib/spatial with the spatial-lucene project, and that
          it has no LGPL ties at all, (only spatial-extras does).

          Without looking at any code myself, if thats really the case I'm +1 on principle because it means we basically
          have an improved spatial module for lucene core with no catch at all. The current code has not seen much maintenance.

          (And i agree, we should be shooting for a proper module/ here, not a contrib).

          Show
          Robert Muir added a comment - Simon do we really need a code grant here? Its my understanding (correct me if i am wrong): the developers involved (David, Ryan, Chris) are all committers with iCLA on file, so is it really any different than any other patch from that perspective? As far as LGPL, according to David's description and the title of this jira issue (possible i did not interpret it correctly, correct me if so), the he wants to replace lucene/contrib/spatial with the spatial-lucene project, and that it has no LGPL ties at all, (only spatial-extras does). Without looking at any code myself, if thats really the case I'm +1 on principle because it means we basically have an improved spatial module for lucene core with no catch at all. The current code has not seen much maintenance. (And i agree, we should be shooting for a proper module/ here, not a contrib).
          Hide
          Mark Miller added a comment -

          ... do we really need a code grant here?

          I think if you go by the letter, since it was 'developed' outside of the Apache ecosystem, we want a code grant. I wonder where apache-extras fits there though.

          Personally, if it was all only developed by committers in a public repo, I don't have a problem with being practical - FWIW though - I'm just one guy INAL.

          Show
          Mark Miller added a comment - ... do we really need a code grant here? I think if you go by the letter, since it was 'developed' outside of the Apache ecosystem, we want a code grant. I wonder where apache-extras fits there though. Personally, if it was all only developed by committers in a public repo, I don't have a problem with being practical - FWIW though - I'm just one guy INAL.
          Hide
          Jan Høydahl added a comment -

          Impressive piece of work! Given license stuff is ok, here is my
          +1

          Show
          Jan Høydahl added a comment - Impressive piece of work! Given license stuff is ok, here is my +1
          Hide
          Uwe Schindler added a comment -

          Cool work! I scanned the code quickly and it seems to fit much better than the current spatial!

          I have some suggestions regarding performance; BooleanQuery usage and related inconsistency with BQ scoring (with coord) in the different strategies; also found some caching problems (AtomicReader is key to cache not AtomicReader.getCoreCacheKey, so new deleted docs after reopen invalidate the cache), but I would prefer to discuss that here once the patch is provided on Lucene's JIRA.

          Show
          Uwe Schindler added a comment - Cool work! I scanned the code quickly and it seems to fit much better than the current spatial! I have some suggestions regarding performance; BooleanQuery usage and related inconsistency with BQ scoring (with coord) in the different strategies; also found some caching problems (AtomicReader is key to cache not AtomicReader.getCoreCacheKey, so new deleted docs after reopen invalidate the cache), but I would prefer to discuss that here once the patch is provided on Lucene's JIRA.
          Hide
          Chris Male added a comment -

          Huge +1

          Thanks so much David for opening this issue and getting the code to a point where it can be contributed. I'm really excited to see this brought into the fold and glad to see support from others.

          As far as LGPL, according to David's description and the title of this jira issue (possible i did not interpret it correctly,
          correct me if so), the he wants to replace lucene/contrib/spatial with the spatial-lucene project, and that
          it has no LGPL ties at all, (only spatial-extras does).

          Absolutely. The portion of the codebase which uses LGPL code is entirely optional and decoupled from the rest of the code. From a functional perspective, as David says, its only really related to polygon support which is hugely powerful but can exist somewhere else if needs be.

          Show
          Chris Male added a comment - Huge +1 Thanks so much David for opening this issue and getting the code to a point where it can be contributed. I'm really excited to see this brought into the fold and glad to see support from others. As far as LGPL, according to David's description and the title of this jira issue (possible i did not interpret it correctly, correct me if so), the he wants to replace lucene/contrib/spatial with the spatial-lucene project, and that it has no LGPL ties at all, (only spatial-extras does). Absolutely. The portion of the codebase which uses LGPL code is entirely optional and decoupled from the rest of the code. From a functional perspective, as David says, its only really related to polygon support which is hugely powerful but can exist somewhere else if needs be.
          Hide
          Chris Male added a comment -

          but I would prefer to discuss that here once the patch is provided on Lucene's JIRA.

          Is it best to create a patch here and iterate on any problems, or create a branch and work through them there?

          Show
          Chris Male added a comment - but I would prefer to discuss that here once the patch is provided on Lucene's JIRA. Is it best to create a patch here and iterate on any problems, or create a branch and work through them there?
          Hide
          David Smiley added a comment -

          FYI the code coverage figure is erroneous, Clover didn't recognize some inner classes extending other tests as tests. Using IntelliJ IDEA Ultimate's built-in coverage, it's 63% (as counted per line), and I believe its higher once the spatial-solr module is brought into the mix which has a bunch of tests.

          Uwe, I'm very interested in your input on anything to make the code better.

          Given the volume of code, I believe a feature branch makes the most sense instead of a humungous patch file.

          Show
          David Smiley added a comment - FYI the code coverage figure is erroneous, Clover didn't recognize some inner classes extending other tests as tests. Using IntelliJ IDEA Ultimate's built-in coverage, it's 63% (as counted per line), and I believe its higher once the spatial-solr module is brought into the mix which has a bunch of tests. Uwe, I'm very interested in your input on anything to make the code better. Given the volume of code, I believe a feature branch makes the most sense instead of a humungous patch file.
          Hide
          Uwe Schindler added a comment -

          From a functional perspective, as David says, its only really related to polygon support which is hugely powerful but can exist somewhere else if needs be.

          Can we add polygon support using the java.awt.geom.Area class? It has lots of code vor overlapping polygons and similar stuff... My own gazeteer using Lucene is based on this. In general we could ad another strategy to handle this (java.awt.geom implements lots of different shape types, in contrast to "normal" AWT double not integer based).

          Show
          Uwe Schindler added a comment - From a functional perspective, as David says, its only really related to polygon support which is hugely powerful but can exist somewhere else if needs be. Can we add polygon support using the java.awt.geom.Area class? It has lots of code vor overlapping polygons and similar stuff... My own gazeteer using Lucene is based on this. In general we could ad another strategy to handle this (java.awt.geom implements lots of different shape types, in contrast to "normal" AWT double not integer based).
          Hide
          Michael McCandless added a comment -

          +1, Lucene badly needs good ootb spatial search.

          I think a branch makes complete sense... we've made branches for much smaller things!

          Show
          Michael McCandless added a comment - +1, Lucene badly needs good ootb spatial search. I think a branch makes complete sense... we've made branches for much smaller things!
          Hide
          David Smiley added a comment -

          Uwe,
          Thanks for bringing java.awt.geom.Area to my attention; I haven't noticed it before. It looks workable, and the code for it looks impressive to me. As an aside, JTS has a particularly scalable polygon scaling to many vertexes which get stored in an in-memory RTree – although I don't think this feature is as pertinent for user-input polygons which would have a small number. It is unfortunate that sun.awt.geom.Crossing is not exposed from Area, since computing it is not particularly cheap and LSP will ask Area two things – intersects() and contains() given a lat-lon box, and Area will compute the same Crossing twice. Use of this in LSP would not be a "stategy", it would be a subclass of SpatialContext which acts as a factory for shapes. Speaking of which, I'm thinking of renaming the "simple" package to be something else like "impl" since some of the implementations are decidedly not simple – GeoCircleImpl case in point, and the addition of a polygon would seal that point.

          I look forward to working with you more Uwe. It appears we do a lot of similar work – geospatial and trie stuff.

          Show
          David Smiley added a comment - Uwe, Thanks for bringing java.awt.geom.Area to my attention; I haven't noticed it before. It looks workable, and the code for it looks impressive to me. As an aside, JTS has a particularly scalable polygon scaling to many vertexes which get stored in an in-memory RTree – although I don't think this feature is as pertinent for user-input polygons which would have a small number. It is unfortunate that sun.awt.geom.Crossing is not exposed from Area, since computing it is not particularly cheap and LSP will ask Area two things – intersects() and contains() given a lat-lon box, and Area will compute the same Crossing twice. Use of this in LSP would not be a "stategy", it would be a subclass of SpatialContext which acts as a factory for shapes. Speaking of which, I'm thinking of renaming the "simple" package to be something else like "impl" since some of the implementations are decidedly not simple – GeoCircleImpl case in point, and the addition of a polygon would seal that point. I look forward to working with you more Uwe. It appears we do a lot of similar work – geospatial and trie stuff.
          Hide
          Bill Bell added a comment -

          +1 getting polygon support ootb is huge as is geohashing and multivalued lat longs. Next step Solr...

          Show
          Bill Bell added a comment - +1 getting polygon support ootb is huge as is geohashing and multivalued lat longs. Next step Solr...
          Hide
          Ryan McKinley added a comment -

          Thanks for pushing this forward David! (sorry i have been offline recently... just had a baby!)

          We should defiantly make a branch for this – getting things integrated with the build system will be non-trivial.

          Re code grant? given that all developers of this code are lucene committers and intend for this be contributed to ASF, I don't think it is necessary. But if we need more paperwork, that is OK too.

          Re polygons / AWT / JTS? I hope this code lets us use an implementation that is appropriate for the need. In some cases, simple math or java.awt.geom may be fine, in others JTS will be necessary. My fear is that with JTS out of the core build/test system, JTS will be a secondary concern. Through this process, I will continue to make sure any design decisions don't exclude a solid JTS solution.

          Show
          Ryan McKinley added a comment - Thanks for pushing this forward David! (sorry i have been offline recently... just had a baby!) We should defiantly make a branch for this – getting things integrated with the build system will be non-trivial. Re code grant? given that all developers of this code are lucene committers and intend for this be contributed to ASF, I don't think it is necessary. But if we need more paperwork, that is OK too. Re polygons / AWT / JTS? I hope this code lets us use an implementation that is appropriate for the need. In some cases, simple math or java.awt.geom may be fine, in others JTS will be necessary. My fear is that with JTS out of the core build/test system, JTS will be a secondary concern. Through this process, I will continue to make sure any design decisions don't exclude a solid JTS solution.
          Hide
          Ryan McKinley added a comment -

          David – do you want to go ahead and make a branch and integrate:
          http://lucene-spatial-playground.googlecode.com/svn/trunk/spatial-lucene/

          I still think we want a .jar file for the spatial code/API that does not need lucene. This will be important for non-lucene clients that should be able to deal with real classes rather then strings.

          Show
          Ryan McKinley added a comment - David – do you want to go ahead and make a branch and integrate: http://lucene-spatial-playground.googlecode.com/svn/trunk/spatial-lucene/ I still think we want a .jar file for the spatial code/API that does not need lucene. This will be important for non-lucene clients that should be able to deal with real classes rather then strings.
          Hide
          David Smiley added a comment -

          I created a branch https://svn.apache.org/repos/asf/lucene/dev/branches/lucene3795_lsp_spatial_module from r1291350 (the most recent version with passing tests as indicated by Jenkins)

          Show
          David Smiley added a comment - I created a branch https://svn.apache.org/repos/asf/lucene/dev/branches/lucene3795_lsp_spatial_module from r1291350 (the most recent version with passing tests as indicated by Jenkins)
          Hide
          David Smiley added a comment -

          Branch Status Update:

          • LSP Lucene spatial is in as a module, actually as two modules, spatial/base & spatial/strategy. That complicates the build but Chris & Ryan insist. The maven build works.
          • Old spatial contrib is gone.
          • Solr is updated to use the new module. 80% was trivial changes, 20% pretty easy. Nothing hard. Tests pass.
          • IntelliJ IDEA build seems done (I use IDEA)
          • Eclipse build is probably done (Ryan worked on it, I don't use it)
          • Maven build is done. That was easy!
          • Ant build in progress. The Solr side isn't seeing the spatial libs.
          Show
          David Smiley added a comment - Branch Status Update: LSP Lucene spatial is in as a module, actually as two modules, spatial/base & spatial/strategy. That complicates the build but Chris & Ryan insist. The maven build works. Old spatial contrib is gone. Solr is updated to use the new module. 80% was trivial changes, 20% pretty easy. Nothing hard. Tests pass. IntelliJ IDEA build seems done (I use IDEA) Eclipse build is probably done (Ryan worked on it, I don't use it) Maven build is done. That was easy! Ant build in progress. The Solr side isn't seeing the spatial libs.
          Hide
          Chris Male added a comment -

          LSP Lucene spatial is in as a module, actually as two modules, spatial/base & spatial/strategy. That complicates the build but Chris & Ryan insist. The maven build works.

          I don't recall insisting that here. I think we should do what's best in this situation. If having two sub modules is causing too much difficulty for no benefit, then +1 to reducing them to one.

          Show
          Chris Male added a comment - LSP Lucene spatial is in as a module, actually as two modules, spatial/base & spatial/strategy. That complicates the build but Chris & Ryan insist. The maven build works. I don't recall insisting that here. I think we should do what's best in this situation. If having two sub modules is causing too much difficulty for no benefit, then +1 to reducing them to one.
          Hide
          David Smiley added a comment -

          Chris, in an email months ago you declared interest in a client library jar, and Ryan absolutely insisted. The difficulties have been figured out.

          There is no benchmarking code. LSP did have about 4 classes in a benchmarking package but AFAIK it was draft in-progress (year old code now) and had probably never been executed before. Consequently, I chose not to copy those classes over. Once there is benchmarking, it will probably exist here: /modules/spatial/benchmarking alongside "base" and "strategy". Admittedly that seems a bit heavyweight (i.e. its own module) but it wouldn't truly belong anywhere else, although there probably wouldn't bee much code to it. I don't feel strongly about where it should go.

          At this point, I think the branch is in committable shape. To the best of my ability to know, the Ant and Maven builds work, same for Eclipse & IntelliJ IDEA.

          Show
          David Smiley added a comment - Chris, in an email months ago you declared interest in a client library jar, and Ryan absolutely insisted. The difficulties have been figured out. There is no benchmarking code. LSP did have about 4 classes in a benchmarking package but AFAIK it was draft in-progress (year old code now) and had probably never been executed before. Consequently, I chose not to copy those classes over. Once there is benchmarking, it will probably exist here: /modules/spatial/benchmarking alongside "base" and "strategy". Admittedly that seems a bit heavyweight (i.e. its own module) but it wouldn't truly belong anywhere else, although there probably wouldn't bee much code to it. I don't feel strongly about where it should go. At this point, I think the branch is in committable shape . To the best of my ability to know, the Ant and Maven builds work, same for Eclipse & IntelliJ IDEA.
          Hide
          Robert Muir added a comment -

          What is the advantage of two spatial modules? Can I run spatial queries with just base by itself?

          For benchmarking code, why not put it in the benchmarking package and have benchmark
          depend on it (thats how all other modules, highlighter, analyzers, anything else we
          benchmark works)

          Show
          Robert Muir added a comment - What is the advantage of two spatial modules? Can I run spatial queries with just base by itself? For benchmarking code, why not put it in the benchmarking package and have benchmark depend on it (thats how all other modules, highlighter, analyzers, anything else we benchmark works)
          Hide
          Ryan McKinley added a comment -

          What is the advantage of two spatial modules? Can I run spatial queries with just base by itself?

          The real advantage is that client code (solrj etc) can know about Shapes/Operations without needing to include lucene. From spatial-base, you can build queries and understand the results; but can't run the query.

          I need my client code to use a real API rather then needing to build the correct String query representations and parsing results.

          Show
          Ryan McKinley added a comment - What is the advantage of two spatial modules? Can I run spatial queries with just base by itself? The real advantage is that client code (solrj etc) can know about Shapes/Operations without needing to include lucene. From spatial-base, you can build queries and understand the results; but can't run the query. I need my client code to use a real API rather then needing to build the correct String query representations and parsing results.
          Hide
          David Smiley added a comment -

          Rob, thanks for your suggestion/opinion on putting the spatial benchmarking code into the benchmarking module. Works for me – one less module.

          Ryan, how would you feel about a single combined spatial module that you would use from your remote SolrJ client? Yes, it wouldn't be great that the jar would be half filled with classes you don't need (the spatial strategies coded against the Lucene API) but is that really such a big deal? It would be annoying to configure your maven build to not include the transitive dependencies but it's doable, and perhaps we could mark lucene as an optional dependency in a combined spatial module. In practice, anyone using this spatial module on the server will certainly have lucene already.

          Show
          David Smiley added a comment - Rob, thanks for your suggestion/opinion on putting the spatial benchmarking code into the benchmarking module. Works for me – one less module. Ryan, how would you feel about a single combined spatial module that you would use from your remote SolrJ client? Yes, it wouldn't be great that the jar would be half filled with classes you don't need (the spatial strategies coded against the Lucene API) but is that really such a big deal? It would be annoying to configure your maven build to not include the transitive dependencies but it's doable, and perhaps we could mark lucene as an optional dependency in a combined spatial module. In practice, anyone using this spatial module on the server will certainly have lucene already.
          Hide
          Ryan McKinley added a comment -

          I guess I don't see the problem with having multiple .jar files (aside from the ant setup effort)

          I'm fine with one .jar if we guarantee that the 'base' classes don't have compile time access to lucene classes. I'm sure there is some ant crazieness to compile half the project with different classpaths then bundle them together, but that seems more complex (and less clear) then two .jar files

          Show
          Ryan McKinley added a comment - I guess I don't see the problem with having multiple .jar files (aside from the ant setup effort) I'm fine with one .jar if we guarantee that the 'base' classes don't have compile time access to lucene classes. I'm sure there is some ant crazieness to compile half the project with different classpaths then bundle them together, but that seems more complex (and less clear) then two .jar files
          Hide
          Chris Male added a comment -

          Chris, in an email months ago you declared interest in a client library jar, and Ryan absolutely insisted. The difficulties have been figured out.

          Indeed I did but that was when we were developing this outside of Lucene, I'm now thinking what's best for Lucene and am open to any ideas.

          I need my client code to use a real API rather then needing to build the correct String query representations and parsing results.

          Are you able to give us some more information on what your client code needs? Is it just being able to instantiate a Shape and then convert it to a queryable String format?

          perhaps we could mark lucene as an optional dependency in a combined spatial module.

          +1 This seems like the best compromise

          I'm fine with one .jar if we guarantee that the 'base' classes don't have compile time access to lucene classes.

          I also agree with this and we should bare it in mind as we progress.

          Show
          Chris Male added a comment - Chris, in an email months ago you declared interest in a client library jar, and Ryan absolutely insisted. The difficulties have been figured out. Indeed I did but that was when we were developing this outside of Lucene, I'm now thinking what's best for Lucene and am open to any ideas. I need my client code to use a real API rather then needing to build the correct String query representations and parsing results. Are you able to give us some more information on what your client code needs? Is it just being able to instantiate a Shape and then convert it to a queryable String format? perhaps we could mark lucene as an optional dependency in a combined spatial module. +1 This seems like the best compromise I'm fine with one .jar if we guarantee that the 'base' classes don't have compile time access to lucene classes. I also agree with this and we should bare it in mind as we progress.
          Hide
          David Smiley added a comment -

          My preference is one module vs. multiple. Now that Ryan and Chris are cool with this, we can continue with that objective.

          Tomorrow night or sooner I'll get on merging them together.

          Show
          David Smiley added a comment - My preference is one module vs. multiple. Now that Ryan and Chris are cool with this, we can continue with that objective. Tomorrow night or sooner I'll get on merging them together.
          Hide
          David Smiley added a comment -

          Ryan and I chatted about this issue more and I didn't take consolidation steps yet. I'm pretty neutral, by the way – I see both sides.

          Another option occurred to me and I'm excited about the prospects because I think it's a good balance. To be clear, spatial-base has nothing to do with Lucene. It largely consists of shape interfaces with implementations, has some distance calculators like Haversine and other spatial calculations, and can (or at least should) parse and emit some dialect of WKT – a popular standard, extended where needed to represent shapes that aren't in WKT such as a circle. There's a good deal of testing too. It is certainly useful in its own right just as other spatial libraries are. To defend its existence when there are other spatial libraries, I'll point out a few things that make this more desirable than other 3rd party libraries:

          • ASL licensed; required for acceptence by the Lucene PMC
          • Geospatial orientation, not just 2D. FYI JTS is purely 2D.
          • Has shapes not found in other libraries like a point-distance (circle) shape. It's inexplicable to me why this isn't elsewhere. And this shape isn't just some POJO for a point & radius, there is sophisticated math for the various relations (e.g. disjoint, contains, etc.) of rectangle-circle intersection.
          • Performance oriented – it was developed concurrent with lucene spatial search algorithms and I try to keep this in mind.

          So how about spatial-base remains in the LSP project off-Lucene. LSP and this component will both probably receive a name change and possible re-hosting on Github. LSP (or whatever its eventual name is) will always need to exist any way because there are integration scenarios involving LGPL libraries that the Lucene PMC is uncomfortable with, and there is a nifty demo webapp too. The spatial-extras module could probably be merged with spatial-base, making testing easier and it's one less jar.

          If spatial-base becomes a 3rd party library required by a single Lucene spatial module, then that brings a simplicity to the code organization insofar as there is just one spatial module, not 2. It also means that the spatial module will be entirely focused on the intersection of Lucene & spatial, and not have other code unrelated to Lucene. When deployed, it would mean 2 jars, the spatial-base.jar (or whatever its renamed to) and lucene-spatial.jar. FYI Solr, at least for the moment, would only need the base one, not lucene-spatial.

          The down side is that both spatial-base and lucene-spatial are in-progress and are largely developed together, and so separating them to live on independent projects will bring about some extra burden in syncing them from time to time. This is reminiscent of the Lucene & Solr projects before they were merged. To mitigate this, our spatial team (me, Ryan, Chris) can initially focus on making changes to the public API of spatial-base to the point we like it even more and are less likely to change it.

          Show
          David Smiley added a comment - Ryan and I chatted about this issue more and I didn't take consolidation steps yet. I'm pretty neutral, by the way – I see both sides. Another option occurred to me and I'm excited about the prospects because I think it's a good balance. To be clear, spatial-base has nothing to do with Lucene. It largely consists of shape interfaces with implementations, has some distance calculators like Haversine and other spatial calculations, and can (or at least should) parse and emit some dialect of WKT – a popular standard, extended where needed to represent shapes that aren't in WKT such as a circle. There's a good deal of testing too. It is certainly useful in its own right just as other spatial libraries are. To defend its existence when there are other spatial libraries, I'll point out a few things that make this more desirable than other 3rd party libraries: ASL licensed; required for acceptence by the Lucene PMC Geospatial orientation, not just 2D. FYI JTS is purely 2D. Has shapes not found in other libraries like a point-distance (circle) shape. It's inexplicable to me why this isn't elsewhere. And this shape isn't just some POJO for a point & radius, there is sophisticated math for the various relations (e.g. disjoint, contains, etc.) of rectangle-circle intersection. Performance oriented – it was developed concurrent with lucene spatial search algorithms and I try to keep this in mind. So how about spatial-base remains in the LSP project off-Lucene. LSP and this component will both probably receive a name change and possible re-hosting on Github. LSP (or whatever its eventual name is) will always need to exist any way because there are integration scenarios involving LGPL libraries that the Lucene PMC is uncomfortable with, and there is a nifty demo webapp too. The spatial-extras module could probably be merged with spatial-base, making testing easier and it's one less jar. If spatial-base becomes a 3rd party library required by a single Lucene spatial module, then that brings a simplicity to the code organization insofar as there is just one spatial module, not 2. It also means that the spatial module will be entirely focused on the intersection of Lucene & spatial, and not have other code unrelated to Lucene. When deployed, it would mean 2 jars, the spatial-base.jar (or whatever its renamed to) and lucene-spatial.jar. FYI Solr, at least for the moment, would only need the base one, not lucene-spatial. The down side is that both spatial-base and lucene-spatial are in-progress and are largely developed together, and so separating them to live on independent projects will bring about some extra burden in syncing them from time to time. This is reminiscent of the Lucene & Solr projects before they were merged. To mitigate this, our spatial team (me, Ryan, Chris) can initially focus on making changes to the public API of spatial-base to the point we like it even more and are less likely to change it.
          Hide
          Ryan McKinley added a comment -

          I like the idea that spatial-base would be external to lucene, and included as a .jar file. This was my original proposal when starting to discuss this long ago.

          As is, the spatial library is quite useful on its own; I think it has the best chance of long term success outside of lucene. Outside lucene (ASF) it can have compile/test dependencies on JTS that make it more robust but still have strong ASL only runtime.

          Show
          Ryan McKinley added a comment - I like the idea that spatial-base would be external to lucene, and included as a .jar file. This was my original proposal when starting to discuss this long ago. As is, the spatial library is quite useful on its own; I think it has the best chance of long term success outside of lucene. Outside lucene (ASF) it can have compile/test dependencies on JTS that make it more robust but still have strong ASL only runtime.
          Hide
          Ryan McKinley added a comment -

          OK, I think the branch is ready to go.

          The one thing I don't like is that the spatial4j.jar gets included twice, once in the modules 'lib' directory and again in the solr lib directory. I could not figure out how to have the solr build compile and distribute this one

          Show
          Ryan McKinley added a comment - OK, I think the branch is ready to go. The one thing I don't like is that the spatial4j.jar gets included twice, once in the modules 'lib' directory and again in the solr lib directory. I could not figure out how to have the solr build compile and distribute this one
          Hide
          David Smiley added a comment -

          For those following along here, the former "spatial-base" module portion of this code is now an ASL licensed 3rd party jar dependency: http://spatial4j.com "Spatial4J" Basically half of LSP is there now going by this new name. The other half is here as the new lucene spatial module.

          I agree that the branch looks ready to be merged into trunk.

          Show
          David Smiley added a comment - For those following along here, the former "spatial-base" module portion of this code is now an ASL licensed 3rd party jar dependency: http://spatial4j.com "Spatial4J" Basically half of LSP is there now going by this new name. The other half is here as the new lucene spatial module. I agree that the branch looks ready to be merged into trunk.
          Hide
          Robert Muir added a comment -

          Can we rethink this structure? In my opinion there is a little bit of dll-hell going on on.

          From Lucene's perspective as a library, 3rd party dependencies are extremely expensive. I realize
          this doesnt matter so much for solr, since its an app, but I think we should minimize this.

          We all agreed modules should be treated like lucene core (which has no dependencies), and sure,
          some modules do have dependencies but they should be minimal and necessary.

          Just looking at the lib/ directory in the branch I see:

          • commons-lang.jar: This is unnecessary and only used for EqualsBuilder/HashCodeBuilder, please remove!
          • slf4j.jar: Lucene doesnt do logging: there have been numerous discussions about this, such logging should be at a higher level app like solr. But, this doesnt seem to be 'actually' used anyway... please remove!
          • spatial4j.jar: I think this approach should be re-thought. I dont understand the advantage of creating the extra level of indirection to a github project here. I also have no clue what dependencies this jar itself has... furthermore i dont even know how to get the source code for this binary jar, there is only a github link with no branches or tags to indicate "0.1". I think all of this is a big no-go.
          Show
          Robert Muir added a comment - Can we rethink this structure? In my opinion there is a little bit of dll-hell going on on. From Lucene's perspective as a library, 3rd party dependencies are extremely expensive . I realize this doesnt matter so much for solr, since its an app, but I think we should minimize this. We all agreed modules should be treated like lucene core (which has no dependencies), and sure, some modules do have dependencies but they should be minimal and necessary. Just looking at the lib/ directory in the branch I see: commons-lang.jar: This is unnecessary and only used for EqualsBuilder/HashCodeBuilder, please remove! slf4j.jar: Lucene doesnt do logging: there have been numerous discussions about this, such logging should be at a higher level app like solr. But, this doesnt seem to be 'actually' used anyway... please remove! spatial4j.jar: I think this approach should be re-thought. I dont understand the advantage of creating the extra level of indirection to a github project here. I also have no clue what dependencies this jar itself has... furthermore i dont even know how to get the source code for this binary jar, there is only a github link with no branches or tags to indicate "0.1". I think all of this is a big no-go.
          Hide
          Robert Muir added a comment -

          Also again about my spatial4j.jar: besides the dll-hell perspective, there is also the community perspective.

          I dont think we should create github projects that only a few people can commit to and then link binary jars from them into lucene's source tree.

          Show
          Robert Muir added a comment - Also again about my spatial4j.jar: besides the dll-hell perspective, there is also the community perspective. I dont think we should create github projects that only a few people can commit to and then link binary jars from them into lucene's source tree.
          Hide
          Chris Male added a comment -

          I dont think we should create github projects that only a few people can commit to and then link binary jars from them into lucene's source tree.

          +1

          Show
          Chris Male added a comment - I dont think we should create github projects that only a few people can commit to and then link binary jars from them into lucene's source tree. +1
          Hide
          Uwe Schindler added a comment - - edited

          commons-lang.jar: This is unnecessary and only used for EqualsBuilder/HashCodeBuilder, please remove!

          die, die, die To come back to an earlier comment: I don't like projects that add a 15 MB JAR file and use one single method/class out of it (this is of course an extreme example). In my opinion, common-lang.jar should only be used, if you heavily depend on it. And if you really want to use it, use version 3 (o.a.commons.lang3 package), not the Java 1.1 versions. Most of the stuff in commons-lang is useless since java 5 and the old pre-commons-lang3 is not typesafe at all (and has millions of bugs regarding unicode).

          Show
          Uwe Schindler added a comment - - edited commons-lang.jar: This is unnecessary and only used for EqualsBuilder/HashCodeBuilder, please remove! die, die, die To come back to an earlier comment: I don't like projects that add a 15 MB JAR file and use one single method/class out of it (this is of course an extreme example). In my opinion, common-lang.jar should only be used, if you heavily depend on it. And if you really want to use it, use version 3 (o.a.commons.lang3 package), not the Java 1.1 versions. Most of the stuff in commons-lang is useless since java 5 and the old pre-commons-lang3 is not typesafe at all (and has millions of bugs regarding unicode).
          Hide
          Simon Willnauer added a comment -

          I agree with robert, can we try to survive without dependencies? What is the reason to have this stuff on github, its your projects anyway right? also the spatial4j notice file is a copy of the commons lang.

          simon

          Show
          Simon Willnauer added a comment - I agree with robert, can we try to survive without dependencies? What is the reason to have this stuff on github, its your projects anyway right? also the spatial4j notice file is a copy of the commons lang. simon
          Hide
          Uwe Schindler added a comment -

          What's the reason to have spatial4j outside of Lucene?

          Show
          Uwe Schindler added a comment - What's the reason to have spatial4j outside of Lucene?
          Hide
          Jan Høydahl added a comment -

          First, Ryan and David, I think you've done a great job with all this code and a million thanks for donating.

          I think I also see the rationale behind splitting the more general spatial4j core into a separate project, hoping that it will attract far more users than only Lucene. While that may happen one day, perhaps we should take one step at a time, letting spatial4j start its life as part of Lucene-java (as a separate module or contrib?), and after a year or so, when it is more mature, cast a vote for whether it shuold become a Lucene sub-project, join Incubator as a new project, join as a library within Apache commons project (commons-spatial), move to GitHub with all Lucene committers invited as committers) or whatever.

          commons-spatial sounds attractive to me

          Show
          Jan Høydahl added a comment - First, Ryan and David, I think you've done a great job with all this code and a million thanks for donating. I think I also see the rationale behind splitting the more general spatial4j core into a separate project, hoping that it will attract far more users than only Lucene. While that may happen one day, perhaps we should take one step at a time, letting spatial4j start its life as part of Lucene-java (as a separate module or contrib?), and after a year or so, when it is more mature, cast a vote for whether it shuold become a Lucene sub-project, join Incubator as a new project, join as a library within Apache commons project (commons-spatial), move to GitHub with all Lucene committers invited as committers) or whatever. commons-spatial sounds attractive to me
          Hide
          Robert Muir added a comment -

          when it is more mature, cast a vote for whether it shuold become a Lucene sub-project, join Incubator as a new project, join as a library within Apache commons project (commons-spatial), move to GitHub with all Lucene committers invited as committers) or whatever.

          In my opinion http://incubator.apache.org/sis/ seems like the correct home.

          Of course the code could be donated there in parallel, if it starts picking up steam and graduates from the incubator,
          then it would be the right thing to depend on it I think?

          Show
          Robert Muir added a comment - when it is more mature, cast a vote for whether it shuold become a Lucene sub-project, join Incubator as a new project, join as a library within Apache commons project (commons-spatial), move to GitHub with all Lucene committers invited as committers) or whatever. In my opinion http://incubator.apache.org/sis/ seems like the correct home. Of course the code could be donated there in parallel, if it starts picking up steam and graduates from the incubator, then it would be the right thing to depend on it I think?
          Hide
          David Smiley added a comment -

          Commons-lang is used by both Spatial4J and the new spatial-module. This dependency can easily be severed and will happen shortly.

          SLF4j is used by both Spatial4J and the new spatial-module. I really like SLF4J but all this resistance to remove dependencies leads me to compromise, and I'll find a way to removing it or have it as an optional dependency.

          Uwe and others, the rationale for a core spatial library off of Lucene is my last (long) comment: https://issues.apache.org/jira/browse/LUCENE-3795?focusedCommentId=13216522&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13216522
          For what its worth, Ryan and I absolutely love the plan for all of the points in it. I wish someone had expressed their dissenting opinion on it at that time – From Ryan and I's perspective there basically isn't anything not to like. Can anything be done to warm people up to this?

          Rob; you're absolutely right that there needs to be a release tagged in the Spatial4J repo. Ryan has already taken steps to get this library in published Maven repos which is the most meaningful step that could be taken to officially release it. Again, we should certainly tag it because it is both best practice and easy.

          The ASF is a bit heavy on process and less permissive on interactions with LGPL dependencies (even optional ones?) and so I don't think ASF/incubator is a good place for Spatial4j as an independent project. As frustrating as I find it, the making of spatial-4j could be reverted, returning back to the 2-module setup that some people here seemed to express resistance to.

          Show
          David Smiley added a comment - Commons-lang is used by both Spatial4J and the new spatial-module. This dependency can easily be severed and will happen shortly. SLF4j is used by both Spatial4J and the new spatial-module. I really like SLF4J but all this resistance to remove dependencies leads me to compromise, and I'll find a way to removing it or have it as an optional dependency. Uwe and others, the rationale for a core spatial library off of Lucene is my last (long) comment: https://issues.apache.org/jira/browse/LUCENE-3795?focusedCommentId=13216522&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13216522 For what its worth, Ryan and I absolutely love the plan for all of the points in it. I wish someone had expressed their dissenting opinion on it at that time – From Ryan and I's perspective there basically isn't anything not to like. Can anything be done to warm people up to this? Rob; you're absolutely right that there needs to be a release tagged in the Spatial4J repo. Ryan has already taken steps to get this library in published Maven repos which is the most meaningful step that could be taken to officially release it. Again, we should certainly tag it because it is both best practice and easy. The ASF is a bit heavy on process and less permissive on interactions with LGPL dependencies (even optional ones?) and so I don't think ASF/incubator is a good place for Spatial4j as an independent project. As frustrating as I find it, the making of spatial-4j could be reverted, returning back to the 2-module setup that some people here seemed to express resistance to.
          Hide
          Robert Muir added a comment -

          The ASF is a bit heavy on process and less permissive on interactions with LGPL dependencies (even optional ones?)

          Which is a great thing in my opinion if we are going to depend on an external library for spatial support.

          Show
          Robert Muir added a comment - The ASF is a bit heavy on process and less permissive on interactions with LGPL dependencies (even optional ones?) Which is a great thing in my opinion if we are going to depend on an external library for spatial support.
          Hide
          Paul Ramirez added a comment -

          Why not help Apache SIS grow and contribute spatial base as a module there. Then make the Lucene/Solr parts a contrib part in SIS or here. I'm sure they would love the support and would hope to get the developers of spatial base as key members of their community. This would put spatial stuff in one place and help an Apache spatial community grow. This seems more like the Apache way. That said, I am jumping in late to this discussion but I think spatial stuff really deserves its own community at Apache.

          Show
          Paul Ramirez added a comment - Why not help Apache SIS grow and contribute spatial base as a module there. Then make the Lucene/Solr parts a contrib part in SIS or here. I'm sure they would love the support and would hope to get the developers of spatial base as key members of their community. This would put spatial stuff in one place and help an Apache spatial community grow. This seems more like the Apache way. That said, I am jumping in late to this discussion but I think spatial stuff really deserves its own community at Apache.
          Hide
          Yonik Seeley added a comment -

          While it's fair game to ask if something barely used like commons-lang can be removed, it doesn't seem like the other things should be blockers to getting this committed.

          As long as spatial4j.jar is properly licensed, where that project should "live" is a different issue and has nothing to do with this. If we didn't know any of the authors of this jar, we wouldn't be having the discussion at all.

          We all agreed modules should be treated like lucene core

          I don't recall that - modules are shared code by both Lucene and Solr. There's a desire to not pull in all of solr in a module of course, but dependencies on other jars or other modules should be fine.

          Show
          Yonik Seeley added a comment - While it's fair game to ask if something barely used like commons-lang can be removed, it doesn't seem like the other things should be blockers to getting this committed. As long as spatial4j.jar is properly licensed, where that project should "live" is a different issue and has nothing to do with this. If we didn't know any of the authors of this jar, we wouldn't be having the discussion at all. We all agreed modules should be treated like lucene core I don't recall that - modules are shared code by both Lucene and Solr. There's a desire to not pull in all of solr in a module of course, but dependencies on other jars or other modules should be fine.
          Hide
          Mark Miller added a comment -

          We all agreed modules should be treated like lucene core

          Just to chime in since this caught my eye - I don't necessarily agree with this either - I think it depends on the module - thats what I thought we had agreed - like some modules could be more like lucene core and some modules might be more like benchmark - some modules might have really strong back compat, and others might declare something more experimental. And why not a module that might depend on a couple other modules?

          Don't take that as an opinion on this spatial issue though - I'm not up to speed on this discussion - just wanted to weigh in on a comment that caught my eye in email.

          Show
          Mark Miller added a comment - We all agreed modules should be treated like lucene core Just to chime in since this caught my eye - I don't necessarily agree with this either - I think it depends on the module - thats what I thought we had agreed - like some modules could be more like lucene core and some modules might be more like benchmark - some modules might have really strong back compat, and others might declare something more experimental. And why not a module that might depend on a couple other modules? Don't take that as an opinion on this spatial issue though - I'm not up to speed on this discussion - just wanted to weigh in on a comment that caught my eye in email.
          Hide
          Ryan McKinley added a comment -

          What's the reason to have spatial4j outside of Lucene?

          1. Making JTS a 1st class test object (also why SIS is not an option)
          2. The spatial4j.jar is useful on its own – it has a chance to build its own community.
          3. Lucene is not a great dev community for things that are not primarily lucene focused.

          I understand my primary concern (JTS) is a non issue for many people here – The trade off to have compile/test dependencies on JTS isn't an option at ASF.

          I like this option because it gives lucene a solid ASL solution to support most things. If people want to add JTS to their runtime, they then get strong polygon support.

          The alternative packaging structure gets pretty crazy:

          In ASF Lucene Reps:

          • modules/spatial-base (no lucene dependencies)
          • modules/spatial-strategies (uses base)
          • solr/spatial...

          Elsewhere

          • spatial-base-jts (base with JTS)
          • spatial-strategies-with-jts
          • spatial-solr-with-jts

          If I want to make sure the JtsSpatialContext passes all the same tests that the SimpleSpatialContext passes, the structure gets even crazier because we have to package the test projects too!

          --------------

          We all agreed modules should be treated like lucene core

          hymmm – my understanding is that modules have flexibility to have dependencies that are appropriate.

          Show
          Ryan McKinley added a comment - What's the reason to have spatial4j outside of Lucene? 1. Making JTS a 1st class test object (also why SIS is not an option) 2. The spatial4j.jar is useful on its own – it has a chance to build its own community. 3. Lucene is not a great dev community for things that are not primarily lucene focused. I understand my primary concern (JTS) is a non issue for many people here – The trade off to have compile/test dependencies on JTS isn't an option at ASF. I like this option because it gives lucene a solid ASL solution to support most things. If people want to add JTS to their runtime, they then get strong polygon support. The alternative packaging structure gets pretty crazy: In ASF Lucene Reps: modules/spatial-base (no lucene dependencies) modules/spatial-strategies (uses base) solr/spatial... Elsewhere spatial-base-jts (base with JTS) spatial-strategies-with-jts spatial-solr-with-jts If I want to make sure the JtsSpatialContext passes all the same tests that the SimpleSpatialContext passes, the structure gets even crazier because we have to package the test projects too! -------------- We all agreed modules should be treated like lucene core hymmm – my understanding is that modules have flexibility to have dependencies that are appropriate.
          Hide
          Uwe Schindler added a comment -

          Commons-lang is used by both Spatial4J and the new spatial-module. This dependency can easily be severed and will happen shortly.

          Fine!

          SLF4j is used by both Spatial4J and the new spatial-module. I really like SLF4J but all this resistance to remove dependencies leads me to compromise, and I'll find a way to removing it or have it as an optional dependency.

          In my opinion, non-end-user components should not log, which affects libraries. E.g. there is nothing in the JDK to enable logging of the JDK itsself, although there are surely parts that could log something. Lucene is the same, it does not need to log anything, the client code should log things like "now executing term query..." and so on. IndexWriter is a little bit special, it has now a simple "log-like" interface for debugging (consisting of abstract InfoStream class). This class can be implemented by a logging framework, but would slowdown indexing immense, as logging frameworks tend to use volatiles on every log request (even when not logging).

          So I strongly recommend to remove logging. For debugging we often comment out System.out.println inside Lucene.

          Uwe and others, the rationale for a core spatial library off of Lucene is my last (long) comment

          OK, OK. I still don't understand the whole rationale to move it outside Lucene or to a separate module. Everybody can use the classes by adding the JAR file, too. The "useless" lucene classes don't hurt. Still I would strongly recommend to use parts of lucene.util package whereever possible, especially when performance and easy Lucene integration is needed. So dont create Strings all the time, use BytesRef and index/search using binary terms like NumericField does in Lucene trunk. If this module outside Lucene uses Strings all the time, but e.g. when indexing searching all those strings are again converted to UTF-8 BytesRefs, thats a laaaaaaaaaaaaaaarge overhead. So I prefer to sometimes duplicate code and add performant impls of e.g. term encoders for indexing/search. Every method in Lucene that is used in tight loops (like scorers or TokenStreams) should never ever use Strings (which are final and unmodifiable).

          Show
          Uwe Schindler added a comment - Commons-lang is used by both Spatial4J and the new spatial-module. This dependency can easily be severed and will happen shortly. Fine! SLF4j is used by both Spatial4J and the new spatial-module. I really like SLF4J but all this resistance to remove dependencies leads me to compromise, and I'll find a way to removing it or have it as an optional dependency. In my opinion, non-end-user components should not log, which affects libraries. E.g. there is nothing in the JDK to enable logging of the JDK itsself, although there are surely parts that could log something. Lucene is the same, it does not need to log anything, the client code should log things like "now executing term query..." and so on. IndexWriter is a little bit special, it has now a simple "log-like" interface for debugging (consisting of abstract InfoStream class). This class can be implemented by a logging framework, but would slowdown indexing immense, as logging frameworks tend to use volatiles on every log request (even when not logging). So I strongly recommend to remove logging. For debugging we often comment out System.out.println inside Lucene. Uwe and others, the rationale for a core spatial library off of Lucene is my last (long) comment OK, OK. I still don't understand the whole rationale to move it outside Lucene or to a separate module. Everybody can use the classes by adding the JAR file, too. The "useless" lucene classes don't hurt. Still I would strongly recommend to use parts of lucene.util package whereever possible, especially when performance and easy Lucene integration is needed. So dont create Strings all the time, use BytesRef and index/search using binary terms like NumericField does in Lucene trunk. If this module outside Lucene uses Strings all the time, but e.g. when indexing searching all those strings are again converted to UTF-8 BytesRefs, thats a laaaaaaaaaaaaaaarge overhead. So I prefer to sometimes duplicate code and add performant impls of e.g. term encoders for indexing/search. Every method in Lucene that is used in tight loops (like scorers or TokenStreams) should never ever use Strings (which are final and unmodifiable).
          Hide
          David Smiley added a comment -

          Everything depends on something. Lucene depends on Java, and it has no control over Java except to complain when there are bugs. This module isn't the first module to have a dependency and frankly I don't understand the aversion to them – it's a natural thing. I agree you can have too many.

          I think Lucene-Spatial's dependency on Spatial4j represents the best type of a dependency that Lucene/Solr could have:

          1. ASL licensed
          2. Has code & tests there that aren't related to Lucene; don't clutter or diffuse scope of Lucene's codebase.
          3. Strong relationship to Lucene/Solr. Put another way, if Spatial4j were a product, it's only customer right now is Lucene/Solr. Consequently:
            1. When Lucene/Solr decides to release a version, Spatial4J committers will do the same. No SNAPSHOT dependency from a Lucene release.
            2. When a bug or feature request comes up via Lucene that requires changes in Spatial4J, you (a Lucene committer) can coordinate these changes with great efficiency given that you know Spatial4J committers.
            3. You can become a committer on Spatial4j with far less time than it took me to become a committer here, I swear especially the spatial-minded folks: Chris (if we had your GitHub username, you'd be grandfathered in), Uwe, Grant, Yonik?

          And because it's a dependency and not 1st-party code, it has a greater opportunity to receive improvements from outside parties since it's a smaller project with a more focused committer pool. This stuff has nothing to do with Lucene, remember.

          Show
          David Smiley added a comment - Everything depends on something. Lucene depends on Java, and it has no control over Java except to complain when there are bugs. This module isn't the first module to have a dependency and frankly I don't understand the aversion to them – it's a natural thing. I agree you can have too many. I think Lucene-Spatial's dependency on Spatial4j represents the best type of a dependency that Lucene/Solr could have: ASL licensed Has code & tests there that aren't related to Lucene; don't clutter or diffuse scope of Lucene's codebase. Strong relationship to Lucene/Solr. Put another way, if Spatial4j were a product, it's only customer right now is Lucene/Solr. Consequently: When Lucene/Solr decides to release a version, Spatial4J committers will do the same. No SNAPSHOT dependency from a Lucene release. When a bug or feature request comes up via Lucene that requires changes in Spatial4J, you (a Lucene committer) can coordinate these changes with great efficiency given that you know Spatial4J committers. You can become a committer on Spatial4j with far less time than it took me to become a committer here, I swear – especially the spatial-minded folks: Chris (if we had your GitHub username, you'd be grandfathered in), Uwe, Grant, Yonik? And because it's a dependency and not 1st-party code, it has a greater opportunity to receive improvements from outside parties since it's a smaller project with a more focused committer pool. This stuff has nothing to do with Lucene, remember.
          Hide
          David Smiley added a comment -

          Uwe,
          Regarding byte / character performance stuff, I understand. Last night in fact, I successfully argued with Ryan that the SpatialPrefixTree (a trie) belongs with the Lucene spatial module because it is tightly coupled to the algorithms there. One of the arguments was that it could/should probably use ByteRef since it works with raw indexed data. Now that this moved over, I don't think there's code in Spatial4j that is or should be byte oriented. There's some geospatial (WKT-like) string parsing code, and code that generates such strings from shapes, and they are String oriented and that makes sense.
          As a general statement about performance, you should know that performance is an important goal of Spatial4j. So if for example the API needs to be a bit uglier to make performance compromises, this spatial library more than others is willing to bend. I think benchmarks need to prove this out first on a case-by-case basis though. Other libraries, like one where I work seems to take another extreme in which Latitude and Longitude are each separate classes!

          Show
          David Smiley added a comment - Uwe, Regarding byte / character performance stuff, I understand. Last night in fact, I successfully argued with Ryan that the SpatialPrefixTree (a trie) belongs with the Lucene spatial module because it is tightly coupled to the algorithms there. One of the arguments was that it could/should probably use ByteRef since it works with raw indexed data. Now that this moved over, I don't think there's code in Spatial4j that is or should be byte oriented. There's some geospatial (WKT-like) string parsing code, and code that generates such strings from shapes, and they are String oriented and that makes sense. As a general statement about performance, you should know that performance is an important goal of Spatial4j. So if for example the API needs to be a bit uglier to make performance compromises, this spatial library more than others is willing to bend. I think benchmarks need to prove this out first on a case-by-case basis though. Other libraries, like one where I work seems to take another extreme in which Latitude and Longitude are each separate classes!
          Hide
          Michael McCandless added a comment -

          Why not help Apache SIS grow and contribute spatial base as a module there.

          +1

          Show
          Michael McCandless added a comment - Why not help Apache SIS grow and contribute spatial base as a module there. +1
          Hide
          Adam Estrada added a comment -

          Yes...why not grow SIS seeing as its already in Apache Incubation? If another project is launched with the same intention as SIS, wouldn't that mean there is the potential for duplicate technologies sitting on an Apache server? There are a ton of different components to building out a proper geospatial core technology. GDAL/OGR, OpenLayers and the database storage techs therein represent some of the high points to operating a full GIS. See Wikipedia article (http://en.wikipedia.org/wiki/Geographic_information_system) and note that in order for there to be a full-fledged GIS, there has to be several mutually exclusive components that work in harmony with each other. I see Spatial4J being a great new way to implement geometric functions and storage within a Lucene index. That leaves the format/projection support, visualization and management components. Wouldn't it be nice to roll all of these fundamental concepts in to a single project like SIS? I am looking at the big picture rather than specific or individual components.

          w/r,
          Adam

          Show
          Adam Estrada added a comment - Yes...why not grow SIS seeing as its already in Apache Incubation? If another project is launched with the same intention as SIS, wouldn't that mean there is the potential for duplicate technologies sitting on an Apache server? There are a ton of different components to building out a proper geospatial core technology. GDAL/OGR, OpenLayers and the database storage techs therein represent some of the high points to operating a full GIS. See Wikipedia article ( http://en.wikipedia.org/wiki/Geographic_information_system ) and note that in order for there to be a full-fledged GIS, there has to be several mutually exclusive components that work in harmony with each other. I see Spatial4J being a great new way to implement geometric functions and storage within a Lucene index. That leaves the format/projection support, visualization and management components. Wouldn't it be nice to roll all of these fundamental concepts in to a single project like SIS? I am looking at the big picture rather than specific or individual components. w/r, Adam
          Hide
          Ryan McKinley added a comment -

          Wouldn't it be nice to roll all of these fundamental concepts in to a single project like SIS?

          I agree – but the elephant in the room is the LGPL library JTS.

          From previous discussions, I believe this was a non-starter for their compile/test environment, but I have emailed the sis-dev@ list to see if this is still their feeling.

          With spatial4j, we want a solid ASL solution, but also support complex polygons if people choose to use JTS in their runtime environment.

          Show
          Ryan McKinley added a comment - Wouldn't it be nice to roll all of these fundamental concepts in to a single project like SIS? I agree – but the elephant in the room is the LGPL library JTS. From previous discussions, I believe this was a non-starter for their compile/test environment, but I have emailed the sis-dev@ list to see if this is still their feeling. With spatial4j, we want a solid ASL solution, but also support complex polygons if people choose to use JTS in their runtime environment.
          Hide
          Ryan McKinley added a comment -

          I have confirmed with SIS that a compile/test dependency on JTS in not possible. (One of their main goals is to make an ASL version of JTS... A great goal, but they are not yet to 1st base)

          So, where does that leave us?

          Is the spatial4j.jar a blocker for anyone? I understand it is not everyone's preferred option – but no option makes everyone happy.

          Show
          Ryan McKinley added a comment - I have confirmed with SIS that a compile/test dependency on JTS in not possible. (One of their main goals is to make an ASL version of JTS... A great goal, but they are not yet to 1st base) So, where does that leave us? Is the spatial4j.jar a blocker for anyone? I understand it is not everyone's preferred option – but no option makes everyone happy.
          Hide
          Chris A. Mattmann added a comment -

          I'd encourage folks to read:

          http://markmail.org/message/75yrfmumxlmdgxzz

          and all of the surrounding discussion there.

          Quoting Greg Stein:

          I simply think that it is a mistake for a PMC to create any sort of dependency upon code that is more restrictive than the Apache License. In this case, it means somebody must grab LGPL code in order to build our provided tarball. I would strongly advise against such a build dependency, whether the runtime requires it or not.

          I strongly agree with his interpretation.

          Show
          Chris A. Mattmann added a comment - I'd encourage folks to read: http://markmail.org/message/75yrfmumxlmdgxzz and all of the surrounding discussion there. Quoting Greg Stein: I simply think that it is a mistake for a PMC to create any sort of dependency upon code that is more restrictive than the Apache License. In this case, it means somebody must grab LGPL code in order to build our provided tarball. I would strongly advise against such a build dependency, whether the runtime requires it or not. I strongly agree with his interpretation.
          Hide
          Ryan McKinley added a comment -

          PMC to create any sort of dependency upon code that is more restrictive than the Apache License.

          Note, the spatial4j.jar file is ASL

          No user ever needs to touch LGPL code or artifacts

          Show
          Ryan McKinley added a comment - PMC to create any sort of dependency upon code that is more restrictive than the Apache License. Note, the spatial4j.jar file is ASL No user ever needs to touch LGPL code or artifacts
          Hide
          Bill Bell added a comment -

          Ryan...

          Why not just include spatial4j.jar into Lucene/Solr and later separate it. It looks like SIS is not even off the ground yet.

          In my opinion an API is not really an API until 3 or more projects use it anyways.

          Why not make JTS pluggable as a separate module into Lucene? Then people can download JTS and add it into the Spatial solution by modifying a config file like solrconfig.xml ?

          I would love to get this committed and done done.

          Bill

          Show
          Bill Bell added a comment - Ryan... Why not just include spatial4j.jar into Lucene/Solr and later separate it. It looks like SIS is not even off the ground yet. In my opinion an API is not really an API until 3 or more projects use it anyways. Why not make JTS pluggable as a separate module into Lucene? Then people can download JTS and add it into the Spatial solution by modifying a config file like solrconfig.xml ? I would love to get this committed and done done. Bill
          Hide
          Chris A. Mattmann added a comment -

          It looks like SIS is not even off the ground yet.

          I guess if you call making an Apache release, with a working Java Quad Tree implementation, point-radius and bounding box against that QuadTree, the ability to load GeoRSS, and a demo webapp that plugs into Google Maps, "not even off the ground yet", then yeah, I guess it's not.

          Show
          Chris A. Mattmann added a comment - It looks like SIS is not even off the ground yet. I guess if you call making an Apache release, with a working Java Quad Tree implementation, point-radius and bounding box against that QuadTree, the ability to load GeoRSS, and a demo webapp that plugs into Google Maps, "not even off the ground yet", then yeah, I guess it's not.
          Hide
          Ryan McKinley added a comment -

          I think this is ready for /trunk

          Unless there are objections, I will commit tomorrow – and we can iterate from there

          Show
          Ryan McKinley added a comment - I think this is ready for /trunk Unless there are objections, I will commit tomorrow – and we can iterate from there
          Hide
          Robert Muir added a comment -

          I'm still hoping the logging issue will get resolved.

          Can we please remove this dependency? Again I don't think we should be logging at this level.
          For example its dangerous and bogus to suppress exceptions and log instead: this is an API component.

          Higher level code (e.g. Solr) with more context can implement logging appropriately, but we should just throw
          Exceptions for Lucene API users.

          For example, TwoDoublesStrategy.makeQuery has this code:

          } catch(Exception ex) {
            log.warn("error making score", ex);
          }
          
          Show
          Robert Muir added a comment - I'm still hoping the logging issue will get resolved. Can we please remove this dependency? Again I don't think we should be logging at this level. For example its dangerous and bogus to suppress exceptions and log instead: this is an API component. Higher level code (e.g. Solr) with more context can implement logging appropriately, but we should just throw Exceptions for Lucene API users. For example, TwoDoublesStrategy.makeQuery has this code: } catch (Exception ex) { log.warn( "error making score" , ex); }
          Hide
          Ryan McKinley added a comment -

          I think we can remove logging... i'll take a look

          Show
          Ryan McKinley added a comment - I think we can remove logging... i'll take a look
          Hide
          David Smiley added a comment -

          I committed the removal of SLF4J just now.

          That catch block that Robert mentions has a bad code smell and so I chose to not catch the exception (and thus not log anything there either). Tests still pass. Perhaps the author of this code (Ryan?) might add a test in which the exception can happen (what subclass?) and then we can consider the right thing to do. TwoDoublesStrategy hasn't seen any love in a long time, compared to RecursivePrefixStrategy which gets all the attention.

          Show
          David Smiley added a comment - I committed the removal of SLF4J just now. That catch block that Robert mentions has a bad code smell and so I chose to not catch the exception (and thus not log anything there either). Tests still pass. Perhaps the author of this code (Ryan?) might add a test in which the exception can happen (what subclass?) and then we can consider the right thing to do. TwoDoublesStrategy hasn't seen any love in a long time, compared to RecursivePrefixStrategy which gets all the attention.
          Hide
          Uwe Schindler added a comment -

          Thank's David! I wanted to respond with the same comment like Robert! Logging should not be done in library code and exceptions should not be ignored. The warn was better than the common eclipse-autogenerated try-catch with e.printStackTrace(), which is the worst anti-pattern I have ever seen, but we still should not do this in library code.

          Show
          Uwe Schindler added a comment - Thank's David! I wanted to respond with the same comment like Robert! Logging should not be done in library code and exceptions should not be ignored. The warn was better than the common eclipse-autogenerated try-catch with e.printStackTrace(), which is the worst anti-pattern I have ever seen, but we still should not do this in library code.
          Hide
          Ryan McKinley added a comment -

          ok – with the logging issue solved, i think we can move things forward

          Show
          Ryan McKinley added a comment - ok – with the logging issue solved, i think we can move things forward
          Hide
          Yonik Seeley added a comment -

          Guys, was there a specific reason why the degrees-radians conversion optimizations were removed?

          Example:

          -        return vals.doubleVal(doc) * DistanceUtils.DEGREES_TO_RADIANS;
          +        return Math.toRadians(vals.doubleVal(doc));
          

          It won't matter for query setup, but will matter for per-document calculations.

          Show
          Yonik Seeley added a comment - Guys, was there a specific reason why the degrees-radians conversion optimizations were removed? Example: - return vals.doubleVal(doc) * DistanceUtils.DEGREES_TO_RADIANS; + return Math .toRadians(vals.doubleVal(doc)); It won't matter for query setup, but will matter for per-document calculations.
          Hide
          David Smiley added a comment -

          I made those conversions because I didn't see the point. Surely the JVM can inline the Math.* methods, especially this one.

          Show
          David Smiley added a comment - I made those conversions because I didn't see the point. Surely the JVM can inline the Math.* methods, especially this one.
          Hide
          Yonik Seeley added a comment -

          Surely the JVM can inline the Math.* methods, especially this one.

          Inlining yes, but not optimizing (compilers are very restricted in how they can optimize floating point calculations).
          Unfortunately Math.toRadians uses double precision division, which is much more expensive than multiplication.
          I just did a quick test, and Math.toRadians was more than 3 times slower.

          I've got most of the changes locally so I'll finish it up...

          Show
          Yonik Seeley added a comment - Surely the JVM can inline the Math.* methods, especially this one. Inlining yes, but not optimizing (compilers are very restricted in how they can optimize floating point calculations). Unfortunately Math.toRadians uses double precision division, which is much more expensive than multiplication. I just did a quick test, and Math.toRadians was more than 3 times slower. I've got most of the changes locally so I'll finish it up...
          Hide
          Chris Male added a comment -

          I just did a quick test, and Math.toRadians was more than 3 times slower.

          Big +1 to this. There are huge benefits to be found in optimizing the actual arithmetic. Math.* methods are very conservative.

          Show
          Chris Male added a comment - I just did a quick test, and Math.toRadians was more than 3 times slower. Big +1 to this. There are huge benefits to be found in optimizing the actual arithmetic. Math.* methods are very conservative.
          Hide
          David Smiley added a comment -

          I'd be very surprised to hear if this is true. What was your benchmarking methodology Yonik? I recently read an excellent presentation by Cliff Click (a JVM implementer and is as expert as they come) on Java benchmarking: http://www.azulsystems.com/presentations/art-of-java-benchmarking

          Show
          David Smiley added a comment - I'd be very surprised to hear if this is true. What was your benchmarking methodology Yonik? I recently read an excellent presentation by Cliff Click (a JVM implementer and is as expert as they come) on Java benchmarking: http://www.azulsystems.com/presentations/art-of-java-benchmarking
          Hide
          Ryan McKinley added a comment -

          I will mark this resolved and we can start new issues for ongoing problems.

          The next big step is to integrate with solr.

          Show
          Ryan McKinley added a comment - I will mark this resolved and we can start new issues for ongoing problems. The next big step is to integrate with solr.
          Hide
          Ryan McKinley added a comment -

          did not mean to 'resolve' the Math.toRadians issue though – I think we should change that back to multiplication... Math.* seems to be pretty clunky

          Show
          Ryan McKinley added a comment - did not mean to 'resolve' the Math.toRadians issue though – I think we should change that back to multiplication... Math.* seems to be pretty clunky
          Hide
          Yonik Seeley added a comment -

          I'd be very surprised to hear if this is true.

          If Math.toRadians had been written as x*(PI/180.0) then the compiler would have done constant folding and it would simply be multiplication by a constant. But it's unfortunately written as x/180.0*PI (for no good reason in this case), and the compiler/JVM is not allowed to do the simple transformation by itself. That's why we do it.

          Sometimes knowing how optimizers work and the restrictions on them allow one to know what will be faster or slower without benchmarking. I did benchmark it after the fact (after you questioned it), and it was indeed the case that Math.toRadians was much slower than a simple multiply.

          Show
          Yonik Seeley added a comment - I'd be very surprised to hear if this is true. If Math.toRadians had been written as x*(PI/180.0) then the compiler would have done constant folding and it would simply be multiplication by a constant. But it's unfortunately written as x/180.0*PI (for no good reason in this case), and the compiler/JVM is not allowed to do the simple transformation by itself. That's why we do it. Sometimes knowing how optimizers work and the restrictions on them allow one to know what will be faster or slower without benchmarking. I did benchmark it after the fact (after you questioned it), and it was indeed the case that Math.toRadians was much slower than a simple multiply.
          Hide
          Ryan McKinley added a comment -

          I did benchmark it after the fact (after you questioned it), and it was indeed the case that Math.toRadians was much slower than a simple multiply.

          Can I see the benchmark? The trivial things I am trying seems to end up equivalent

          Show
          Ryan McKinley added a comment - I did benchmark it after the fact (after you questioned it), and it was indeed the case that Math.toRadians was much slower than a simple multiply. Can I see the benchmark? The trivial things I am trying seems to end up equivalent
          Hide
          Yonik Seeley added a comment -

          Can I see the benchmark? The trivial things I am trying seems to end up equivalent

          One way that can happen is if you don't use the values produced - hotspot can eliminate the method calls altogether.

          public class X {
            public static double foo(double val) {
              // return Math.toRadians(val);
              return val * (Math.PI/180.0);
            }
            
            public static void main(String[] args) {
              double x = 1.12345;
              for (int i=0; i<100000000; i++) {
                x += foo(x) - foo(x+1); 
              }    
              System.out.println(x);
            }
          }
          
          Show
          Yonik Seeley added a comment - Can I see the benchmark? The trivial things I am trying seems to end up equivalent One way that can happen is if you don't use the values produced - hotspot can eliminate the method calls altogether. public class X { public static double foo( double val) { // return Math .toRadians(val); return val * ( Math .PI/180.0); } public static void main( String [] args) { double x = 1.12345; for ( int i=0; i<100000000; i++) { x += foo(x) - foo(x+1); } System .out.println(x); } }
          Hide
          Ryan McKinley added a comment -

          yup – I get the same results as you. I've updated things to use this optimization at spatial4j

          Show
          Ryan McKinley added a comment - yup – I get the same results as you. I've updated things to use this optimization at spatial4j
          Hide
          Commit Tag Bot added a comment -

          [branch_4x commit] Ryan McKinley
          http://svn.apache.org/viewvc?view=revision&revision=1419688

          LUCENE-3795: makeQuery should not require ConstantScoreQuery (merge from trunk)

          Show
          Commit Tag Bot added a comment - [branch_4x commit] Ryan McKinley http://svn.apache.org/viewvc?view=revision&revision=1419688 LUCENE-3795 : makeQuery should not require ConstantScoreQuery (merge from trunk)
          Hide
          Commit Tag Bot added a comment -

          [trunk commit] Ryan McKinley
          http://svn.apache.org/viewvc?view=revision&revision=1419687

          LUCENE-3795: makeQuery should not require ConstantScoreQuery

          Show
          Commit Tag Bot added a comment - [trunk commit] Ryan McKinley http://svn.apache.org/viewvc?view=revision&revision=1419687 LUCENE-3795 : makeQuery should not require ConstantScoreQuery

            People

            • Assignee:
              David Smiley
              Reporter:
              David Smiley
            • Votes:
              8 Vote for this issue
              Watchers:
              10 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development