From b0a493a41c6481e10ef6e737f18adcf706c3f64e Mon Sep 17 00:00:00 2001 From: Mayya Sharipova Date: Wed, 10 Mar 2021 14:55:54 -0500 Subject: [PATCH] Add positive_score_impact to rank_features type rank_features field type misses positive_score_impact parameter that rank_feature type has. This adds this parameter. Backport for #69994 Closes #68619 --- .../mapping/types/rank-features.asciidoc | 37 ++++++++++++++-- .../index/mapper/RankFeaturesFieldMapper.java | 31 +++++++++++--- .../mapper/RankFeaturesFieldMapperTests.java | 31 +++++++++++++- .../mapper/RankFeaturesFieldTypeTests.java | 2 +- .../test/rank_features/10_basic.yml | 42 +++++++++++++++++++ 5 files changed, 131 insertions(+), 12 deletions(-) diff --git a/docs/reference/mapping/types/rank-features.asciidoc b/docs/reference/mapping/types/rank-features.asciidoc index 69bbdbb0bc4db..0218fd3136631 100644 --- a/docs/reference/mapping/types/rank-features.asciidoc +++ b/docs/reference/mapping/types/rank-features.asciidoc @@ -20,6 +20,10 @@ PUT my-index-000001 "properties": { "topics": { "type": "rank_features" <1> + }, + "negative_reviews" : { + "type": "rank_features", + "positive_score_impact": false <2> } } } @@ -27,9 +31,13 @@ PUT my-index-000001 PUT my-index-000001/_doc/1 { - "topics": { <2> + "topics": { <3> "politics": 20, "economics": 50.8 + }, + "negative_reviews": { + "1star": 10, + "2star": 100 } } @@ -38,21 +46,38 @@ PUT my-index-000001/_doc/2 "topics": { "politics": 5.2, "sports": 80.1 + }, + "negative_reviews": { + "1star": 1, + "2star": 10 } } GET my-index-000001/_search { - "query": { + "query": { <4> "rank_feature": { "field": "topics.politics" } } } + +GET my-index-000001/_search +{ + "query": { <5> + "rank_feature": { + "field": "negative_reviews.1star" + } + } +} -------------------------------------------------- <1> Rank features fields must use the `rank_features` field type -<2> Rank features fields must be a hash with string keys and strictly positive numeric values +<2> Rank features that correlate negatively with the score need to declare it +<3> Rank features fields must be a hash with string keys and strictly positive numeric values +<4> This query ranks documents by how much they are about the "politics" topic. +<5> This query ranks documents inversely to the number of "1star" reviews they received. + NOTE: `rank_features` fields only support single-valued features and strictly positive values. Multi-valued fields and zero or negative values will be rejected. @@ -63,3 +88,9 @@ only be queried using <> queries. NOTE: `rank_features` fields only preserve 9 significant bits for the precision, which translates to a relative error of about 0.4%. +Rank features that correlate negatively with the score should set +`positive_score_impact` to `false` (defaults to `true`). This will be used by +the <> query to modify the scoring formula +in such a way that the score decreases with the value of the feature instead of +increasing. + diff --git a/modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/RankFeaturesFieldMapper.java b/modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/RankFeaturesFieldMapper.java index a0538ebb48321..02867f17693a7 100644 --- a/modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/RankFeaturesFieldMapper.java +++ b/modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/RankFeaturesFieldMapper.java @@ -17,7 +17,7 @@ import org.elasticsearch.search.lookup.SearchLookup; import java.io.IOException; -import java.util.Collections; +import java.util.Arrays; import java.util.List; import java.util.Map; import java.util.function.Supplier; @@ -30,8 +30,14 @@ public class RankFeaturesFieldMapper extends FieldMapper { public static final String CONTENT_TYPE = "rank_features"; + private static RankFeaturesFieldType ft(FieldMapper in) { + return ((RankFeaturesFieldMapper)in).fieldType(); + } + public static class Builder extends FieldMapper.Builder { + private final Parameter positiveScoreImpact + = Parameter.boolParam("positive_score_impact", false, m -> ft(m).positiveScoreImpact, true); private final Parameter> meta = Parameter.metaParam(); public Builder(String name) { @@ -40,14 +46,14 @@ public Builder(String name) { @Override protected List> getParameters() { - return Collections.singletonList(meta); + return Arrays.asList(positiveScoreImpact, meta); } @Override public RankFeaturesFieldMapper build(ContentPath contentPath) { return new RankFeaturesFieldMapper( - name, new RankFeaturesFieldType(buildFullName(contentPath), meta.getValue()), - multiFieldsBuilder.build(this, contentPath), copyTo.build()); + name, new RankFeaturesFieldType(buildFullName(contentPath), meta.getValue(), positiveScoreImpact.getValue()), + multiFieldsBuilder.build(this, contentPath), copyTo.build(), positiveScoreImpact.getValue()); } } @@ -55,8 +61,11 @@ name, new RankFeaturesFieldType(buildFullName(contentPath), meta.getValue()), public static final class RankFeaturesFieldType extends MappedFieldType { - public RankFeaturesFieldType(String name, Map meta) { + private final boolean positiveScoreImpact; + + public RankFeaturesFieldType(String name, Map meta, boolean positiveScoreImpact) { super(name, false, false, false, TextSearchInfo.NONE, meta); + this.positiveScoreImpact = positiveScoreImpact; } @Override @@ -64,6 +73,10 @@ public String typeName() { return CONTENT_TYPE; } + public boolean positiveScoreImpact() { + return positiveScoreImpact; + } + @Override public Query existsQuery(SearchExecutionContext context) { throw new IllegalArgumentException("[rank_features] fields do not support [exists] queries"); @@ -85,9 +98,12 @@ public Query termQuery(Object value, SearchExecutionContext context) { } } + private final boolean positiveScoreImpact; + private RankFeaturesFieldMapper(String simpleName, MappedFieldType mappedFieldType, - MultiFields multiFields, CopyTo copyTo) { + MultiFields multiFields, CopyTo copyTo, boolean positiveScoreImpact) { super(simpleName, mappedFieldType, Lucene.KEYWORD_ANALYZER, multiFields, copyTo); + this.positiveScoreImpact = positiveScoreImpact; } @Override @@ -124,6 +140,9 @@ public void parse(ParseContext context) throws IOException { throw new IllegalArgumentException("[rank_features] fields do not support indexing multiple values for the same " + "rank feature [" + key + "] in the same document"); } + if (positiveScoreImpact == false) { + value = 1 / value; + } context.doc().addWithKey(key, new FeatureField(name(), feature, value)); } else { throw new IllegalArgumentException("[rank_features] fields take hashes that map a feature to a strictly positive " + diff --git a/modules/mapper-extras/src/test/java/org/elasticsearch/index/mapper/RankFeaturesFieldMapperTests.java b/modules/mapper-extras/src/test/java/org/elasticsearch/index/mapper/RankFeaturesFieldMapperTests.java index e9271ae1bb5d1..0300411bb0298 100644 --- a/modules/mapper-extras/src/test/java/org/elasticsearch/index/mapper/RankFeaturesFieldMapperTests.java +++ b/modules/mapper-extras/src/test/java/org/elasticsearch/index/mapper/RankFeaturesFieldMapperTests.java @@ -44,8 +44,8 @@ protected void minimalMapping(XContentBuilder b) throws IOException { } @Override - protected void registerParameters(ParameterChecker checker) { - // no parameters to configure + protected void registerParameters(ParameterChecker checker) throws IOException { + checker.registerConflictCheck("positive_score_impact", b -> b.field("positive_score_impact", false)); } @Override @@ -79,6 +79,33 @@ public void testDefaults() throws Exception { assertTrue(freq1 < freq2); } + public void testNegativeScoreImpact() throws Exception { + DocumentMapper mapper = createDocumentMapper( + fieldMapping(b -> b.field("type", "rank_features").field("positive_score_impact", false)) + ); + + ParsedDocument doc1 = mapper.parse(source(this::writeField)); + + IndexableField[] fields = doc1.rootDoc().getFields("field"); + assertEquals(2, fields.length); + assertThat(fields[0], Matchers.instanceOf(FeatureField.class)); + FeatureField featureField1 = null; + FeatureField featureField2 = null; + for (IndexableField field : fields) { + if (field.stringValue().equals("ten")) { + featureField1 = (FeatureField)field; + } else if (field.stringValue().equals("twenty")) { + featureField2 = (FeatureField)field; + } else { + throw new UnsupportedOperationException(); + } + } + + int freq1 = RankFeatureFieldMapperTests.getFrequency(featureField1.tokenStream(null, null)); + int freq2 = RankFeatureFieldMapperTests.getFrequency(featureField2.tokenStream(null, null)); + assertTrue(freq1 > freq2); + } + public void testRejectMultiValuedFields() throws MapperParsingException, IOException { DocumentMapper mapper = createDocumentMapper(mapping(b -> { b.startObject("field").field("type", "rank_features").endObject(); diff --git a/modules/mapper-extras/src/test/java/org/elasticsearch/index/mapper/RankFeaturesFieldTypeTests.java b/modules/mapper-extras/src/test/java/org/elasticsearch/index/mapper/RankFeaturesFieldTypeTests.java index 7ca39594bf6ec..595f4827afc5a 100644 --- a/modules/mapper-extras/src/test/java/org/elasticsearch/index/mapper/RankFeaturesFieldTypeTests.java +++ b/modules/mapper-extras/src/test/java/org/elasticsearch/index/mapper/RankFeaturesFieldTypeTests.java @@ -13,7 +13,7 @@ public class RankFeaturesFieldTypeTests extends FieldTypeTestCase { public void testIsNotAggregatable() { - MappedFieldType fieldType = new RankFeaturesFieldMapper.RankFeaturesFieldType("field", Collections.emptyMap()); + MappedFieldType fieldType = new RankFeaturesFieldMapper.RankFeaturesFieldType("field", Collections.emptyMap(), true); assertFalse(fieldType.isAggregatable()); } } diff --git a/modules/mapper-extras/src/yamlRestTest/resources/rest-api-spec/test/rank_features/10_basic.yml b/modules/mapper-extras/src/yamlRestTest/resources/rest-api-spec/test/rank_features/10_basic.yml index 6314f8f218d16..ffa8712256222 100644 --- a/modules/mapper-extras/src/yamlRestTest/resources/rest-api-spec/test/rank_features/10_basic.yml +++ b/modules/mapper-extras/src/yamlRestTest/resources/rest-api-spec/test/rank_features/10_basic.yml @@ -13,6 +13,10 @@ setup: properties: tags: type: rank_features + negative_reviews: + type: rank_features + positive_score_impact: false + - do: index: @@ -22,6 +26,9 @@ setup: tags: foo: 3 bar: 5 + negative_reviews: + 1star: 10 + 2star: 1 - do: index: @@ -31,6 +38,9 @@ setup: tags: bar: 6 quux: 10 + negative_reviews: + 1star: 1 + 2star: 10 - do: indices.refresh: {} @@ -129,3 +139,35 @@ setup: hits.hits.1._id: "1" - match: hits.hits.1._score: 5.0 + + +--- +"Linear negative impact": + + - do: + search: + index: test + body: + query: + rank_feature: + field: negative_reviews.1star + linear: {} + + - match: + hits.hits.0._id: "2" + - match: + hits.hits.1._id: "1" + + - do: + search: + index: test + body: + query: + rank_feature: + field: negative_reviews.2star + linear: {} + + - match: + hits.hits.0._id: "1" + - match: + hits.hits.1._id: "2"