From adb24333caf72e7bc3e99eecc34f2ca5f9cd9070 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20B=C3=BCscher?= Date: Tue, 8 Nov 2016 12:17:36 +0100 Subject: [PATCH] Remove maxAcceptableRank parameter from ReciprocalRank --- .../index/rankeval/ReciprocalRank.java | 42 +++---------------- .../index/rankeval/ReciprocalRankTests.java | 35 ++++++---------- 2 files changed, 19 insertions(+), 58 deletions(-) diff --git a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/ReciprocalRank.java b/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/ReciprocalRank.java index 160cc0aee77..58cc8a3f100 100644 --- a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/ReciprocalRank.java +++ b/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/ReciprocalRank.java @@ -45,8 +45,6 @@ import static org.elasticsearch.index.rankeval.RankedListQualityMetric.joinHitsW public class ReciprocalRank implements RankedListQualityMetric { public static final String NAME = "reciprocal_rank"; - public static final int DEFAULT_MAX_ACCEPTABLE_RANK = Integer.MAX_VALUE; - private int maxAcceptableRank = DEFAULT_MAX_ACCEPTABLE_RANK; /** ratings equal or above this value will be considered relevant. */ private int relevantRatingThreshhold = 1; @@ -58,19 +56,8 @@ public class ReciprocalRank implements RankedListQualityMetric { // use defaults } - /** - * @param maxAcceptableRank - * maximal acceptable rank. Must be positive. - */ - public ReciprocalRank(int maxAcceptableRank) { - if (maxAcceptableRank <= 0) { - throw new IllegalArgumentException("maximal acceptable rank needs to be positive but was [" + maxAcceptableRank + "]"); - } - this.maxAcceptableRank = maxAcceptableRank; - } - public ReciprocalRank(StreamInput in) throws IOException { - this.maxAcceptableRank = in.readInt(); + this.relevantRatingThreshhold = in.readInt(); } @Override @@ -78,21 +65,6 @@ public class ReciprocalRank implements RankedListQualityMetric { return NAME; } - /** - * @param maxAcceptableRank - * maximal acceptable rank. Must be positive. - */ - public void setMaxAcceptableRank(int maxAcceptableRank) { - if (maxAcceptableRank <= 0) { - throw new IllegalArgumentException("maximal acceptable rank needs to be positive but was [" + maxAcceptableRank + "]"); - } - this.maxAcceptableRank = maxAcceptableRank; - } - - public int getMaxAcceptableRank() { - return this.maxAcceptableRank; - } - /** * Sets the rating threshold above which ratings are considered to be "relevant" for this metric. * */ @@ -117,7 +89,7 @@ public class ReciprocalRank implements RankedListQualityMetric { List ratedHits = joinHitsWithRatings(hits, ratedDocs); int firstRelevant = -1; int rank = 1; - for (RatedSearchHit hit : ratedHits.subList(0, Math.min(maxAcceptableRank, ratedHits.size()))) { + for (RatedSearchHit hit : ratedHits) { Optional rating = hit.getRating(); if (rating.isPresent()) { if (rating.get() >= this.relevantRatingThreshhold) { @@ -137,16 +109,14 @@ public class ReciprocalRank implements RankedListQualityMetric { @Override public void writeTo(StreamOutput out) throws IOException { - out.writeVInt(maxAcceptableRank); + out.writeVInt(relevantRatingThreshhold); } - private static final ParseField MAX_RANK_FIELD = new ParseField("max_acceptable_rank"); private static final ParseField RELEVANT_RATING_FIELD = new ParseField("relevant_rating_threshold"); private static final ObjectParser PARSER = new ObjectParser<>( "reciprocal_rank", () -> new ReciprocalRank()); static { - PARSER.declareInt(ReciprocalRank::setMaxAcceptableRank, MAX_RANK_FIELD); PARSER.declareInt(ReciprocalRank::setRelevantRatingThreshhold, RELEVANT_RATING_FIELD); } @@ -158,7 +128,7 @@ public class ReciprocalRank implements RankedListQualityMetric { public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { builder.startObject(); builder.startObject(NAME); - builder.field(MAX_RANK_FIELD.getPreferredName(), this.maxAcceptableRank); + builder.field(RELEVANT_RATING_FIELD.getPreferredName(), this.relevantRatingThreshhold); builder.endObject(); builder.endObject(); return builder; @@ -173,12 +143,12 @@ public class ReciprocalRank implements RankedListQualityMetric { return false; } ReciprocalRank other = (ReciprocalRank) obj; - return Objects.equals(maxAcceptableRank, other.maxAcceptableRank); + return Objects.equals(relevantRatingThreshhold, other.relevantRatingThreshhold); } @Override public final int hashCode() { - return Objects.hash(maxAcceptableRank); + return Objects.hash(relevantRatingThreshhold); } public static class Breakdown implements MetricDetails { diff --git a/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/ReciprocalRankTests.java b/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/ReciprocalRankTests.java index 9220d501c84..3f00562d1e1 100644 --- a/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/ReciprocalRankTests.java +++ b/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/ReciprocalRankTests.java @@ -31,6 +31,7 @@ import org.elasticsearch.test.ESTestCase; import java.io.IOException; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.Vector; @@ -39,16 +40,13 @@ public class ReciprocalRankTests extends ESTestCase { public void testMaxAcceptableRank() { ReciprocalRank reciprocalRank = new ReciprocalRank(); - assertEquals(ReciprocalRank.DEFAULT_MAX_ACCEPTABLE_RANK, reciprocalRank.getMaxAcceptableRank()); - int maxRank = randomIntBetween(1, 100); - reciprocalRank.setMaxAcceptableRank(maxRank); - assertEquals(maxRank, reciprocalRank.getMaxAcceptableRank()); + int searchHits = randomIntBetween(1, 50); - SearchHit[] hits = createSearchHits(0, 9, "test", "type"); + SearchHit[] hits = createSearchHits(0, searchHits, "test", "type"); List ratedDocs = new ArrayList<>(); - int relevantAt = 5; - for (int i = 0; i < 10; i++) { + int relevantAt = randomIntBetween(0, searchHits); + for (int i = 0; i <= searchHits; i++) { if (i == relevantAt) { ratedDocs.add(new RatedDocument("test", "type", Integer.toString(i), Rating.RELEVANT.ordinal())); } else { @@ -58,19 +56,13 @@ public class ReciprocalRankTests extends ESTestCase { int rankAtFirstRelevant = relevantAt + 1; EvalQueryQuality evaluation = reciprocalRank.evaluate("id", hits, ratedDocs); - if (rankAtFirstRelevant <= maxRank) { - assertEquals(1.0 / rankAtFirstRelevant, evaluation.getQualityLevel(), Double.MIN_VALUE); - assertEquals(rankAtFirstRelevant, ((ReciprocalRank.Breakdown) evaluation.getMetricDetails()).getFirstRelevantRank()); + assertEquals(1.0 / rankAtFirstRelevant, evaluation.getQualityLevel(), Double.MIN_VALUE); + assertEquals(rankAtFirstRelevant, ((ReciprocalRank.Breakdown) evaluation.getMetricDetails()).getFirstRelevantRank()); - // check that if we lower maxRank by one, we don't find any result and get 0.0 quality level - reciprocalRank = new ReciprocalRank(rankAtFirstRelevant - 1); - evaluation = reciprocalRank.evaluate("id", hits, ratedDocs); - assertEquals(0.0, evaluation.getQualityLevel(), Double.MIN_VALUE); - - } else { - assertEquals(0.0, evaluation.getQualityLevel(), Double.MIN_VALUE); - assertEquals(-1, ((ReciprocalRank.Breakdown) evaluation.getMetricDetails()).getFirstRelevantRank()); - } + // check that if we have fewer search hits than relevant doc position, we don't find any result and get 0.0 quality level + reciprocalRank = new ReciprocalRank(); + evaluation = reciprocalRank.evaluate("id", Arrays.copyOfRange(hits, 0, relevantAt), ratedDocs); + assertEquals(0.0, evaluation.getQualityLevel(), Double.MIN_VALUE); } public void testEvaluationOneRelevantInResults() { @@ -131,9 +123,8 @@ public class ReciprocalRankTests extends ESTestCase { } public void testXContentRoundtrip() throws IOException { - int position = randomIntBetween(0, 1000); - - ReciprocalRank testItem = new ReciprocalRank(position); + ReciprocalRank testItem = new ReciprocalRank(); + testItem.setRelevantRatingThreshhold(randomIntBetween(0, 20)); XContentParser itemParser = RankEvalTestHelper.roundtrip(testItem); itemParser.nextToken(); itemParser.nextToken();