Uploaded image for project: 'Lucene - Core'
  1. Lucene - Core
  2. LUCENE-7486

DisjunctionMaxScorer Initializes scoreMax to Zero Preventing From Using Negative Scores

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 5.5.2
    • Fix Version/s: 6.x, 6.3, 7.0
    • Component/s: core/search
    • Labels:
      None
    • Lucene Fields:
      New

      Description

      We are using a log of probability for scoring, which gives us negative scores.

      DisjunctionMaxScorer initializes scoreMax in the score(...) function to zero preventing us from using negative scores. Is there a reason it couldn't be initialized to something like this:

      float scoreMax = Float.MAX_VALUE * -1;

      1. LUCENE-7486.patch
        2 kB
        Uwe Schindler
      2. LUCENE-7486.patch
        0.8 kB
        Uwe Schindler

        Activity

        Hide
        shalinmangar Shalin Shekhar Mangar added a comment -

        Closing after 6.3.0 release.

        Show
        shalinmangar Shalin Shekhar Mangar added a comment - Closing after 6.3.0 release.
        Hide
        thetaphi Uwe Schindler added a comment -

        Thanks Adrien for fixing this bug!

        Show
        thetaphi Uwe Schindler added a comment - Thanks Adrien for fixing this bug!
        Hide
        jira-bot ASF subversion and git services added a comment -

        Commit 9304ef9f118d24f76b280299706310ca8a0d40e6 in lucene-solr's branch refs/heads/master from Adrien Grand
        [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=9304ef9 ]

        LUCENE-7486: Explain() should initialize maxScore to NEGATIVE_INFINITY too.

        Show
        jira-bot ASF subversion and git services added a comment - Commit 9304ef9f118d24f76b280299706310ca8a0d40e6 in lucene-solr's branch refs/heads/master from Adrien Grand [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=9304ef9 ] LUCENE-7486 : Explain() should initialize maxScore to NEGATIVE_INFINITY too.
        Hide
        jira-bot ASF subversion and git services added a comment -

        Commit b5a98cc52c29dbcb79299396d44db6b604c2c422 in lucene-solr's branch refs/heads/branch_6x from Adrien Grand
        [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=b5a98cc ]

        LUCENE-7486: Explain() should initialize maxScore to NEGATIVE_INFINITY too.

        Show
        jira-bot ASF subversion and git services added a comment - Commit b5a98cc52c29dbcb79299396d44db6b604c2c422 in lucene-solr's branch refs/heads/branch_6x from Adrien Grand [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=b5a98cc ] LUCENE-7486 : Explain() should initialize maxScore to NEGATIVE_INFINITY too.
        Hide
        iprovalo Ivan Provalov added a comment -

        +1

        Show
        iprovalo Ivan Provalov added a comment - +1
        Hide
        jira-bot ASF subversion and git services added a comment -

        Commit 3bd3a4cbc460db20e29e242f27041c20a03218de in lucene-solr's branch refs/heads/branch_6x from Uwe Schindler
        [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=3bd3a4c ]

        LUCENE-7486: DisjunctionMaxQuery does not work correctly with queries that return negative scores

        Show
        jira-bot ASF subversion and git services added a comment - Commit 3bd3a4cbc460db20e29e242f27041c20a03218de in lucene-solr's branch refs/heads/branch_6x from Uwe Schindler [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=3bd3a4c ] LUCENE-7486 : DisjunctionMaxQuery does not work correctly with queries that return negative scores
        Hide
        jira-bot ASF subversion and git services added a comment -

        Commit d25aba2336fe8e7e2a831ccceb6635eb8881cf0f in lucene-solr's branch refs/heads/master from Uwe Schindler
        [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=d25aba2 ]

        LUCENE-7486: DisjunctionMaxQuery does not work correctly with queries that return negative scores

        Show
        jira-bot ASF subversion and git services added a comment - Commit d25aba2336fe8e7e2a831ccceb6635eb8881cf0f in lucene-solr's branch refs/heads/master from Uwe Schindler [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=d25aba2 ] LUCENE-7486 : DisjunctionMaxQuery does not work correctly with queries that return negative scores
        Hide
        jpountz Adrien Grand added a comment -

        +1

        Show
        jpountz Adrien Grand added a comment - +1
        Hide
        thetaphi Uwe Schindler added a comment -

        New patch with test. Test fails without the NEGATIVE_INFINITY fix. The trick was to use a BoostQuery with negative boost.

        Show
        thetaphi Uwe Schindler added a comment - New patch with test. Test fails without the NEGATIVE_INFINITY fix. The trick was to use a BoostQuery with negative boost.
        Hide
        thetaphi Uwe Schindler added a comment -

        Simple patch. All Lucene Core tests pass. I am not sure if its worth to add some kind of test for this.

        I also checked the code: It should behave the same as before. The score returned can never be NEGATIVE_INFINITY by default, because DisjSumScorer must at least have one scorer, whose score is always > than NEGATIVE_INFINITY. There is only a difference if one of the subscorers returns a score < 0, which is what this issue wants to fix.

        The simplest way to test this would be to create a test using a ConstantScoreQuery with a negative score and add it to DisjMaxQuery.

        I will run all tests later, but for now I posted the patch.

        Show
        thetaphi Uwe Schindler added a comment - Simple patch. All Lucene Core tests pass. I am not sure if its worth to add some kind of test for this. I also checked the code: It should behave the same as before. The score returned can never be NEGATIVE_INFINITY by default, because DisjSumScorer must at least have one scorer, whose score is always > than NEGATIVE_INFINITY. There is only a difference if one of the subscorers returns a score < 0, which is what this issue wants to fix. The simplest way to test this would be to create a test using a ConstantScoreQuery with a negative score and add it to DisjMaxQuery. I will run all tests later, but for now I posted the patch.
        Hide
        iprovalo Ivan Provalov added a comment -

        Thanks, Uwe!

        Show
        iprovalo Ivan Provalov added a comment - Thanks, Uwe!
        Hide
        thetaphi Uwe Schindler added a comment -

        Of course, the comment was just about your suggestion. Of course we can fix this in Lucene, it is not a risk at all.

        Show
        thetaphi Uwe Schindler added a comment - Of course, the comment was just about your suggestion. Of course we can fix this in Lucene, it is not a risk at all.
        Hide
        thetaphi Uwe Schindler added a comment -

        I can take care of this! It should be a one-line change only, if all tests pass.

        Show
        thetaphi Uwe Schindler added a comment - I can take care of this! It should be a one-line change only, if all tests pass.
        Hide
        jpountz Adrien Grand added a comment -

        I'm not sure DisjunctionMaxScorer makes a lot of sense with negative scores, but I'd be +1 to initialize scoreMax with Float.NEGATIVE_INFINTY like Uwe suggested for consistency.

        Show
        jpountz Adrien Grand added a comment - I'm not sure DisjunctionMaxScorer makes a lot of sense with negative scores, but I'd be +1 to initialize scoreMax with Float.NEGATIVE_INFINTY like Uwe suggested for consistency.
        Hide
        iprovalo Ivan Provalov added a comment -

        Good point, Uwe. Is there a reason it shouldn't be done in Lucene source?

        Show
        iprovalo Ivan Provalov added a comment - Good point, Uwe. Is there a reason it shouldn't be done in Lucene source?
        Hide
        thetaphi Uwe Schindler added a comment -

        It should be Float.NEGATIVE_INFINTY if you would like to do this.

        Show
        thetaphi Uwe Schindler added a comment - It should be Float.NEGATIVE_INFINTY if you would like to do this.

          People

          • Assignee:
            thetaphi Uwe Schindler
            Reporter:
            iprovalo Ivan Provalov
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development