Lucene - Core
  1. Lucene - Core
  2. LUCENE-2986

divorce defaultsimilarityprovider from defaultsimilarity

    Details

    • Type: Task Task
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 4.0-ALPHA
    • Component/s: None
    • Labels:
      None
    • Lucene Fields:
      New, Patch Available

      Description

      In LUCENE-2236 as a start, we made DefaultSimilarity which implements the factory interface (SimilarityProvider), and also extends Similarity.

      Its factory interface just returns itself always by default.

      Doron mentioned it would be cleaner to split the two, and I thought it would be good to revisit it later.

      Today as I was looking at SOLR-2338, it became pretty clear that we should do this, it makes things a lot cleaner. I think currently its confusing to users to see the two apis mixed if they are trying to subclass.

        Issue Links

          Activity

          Uwe Schindler made changes -
          Status Resolved [ 5 ] Closed [ 6 ]
          Gavin made changes -
          Link This issue is depended upon by SOLR-2338 [ SOLR-2338 ]
          Gavin made changes -
          Link This issue blocks SOLR-2338 [ SOLR-2338 ]
          Robert Muir made changes -
          Status Open [ 1 ] Resolved [ 5 ]
          Resolution Fixed [ 1 ]
          Hide
          Robert Muir added a comment -

          Committed revision 1085374.

          I cleaned up all the tests as Doron suggested, except for the solr MockConfigurableSimilarity as I discussed (since solr scoring is not yet per-field, but i have a patch for this that fixes the naming issues there too).

          Show
          Robert Muir added a comment - Committed revision 1085374. I cleaned up all the tests as Doron suggested, except for the solr MockConfigurableSimilarity as I discussed (since solr scoring is not yet per-field, but i have a patch for this that fixes the naming issues there too).
          rmuir committed 1085374 (23 files)
          Reviews: none

          LUCENE-2986: split DefaultSimilarity's provider methods into DefaultSimilarityProvider; clean up uses of sim in tests

          Lucene trunk
          Hide
          Robert Muir added a comment -

          Thanks, I agree the tests look messy/lazy... I'll fix these (well I think I would like to delay the solr one until SOLR-2338, i fixed the MockConfigurableSimilarity in that patch already)

          Show
          Robert Muir added a comment - Thanks, I agree the tests look messy/lazy... I'll fix these (well I think I would like to delay the solr one until SOLR-2338 , i fixed the MockConfigurableSimilarity in that patch already)
          Hide
          Doron Cohen added a comment -

          +1 for this change (I did not remember discussing this, but other than remembering I am consistent )

          Patch looks very clean.

          Minor technical comments - concerning just some tests:

          • some of the DSP implementations are still named xyzSimilarity - I think it would be more clear to name them xyzSimilarityProvider:
            • o.a.l.search.payloads.TestPayloadNearQuery.BoostingSimilarity
            • o.a.l.search.payloads.TestPayloadTermQuery.BoostingSimilarity
            • o.a.solr.schema.MockConfigurableSimilarity
            • o.a.l.index.TestIndexWriterConfig.MySimilarity
            • o.a.l.index.TestIndexReaderCloneNorms.SimilarityOne
            • o.a.l.index.TestNorms.SimilarityOne
            • o.a.l.index.TestOmitTf.SimpleSimilarity
            • o.a.l.search.TestSimilarity.SimpleSimilarity
          • for few of the above it is not only the name - they are still doing both roles:
            extends DefaultSimilarity implements SimilarityProvider

            :

            • o.a.l.search.payloads.TestPayloadNearQuery.BoostingSimilarity
            • o.a.l.search.payloads.TestPayloadTermQuery.BoostingSimilarity
            • o.a.l.index.TestOmitTf.SimpleSimilarity
            • o.a.l.search.TestSimilarity.SimpleSimilarity

          Other than that I think it is good to go in.

          Also, tests from trunk/lucene and trunk/solr passed.
          (I am seeing problems in running all trunk tests, at least on Windows, but I'll send a separate mail to the list on that)

          Show
          Doron Cohen added a comment - +1 for this change (I did not remember discussing this, but other than remembering I am consistent ) Patch looks very clean. Minor technical comments - concerning just some tests: some of the DSP implementations are still named xyzSimilarity - I think it would be more clear to name them xyzSimilarityProvider: o.a.l.search.payloads.TestPayloadNearQuery.BoostingSimilarity o.a.l.search.payloads.TestPayloadTermQuery.BoostingSimilarity o.a.solr.schema.MockConfigurableSimilarity o.a.l.index.TestIndexWriterConfig.MySimilarity o.a.l.index.TestIndexReaderCloneNorms.SimilarityOne o.a.l.index.TestNorms.SimilarityOne o.a.l.index.TestOmitTf.SimpleSimilarity o.a.l.search.TestSimilarity.SimpleSimilarity for few of the above it is not only the name - they are still doing both roles: extends DefaultSimilarity implements SimilarityProvider : o.a.l.search.payloads.TestPayloadNearQuery.BoostingSimilarity o.a.l.search.payloads.TestPayloadTermQuery.BoostingSimilarity o.a.l.index.TestOmitTf.SimpleSimilarity o.a.l.search.TestSimilarity.SimpleSimilarity Other than that I think it is good to go in. Also, tests from trunk/lucene and trunk/solr passed. (I am seeing problems in running all trunk tests, at least on Windows, but I'll send a separate mail to the list on that)
          Robert Muir made changes -
          Link This issue blocks SOLR-2338 [ SOLR-2338 ]
          Robert Muir made changes -
          Field Original Value New Value
          Attachment LUCENE-2986.patch [ 12474413 ]
          Hide
          Robert Muir added a comment -

          Attached is a patch: adds DefaultSimilarityProvider, which has our default implementations of the non-field-specific methods (coord/queryNorm/etc), and always returns DefaultSimilarity.

          Show
          Robert Muir added a comment - Attached is a patch: adds DefaultSimilarityProvider, which has our default implementations of the non-field-specific methods (coord/queryNorm/etc), and always returns DefaultSimilarity.
          Robert Muir created issue -

            People

            • Assignee:
              Robert Muir
              Reporter:
              Robert Muir
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development