Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 4.8, 6.0
    • Component/s: None
    • Labels:
      None
    • Lucene Fields:
      New

      Description

      In LUCENE-5489 we added QueryRescorer, to rescore first-pass hits using scores from a (usually) more expensive second-pass query.

      I think we should also add ExpressionRescorer, to compute the second pass score using an arbitrary JS expression.

      1. LUCENE-5545.patch
        37 kB
        Michael McCandless
      2. LUCENE-5545.patch
        23 kB
        Michael McCandless
      3. LUCENE-5545.patch
        14 kB
        Michael McCandless
      4. LUCENE-5545.patch
        12 kB
        Michael McCandless

        Activity

        Hide
        Michael McCandless added a comment -

        Rough first patch quickly banged out ... it has nocommits, but I think it's a good start. It has a testBasic!

        Show
        Michael McCandless added a comment - Rough first patch quickly banged out ... it has nocommits, but I think it's a good start. It has a testBasic!
        Hide
        Michael McCandless added a comment -

        Another iteration, with explain() implemented.

        It's nice because explain() includes the values of each of the bindings ... so you can see the values that led to the overall expression.

        Still nocommits for better tests ...

        Show
        Michael McCandless added a comment - Another iteration, with explain() implemented. It's nice because explain() includes the values of each of the bindings ... so you can see the values that led to the overall expression. Still nocommits for better tests ...
        Hide
        Robert Muir added a comment -

        what does the second ctor do?

        Show
        Robert Muir added a comment - what does the second ctor do?
        Hide
        Robert Muir added a comment -

        I think i see, i don't actually like the hardcoded javascript+bindings there.

        The way this is currently handled in the package is a little different. Impls like this class are package private and you get a "factory" method on Expression.java, which keeps the API surface area very small (http://lucene.apache.org/core/4_7_0/expressions/org/apache/lucene/expressions/package-summary.html), but still allows e.g. specialization in the case of certain implementations (for example, haversin function could override getSortField to do some of those optimizations Ted mentioned on another issue).

        public ValueSource getValueSource(Bindings bindings);
        public SortField getSortField(Bindings bindings, boolean reverse);
        

        Perhaps we should do the same with this Rescorer?

        public Rescorer getRescorer(Bindings bindings);
        
        Show
        Robert Muir added a comment - I think i see, i don't actually like the hardcoded javascript+bindings there. The way this is currently handled in the package is a little different. Impls like this class are package private and you get a "factory" method on Expression.java, which keeps the API surface area very small ( http://lucene.apache.org/core/4_7_0/expressions/org/apache/lucene/expressions/package-summary.html ), but still allows e.g. specialization in the case of certain implementations (for example, haversin function could override getSortField to do some of those optimizations Ted mentioned on another issue). public ValueSource getValueSource(Bindings bindings); public SortField getSortField(Bindings bindings, boolean reverse); Perhaps we should do the same with this Rescorer? public Rescorer getRescorer(Bindings bindings);
        Hide
        Michael McCandless added a comment -

        Another iteration.

        I factored out a SortRescorer, which lets you re-rank according to any Sort, and made ExpressionRescorer package private and only accessible via Expression.getRescorer. It just subclasses SortRescorer and overrides explain to add the variable bindings for that doc ...

        Still nocommits / iterations to do.

        Show
        Michael McCandless added a comment - Another iteration. I factored out a SortRescorer, which lets you re-rank according to any Sort, and made ExpressionRescorer package private and only accessible via Expression.getRescorer. It just subclasses SortRescorer and overrides explain to add the variable bindings for that doc ... Still nocommits / iterations to do.
        Hide
        Robert Muir added a comment -

        This is looking great!

        Show
        Robert Muir added a comment - This is looking great!
        Hide
        Michael McCandless added a comment -

        New patch, improving the tests, which found a problem (infinite loop!)
        with QueryRescorer, which I fixed by removing entirely the
        OnlyDocIDsFilter. Now I just interact directly w/ the Scorer per
        segment instead, and it also addresses the TODO about not creating
        lots of objects.

        I think it's ready.

        Show
        Michael McCandless added a comment - New patch, improving the tests, which found a problem (infinite loop!) with QueryRescorer, which I fixed by removing entirely the OnlyDocIDsFilter. Now I just interact directly w/ the Scorer per segment instead, and it also addresses the TODO about not creating lots of objects. I think it's ready.
        Hide
        Robert Muir added a comment -

        +1

        Show
        Robert Muir added a comment - +1
        Hide
        Ryan Ernst added a comment -

        +1

        I think you can remove the comment in QueryRescorer to add an ExpressionRescorer?

        Show
        Ryan Ernst added a comment - +1 I think you can remove the comment in QueryRescorer to add an ExpressionRescorer?
        Hide
        Uwe Schindler added a comment -

        +1 very nice!

        Show
        Uwe Schindler added a comment - +1 very nice!
        Hide
        Michael McCandless added a comment -

        I think you can remove the comment in QueryRescorer to add an ExpressionRescorer?

        Woops, thanks for catching! I'll remove it.

        Show
        Michael McCandless added a comment - I think you can remove the comment in QueryRescorer to add an ExpressionRescorer? Woops, thanks for catching! I'll remove it.
        Hide
        Shalin Shekhar Mangar added a comment -

        +1 This is neat!

        Show
        Shalin Shekhar Mangar added a comment - +1 This is neat!
        Hide
        ASF subversion and git services added a comment -

        Commit 1580490 from Michael McCandless in branch 'dev/trunk'
        [ https://svn.apache.org/r1580490 ]

        LUCENE-5545: add SortRescorer and Expression.getRescorer

        Show
        ASF subversion and git services added a comment - Commit 1580490 from Michael McCandless in branch 'dev/trunk' [ https://svn.apache.org/r1580490 ] LUCENE-5545 : add SortRescorer and Expression.getRescorer
        Hide
        ASF subversion and git services added a comment -

        Commit 1580491 from Michael McCandless in branch 'dev/branches/branch_4x'
        [ https://svn.apache.org/r1580491 ]

        LUCENE-5545: add SortRescorer and Expression.getRescorer

        Show
        ASF subversion and git services added a comment - Commit 1580491 from Michael McCandless in branch 'dev/branches/branch_4x' [ https://svn.apache.org/r1580491 ] LUCENE-5545 : add SortRescorer and Expression.getRescorer
        Hide
        Uwe Schindler added a comment -

        Close issue after release of 4.8.0

        Show
        Uwe Schindler added a comment - Close issue after release of 4.8.0

          People

          • Assignee:
            Michael McCandless
            Reporter:
            Michael McCandless
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development