Bug 18316 - Add weighting/relevancy options to ElasticSearch
Summary: Add weighting/relevancy options to ElasticSearch
Status: Failed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: Searching - Elasticsearch (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Alex Arnaud
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 20388 20602 20607
  Show dependency treegraph
 
Reported: 2017-03-22 15:36 UTC by Nick Clemens
Modified: 2018-04-19 12:04 UTC (History)
7 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: ---
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 18316 - Add weighting/relevancy options to ElasticSearch (4.04 KB, patch)
2017-03-22 15:39 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 18316 - Add weighting/relevancy options to ElasticSearch (4.02 KB, patch)
2018-03-21 13:38 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 18316 - Add weighting/relevancy options to ElasticSearch (4.08 KB, patch)
2018-03-21 13:40 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 18316 - Ability to weight search fields (11.81 KB, patch)
2018-03-30 15:20 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 18316 - Add weighting/relevancy options to ES query on simple search (6.78 KB, patch)
2018-03-30 15:20 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 18316 - Ability to weight search fields (11.95 KB, patch)
2018-04-04 10:01 UTC, Séverine Queune
Details | Diff | Splinter Review
Bug 18316 - Add weighting/relevancy options to ES query on simple search (6.93 KB, patch)
2018-04-04 10:01 UTC, Séverine Queune
Details | Diff | Splinter Review
Bug 18316 - String changes (1.51 KB, patch)
2018-04-12 16:13 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 18316: Ability to weight search fields (12.01 KB, patch)
2018-04-12 16:52 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 18316: Add weighting/relevancy options to ES query on simple search (6.98 KB, patch)
2018-04-12 16:52 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 18316: String changes (1.56 KB, patch)
2018-04-12 16:52 UTC, Nick Clemens
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens 2017-03-22 15:36:16 UTC

    
Comment 1 Nick Clemens 2017-03-22 15:39:16 UTC Comment hidden (obsolete)
Comment 2 Alex Arnaud 2018-03-21 13:38:31 UTC Comment hidden (obsolete)
Comment 3 Alex Arnaud 2018-03-21 13:40:41 UTC Comment hidden (obsolete)
Comment 4 Gaetan Boisson 2018-03-28 12:41:14 UTC
Alex is working on this.

We are assuming relevancy working is mostly important for simple search, or what zebra calls the "keyword" index.

What we plan to do:

Add columns to the "search fields" tab of the search engine configuration page, where the user will be able to indicate a weight for each field.

This will allow to configure the way the simple search is weighted: it will search in all fields for which a weight has been indicated. If no weight was indicated it will search in all fields. (In addition, this allows excluding some search fields from the simple search!)

This will not affect advanced search, in which the user can pick indices himself. (But it could, and we would then run the query with the weight defined for each field.)

The possibility to add a weight to a search field will only be offered if this field is used in the biblio mapping. So it won't be possible to give weight to a field only used in the authorities mapping, or defined but unused.

In the future it should be possible to have "koha indices" that search across multiple elasticsearch fields, each with a different weight. But this seems a bit ambitious for now, especially with 18.05 so close. So this seems like a reasonable goal for the next release, and certainly a very useful one.
Comment 5 Nick Clemens 2018-03-28 13:07:17 UTC
This sounds like a reasonable approach, looking forward to seeing it :-)
Comment 6 Alex Arnaud 2018-03-30 15:20:40 UTC
Created attachment 73492 [details] [review]
Bug 18316 - Ability to weight search fields

Weight can be defined between 1 and 99.
only search fields mapped with biblios can be weighted

Test plan:
  - apply this patch,
  - update schema file (perl misc/devel/update_dbix_class_files.pl),
  - go to Administration > Search engine configuration,
  - in search fields tab, define weights for some fields and save,
  - check weights has been saved,
  - reset some weights (empty) and save,
  - check weights has been reset
Comment 7 Alex Arnaud 2018-03-30 15:20:49 UTC
Created attachment 73493 [details] [review]
Bug 18316 - Add weighting/relevancy options to ES query on simple search

The idea is the following: if some search field(s) are weighted in
search
engine config page, Koha will query ES on these fields only and with
the coresponding weights. Else, search is done on the entire record.

Test plan (having Koha working with Elasticsearch):
  - apply this patch,
  - try searches with and without weight defined on search fields
Comment 8 Séverine Queune 2018-04-04 10:01:25 UTC
Created attachment 73600 [details] [review]
Bug 18316 - Ability to weight search fields

Weight can be defined between 1 and 99.
only search fields mapped with biblios can be weighted

Test plan:
  - apply this patch,
  - update schema file (perl misc/devel/update_dbix_class_files.pl),
  - go to Administration > Search engine configuration,
  - in search fields tab, define weights for some fields and save,
  - check weights has been saved,
  - reset some weights (empty) and save,
  - check weights has been reset

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Comment 9 Séverine Queune 2018-04-04 10:01:28 UTC
Created attachment 73601 [details] [review]
Bug 18316 - Add weighting/relevancy options to ES query on simple search

The idea is the following: if some search field(s) are weighted in
search
engine config page, Koha will query ES on these fields only and with
the coresponding weights. Else, search is done on the entire record.

Test plan (having Koha working with Elasticsearch):
  - apply this patch,
  - try searches with and without weight defined on search fields

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Comment 10 Séverine Queune 2018-04-04 14:33:28 UTC
Patchs work as described.

I misunderstood the way to use numbers, thinking "1" was the most important field(s) I wanted to search in.
Maybe a warning or more detailed description would be useful for reindex can take time for big databases, it would be sad if others make the same mistake as I.

This weighting add a lot of relevance to the simple search.

In reaction to Gaetan's comment, it also appears that having weight on sub-fields used on the same index would be great.
For example, when searching with index "title" in our Unimarc system, we would like to display books that contains the searched word in 200$b before the ones in 410$t.
Comment 11 Alex Arnaud 2018-04-04 15:11:45 UTC
(In reply to Séverine Queune from comment #10)
> Patchs work as described.
> 
> I misunderstood the way to use numbers, thinking "1" was the most important
> field(s) I wanted to search in.
> Maybe a warning or more detailed description would be useful for reindex can
> take time for big databases, it would be sad if others make the same mistake
> as I.
You mean add a warning to tell users that more the weight is hight more the field is important?
> 
> This weighting add a lot of relevance to the simple search.
> 
> In reaction to Gaetan's comment, it also appears that having weight on
> sub-fields used on the same index would be great.
> For example, when searching with index "title" in our Unimarc system, we
> would like to display books that contains the searched word in 200$b before
> the ones in 410$t.
I'm not yet confortable enough with all ES capabilities, but it could be done by adding a "other-title" search field with a different weight. Does it make sens for you?
Comment 12 Nicolas Legrand 2018-04-11 11:21:57 UTC
(In reply to Alex Arnaud from comment #11)
> (In reply to Séverine Queune from comment #10)
> > Patchs work as described.
> > 
> > I misunderstood the way to use numbers, thinking "1" was the most important
> > field(s) I wanted to search in.
> > Maybe a warning or more detailed description would be useful for reindex can
> > take time for big databases, it would be sad if others make the same mistake
> > as I.
> You mean add a warning to tell users that more the weight is hight more the
> field is important?

I think that's what she meant.

> > This weighting add a lot of relevance to the simple search.
> > 
> > In reaction to Gaetan's comment, it also appears that having weight on
> > sub-fields used on the same index would be great.
> > For example, when searching with index "title" in our Unimarc system, we
> > would like to display books that contains the searched word in 200$b before
> > the ones in 410$t.
> I'm not yet confortable enough with all ES capabilities, but it could be
> done by adding a "other-title" search field with a different weight. Does it
> make sens for you?

Yes, but not to our librarians colleagues who want to describe titles in a myriad of MARC' subfield while uniting them all under one index entry. I'm not sure to be able to convince them this may not be a good idea :). The 4XX$t as a title index  for instance, is a relevancy killer. I'll try to convince them to bind it to a linked-with-title index.

Anyway, having a different relevancy for same index/different subfield may be another bug.
Comment 13 Nick Clemens 2018-04-12 11:28:30 UTC
I agree - different weighting for specific fields in an index would be a separate bug

Searching and weighting works well and tests pass

I have questions about the comments:

>Weight: define weight between 1 and 99.
add: Higher numbers indicate increased relevenacy

>search will be done on weighted fields only
this doesn't seem to be the case - weight author - search for an isbn, you get the record

>if no field is weighted, search will be done on all the record
true, but we search all fields even with some wieghts

I think the behaviour is correct, searching _all and adding weighted fields, just need to adjust comments

Should we indicate that weights are not used during adv search? (If we want them I think that can be done on a future patch)
Comment 14 Alex Arnaud 2018-04-12 15:57:17 UTC
(In reply to Nick Clemens from comment #13)
> I agree - different weighting for specific fields in an index would be a
> separate bug
+1
> 
> Searching and weighting works well and tests pass
> 
> I have questions about the comments:
> 
> >Weight: define weight between 1 and 99.
> add: Higher numbers indicate increased relevenacy
> 
> >search will be done on weighted fields only
> this doesn't seem to be the case - weight author - search for an isbn, you
> get the record
Right. Search is done on all fields but boost the weighted ones.
> 
> >if no field is weighted, search will be done on all the record
> true, but we search all fields even with some wieghts
> 
> I think the behaviour is correct, searching _all and adding weighted fields,
> just need to adjust comments
> 
> Should we indicate that weights are not used during adv search? (If we want
> them I think that can be done on a future patch)
I can do it. The future patch for adv search will have to remove the comment.
Comment 15 Alex Arnaud 2018-04-12 16:13:59 UTC
Created attachment 74112 [details] [review]
Bug 18316 - String changes
Comment 16 Nick Clemens 2018-04-12 16:52:14 UTC
Created attachment 74115 [details] [review]
Bug 18316: Ability to weight search fields

Weight can be defined between 1 and 99.
only search fields mapped with biblios can be weighted

Test plan:
  - apply this patch,
  - update schema file (perl misc/devel/update_dbix_class_files.pl),
  - go to Administration > Search engine configuration,
  - in search fields tab, define weights for some fields and save,
  - check weights has been saved,
  - reset some weights (empty) and save,
  - check weights has been reset

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 17 Nick Clemens 2018-04-12 16:52:18 UTC
Created attachment 74116 [details] [review]
Bug 18316: Add weighting/relevancy options to ES query on simple search

The idea is the following: if some search field(s) are weighted in
search
engine config page, Koha will query ES on these fields only and with
the coresponding weights. Else, search is done on the entire record.

Test plan (having Koha working with Elasticsearch):
  - apply this patch,
  - try searches with and without weight defined on search fields

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 18 Nick Clemens 2018-04-12 16:52:21 UTC
Created attachment 74117 [details] [review]
Bug 18316: String changes

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 19 Jonathan Druart 2018-04-18 20:33:43 UTC
1. 
From Koha::SearchFields->weighted_fields

+    while ( my $field = $fields->next ) {
+        push @$w_fields, $field->name;
+        push @$weight, $field->weight;
+    }
+
+    return ($w_fields, $weight);

From search.pl:
($w_fields, $weight) = Koha::SearchFields->weighted_fields();

then we call build_query_compat with { w_fields => @$w_fields, weight => @$weight  }

which will finally do:
+    if ( defined $weights[0] ) {
+        for (my $i = 0 ; $i < (scalar @weights) ; $i++ ){
+            push @fields, "$w_fields[$i]^$weights[$i]";
+        }
+    }

so basically you want to fetch the weighted fields from build_query_compat, with something like:
  push @fields, sprintf("%s^%s", $_->name, $_->weight) for Koha::SearchFields->weighted_fields;

That seems much more easier, what did I miss?


2. Sounds like you could replaced the loop in is_mapped_biblios, with $self->search_marc_maps->search({ index_name => 'biblios' })->count ? 1 : 0;

3. search_marc_maps reads wrong too.
Comment 20 Alex Arnaud 2018-04-19 12:04:18 UTC
(In reply to Jonathan Druart from comment #19)
> 1. 
> From Koha::SearchFields->weighted_fields
> 
> +    while ( my $field = $fields->next ) {
> +        push @$w_fields, $field->name;
> +        push @$weight, $field->weight;
> +    }
> +
> +    return ($w_fields, $weight);
> 
> From search.pl:
> ($w_fields, $weight) = Koha::SearchFields->weighted_fields();
> 
> then we call build_query_compat with { w_fields => @$w_fields, weight =>
> @$weight  }
> 
> which will finally do:
> +    if ( defined $weights[0] ) {
> +        for (my $i = 0 ; $i < (scalar @weights) ; $i++ ){
> +            push @fields, "$w_fields[$i]^$weights[$i]";
> +        }
> +    }
> 
> so basically you want to fetch the weighted fields from build_query_compat,
> with something like:
>   push @fields, sprintf("%s^%s", $_->name, $_->weight) for
> Koha::SearchFields->weighted_fields;
> 
> That seems much more easier, what did I miss?
We don't want to use weighted fields on advanced search and build_query_compat is called for both simple and adv search.
But i can propose something similar in search.pl
> 
> 
> 2. Sounds like you could replaced the loop in is_mapped_biblios, with
> $self->search_marc_maps->search({ index_name => 'biblios' })->count ? 1 : 0;
Right.
> 
> 3. search_marc_maps reads wrong too.
What is wrong?