diff options
author | Jon Bratseth <bratseth@gmail.com> | 2022-10-19 16:09:54 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-10-19 16:09:54 +0200 |
commit | dfb918506e63fee9c7c41949bc5fa10b7d98bc93 (patch) | |
tree | f23bdaea5364767503a527982b366b2b0a6e47e5 | |
parent | 71922a7b3329fc90cd0f1a51568680758c7dc72a (diff) | |
parent | 8aada7b3ce5f66204d36f1fb88f1d8873d0f292d (diff) |
Merge pull request #24506 from vespa-engine/bratseth/inputs
More input tests and better error message
8 files changed, 107 insertions, 18 deletions
diff --git a/config-model/src/main/java/com/yahoo/schema/RankProfile.java b/config-model/src/main/java/com/yahoo/schema/RankProfile.java index 31c68d2f2c8..cc1c87e6f47 100644 --- a/config-model/src/main/java/com/yahoo/schema/RankProfile.java +++ b/config-model/src/main/java/com/yahoo/schema/RankProfile.java @@ -783,12 +783,13 @@ public class RankProfile implements Cloneable { // Combine Map<Reference, Input> allInputs = new LinkedHashMap<>(); + for (var inheritedProfile : inherited()) { for (var input : inheritedProfile.inputs().entrySet()) { Input existing = allInputs.get(input.getKey()); if (existing != null && ! existing.equals(input.getValue())) throw new IllegalArgumentException(this + " inherits " + inheritedProfile + " which contains " + - input.getValue() + ", but this input is already defined as " + + input.getValue() + ", but this is already defined as " + existing + " in another profile this inherits"); allInputs.put(input.getKey(), input.getValue()); } @@ -1403,8 +1404,7 @@ public class RankProfile implements Cloneable { @Override public boolean equals(Object o) { if (o == this) return true; - if ( ! (o instanceof Input)) return false; - Input other = (Input)o; + if ( ! (o instanceof Input other)) return false; if ( ! other.name().equals(this.name())) return false; if ( ! other.type().equals(this.type())) return false; if ( ! other.defaultValue().equals(this.defaultValue())) return false; @@ -1418,8 +1418,8 @@ public class RankProfile implements Cloneable { @Override public String toString() { - return "input '" + name + "' " + type + - (defaultValue().isPresent() ? ":" + defaultValue.get().toAbbreviatedString() : ""); + return "input " + name + " " + type + + (defaultValue().isPresent() ? ":" + defaultValue.get().toAbbreviatedString(false, true) : ""); } } diff --git a/config-model/src/test/java/com/yahoo/schema/RankProfileTestCase.java b/config-model/src/test/java/com/yahoo/schema/RankProfileTestCase.java index 51d19283b1e..85225f0d255 100644 --- a/config-model/src/test/java/com/yahoo/schema/RankProfileTestCase.java +++ b/config-model/src/test/java/com/yahoo/schema/RankProfileTestCase.java @@ -24,11 +24,13 @@ import ai.vespa.rankingexpression.importer.configmodelview.ImportedMlModels; import static com.yahoo.config.model.test.TestUtil.joinLines; +import com.yahoo.searchlib.rankingexpression.Reference; import org.junit.jupiter.api.Test; import java.util.Iterator; import java.util.List; import java.util.Optional; +import java.util.Set; import static org.junit.jupiter.api.Assertions.*; @@ -168,6 +170,74 @@ public class RankProfileTestCase extends AbstractSchemaTestCase { } @Test + void inputsAreInheritedAndValuesAreOverridable() throws ParseException { + RankProfileRegistry registry = new RankProfileRegistry(); + ApplicationBuilder builder = new ApplicationBuilder(registry, new QueryProfileRegistry()); + builder.addSchema(joinLines( + "schema test {", + " document test { } ", + " rank-profile parent1 {", + " inputs {", + " input1 double: 1", + " input2 double: 2", + " }", + " }", + " rank-profile parent2 {", + " inputs {", + " input4 double: 4", + " }", + " }", + " rank-profile child inherits parent1, parent2 {", + " inputs {", + " input2 double: 2.5", + " input3 double: 3", + " }" + + " }", + "}")); + var application = builder.build(true); + RankProfile child = registry.get(application.schemas().get("test"), "child"); + assertEquals(4, child.inputs().size()); + assertEquals(Set.of(Reference.simple("query", "input1"), + Reference.simple("query", "input2"), + Reference.simple("query", "input3"), + Reference.simple("query", "input4")), + child.inputs().keySet()); + var input2 = child.inputs().get(Reference.simple("query", "input2")); + assertEquals(2.5, input2.defaultValue().get().asDouble(), 0.000000001); + } + + @Test + void inputConflictsAreDetected() throws ParseException { + try { + RankProfileRegistry registry = new RankProfileRegistry(); + ApplicationBuilder builder = new ApplicationBuilder(registry, new QueryProfileRegistry()); + builder.addSchema(joinLines( + "schema test {", + " document test { } ", + " rank-profile parent1 {", + " inputs {", + " input1 double: 1", + " }", + " }", + " rank-profile parent2 {", + " inputs {", + " input1 tensor(x[100])", + " }", + " }", + " rank-profile child inherits parent1, parent2 {", + " }", + "}")); + builder.build(true); + fail("Should have failed"); + } + catch (IllegalArgumentException e) { + assertEquals("rank profile 'child' inherits rank profile 'parent2' which contains input query(input1) tensor(x[100])" + + ", but this is already defined as input query(input1) tensor():{1.0} in another profile this inherits", + e.getCause().getMessage()); + } + } + + @Test void requireThatDefaultInheritingDefaultIsIgnored() throws ParseException { RankProfileRegistry registry = new RankProfileRegistry(); ApplicationBuilder builder = new ApplicationBuilder(registry, setupQueryProfileTypes()); diff --git a/vespajlib/abi-spec.json b/vespajlib/abi-spec.json index 8714285acd8..5d491dcbd4a 100644 --- a/vespajlib/abi-spec.json +++ b/vespajlib/abi-spec.json @@ -906,7 +906,7 @@ "public com.yahoo.tensor.Tensor remove(java.util.Set)", "public java.lang.String toString()", "public java.lang.String toString(boolean, boolean)", - "public java.lang.String toAbbreviatedString()", + "public java.lang.String toAbbreviatedString(boolean, boolean)", "public boolean equals(java.lang.Object)", "public bridge synthetic com.yahoo.tensor.Tensor withType(com.yahoo.tensor.TensorType)" ], @@ -958,7 +958,7 @@ "public int hashCode()", "public java.lang.String toString()", "public java.lang.String toString(boolean, boolean)", - "public java.lang.String toAbbreviatedString()", + "public java.lang.String toAbbreviatedString(boolean, boolean)", "public boolean equals(java.lang.Object)" ], "fields": [] @@ -1051,7 +1051,7 @@ "public int hashCode()", "public java.lang.String toString()", "public java.lang.String toString(boolean, boolean)", - "public java.lang.String toAbbreviatedString()", + "public java.lang.String toAbbreviatedString(boolean, boolean)", "public boolean equals(java.lang.Object)", "public long denseSubspaceSize()", "public static com.yahoo.tensor.TensorType createPartialType(com.yahoo.tensor.TensorType$Value, java.util.List)" @@ -1240,7 +1240,8 @@ "public java.util.List smallest()", "public abstract java.lang.String toString()", "public abstract java.lang.String toString(boolean, boolean)", - "public abstract java.lang.String toAbbreviatedString()", + "public java.lang.String toAbbreviatedString()", + "public abstract java.lang.String toAbbreviatedString(boolean, boolean)", "public static java.lang.String toStandardString(com.yahoo.tensor.Tensor, boolean, boolean, long)", "public static java.lang.String valueToString(com.yahoo.tensor.Tensor, boolean, long)", "public abstract boolean equals(java.lang.Object)", diff --git a/vespajlib/src/main/java/com/yahoo/tensor/IndexedTensor.java b/vespajlib/src/main/java/com/yahoo/tensor/IndexedTensor.java index c4316eb334a..50809ab3ff6 100644 --- a/vespajlib/src/main/java/com/yahoo/tensor/IndexedTensor.java +++ b/vespajlib/src/main/java/com/yahoo/tensor/IndexedTensor.java @@ -219,7 +219,9 @@ public abstract class IndexedTensor implements Tensor { } @Override - public String toString() { return toString(true, true); } + public String toString() { + return toString(true, true); + } @Override public String toString(boolean withType, boolean shortForms) { @@ -227,8 +229,8 @@ public abstract class IndexedTensor implements Tensor { } @Override - public String toAbbreviatedString() { - return toString(true, true, Math.max(2, 10 / (type().dimensions().stream().filter(d -> d.isMapped()).count() + 1))); + public String toAbbreviatedString(boolean withType, boolean shortForms) { + return toString(withType, shortForms, Math.max(2, 10 / (type().dimensions().stream().filter(d -> d.isMapped()).count() + 1))); } private String toString(boolean withType, boolean shortForms, long maxCells) { diff --git a/vespajlib/src/main/java/com/yahoo/tensor/MappedTensor.java b/vespajlib/src/main/java/com/yahoo/tensor/MappedTensor.java index 946d8fe0f4a..3ea128ffa9f 100644 --- a/vespajlib/src/main/java/com/yahoo/tensor/MappedTensor.java +++ b/vespajlib/src/main/java/com/yahoo/tensor/MappedTensor.java @@ -78,8 +78,8 @@ public class MappedTensor implements Tensor { public String toString(boolean withType, boolean shortForms) { return toString(withType, shortForms, Long.MAX_VALUE); } @Override - public String toAbbreviatedString() { - return toString(true, true, Math.max(2, 10 / (type().dimensions().stream().filter(d -> d.isMapped()).count() + 1))); + public String toAbbreviatedString(boolean withType, boolean shortForms) { + return toString(withType, shortForms, Math.max(2, 10 / (type().dimensions().stream().filter(d -> d.isMapped()).count() + 1))); } private String toString(boolean withType, boolean shortForms, long maxCells) { diff --git a/vespajlib/src/main/java/com/yahoo/tensor/MixedTensor.java b/vespajlib/src/main/java/com/yahoo/tensor/MixedTensor.java index d2fed9b96f9..e7690876434 100644 --- a/vespajlib/src/main/java/com/yahoo/tensor/MixedTensor.java +++ b/vespajlib/src/main/java/com/yahoo/tensor/MixedTensor.java @@ -154,8 +154,8 @@ public class MixedTensor implements Tensor { } @Override - public String toAbbreviatedString() { - return toString(true, true, Math.max(2, 10 / (type().dimensions().stream().filter(d -> d.isMapped()).count() + 1))); + public String toAbbreviatedString(boolean withType, boolean shortForms) { + return toString(withType, shortForms, Math.max(2, 10 / (type().dimensions().stream().filter(d -> d.isMapped()).count() + 1))); } private String toString(boolean withType, boolean shortForms, long maxCells) { diff --git a/vespajlib/src/main/java/com/yahoo/tensor/Tensor.java b/vespajlib/src/main/java/com/yahoo/tensor/Tensor.java index 349214ee7f9..2ad3212c424 100644 --- a/vespajlib/src/main/java/com/yahoo/tensor/Tensor.java +++ b/vespajlib/src/main/java/com/yahoo/tensor/Tensor.java @@ -325,7 +325,17 @@ public interface Tensor { String toString(boolean withType, boolean shortForms); /** Returns an abbreviated string representation of this tensor suitable for human-readable messages */ - String toAbbreviatedString(); + default String toAbbreviatedString() { + return toAbbreviatedString(true, true); + } + + /** + * Returns an abbreviated string representation of this tensor suitable for human-readable messages + * + * @param withType whether to prefix the value by the type of this + * @param shortForms whether to use short forms where applicable, or always using the verbose form + */ + String toAbbreviatedString(boolean withType, boolean shortForms); /** * Call this from toString in implementations to return this tensor on the diff --git a/vespajlib/src/test/java/com/yahoo/tensor/TensorTestCase.java b/vespajlib/src/test/java/com/yahoo/tensor/TensorTestCase.java index 920f8512c53..428c8a83b47 100644 --- a/vespajlib/src/test/java/com/yahoo/tensor/TensorTestCase.java +++ b/vespajlib/src/test/java/com/yahoo/tensor/TensorTestCase.java @@ -63,9 +63,11 @@ public class TensorTestCase { } @Test - public void testToShortString() { + public void testToAbbreviatedString() { assertEquals("tensor(x[10]):[0.0, 1.0, 2.0, 3.0, 4.0, 5.0, 6.0, 7.0, 8.0, 9.0]", Tensor.from("tensor(x[10]):[0, 1, 2, 3, 4, 5, 6, 7, 8, 9]").toAbbreviatedString()); + assertEquals("[0.0, 1.0, 2.0, 3.0, 4.0, 5.0, 6.0, 7.0, 8.0, 9.0]", + Tensor.from("tensor(x[10]):[0, 1, 2, 3, 4, 5, 6, 7, 8, 9]").toAbbreviatedString(false, true)); assertEquals("tensor(x[14]):[0.0, 1.0, 2.0, 3.0, 4.0, 5.0, 6.0, 7.0, 8.0, 9.0, ...]", Tensor.from("tensor(x[14]):[0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13]").toAbbreviatedString()); assertEquals("tensor(d1{},d2{}):{{d1:l1,d2:l1}:6.0, {d1:l1,d2:l2}:6.0, {d1:l1,d2:l3}:6.0, ...}", @@ -83,6 +85,10 @@ public class TensorTestCase { " {m:k3,n:k1,x:0}:0, {m:k3,n:k1,x:1}:1, {m:k3,n:k1,x:2}:2}").toAbbreviatedString()); assertEquals("tensor(m{},x[2],y[2]):{k1:[[0.0, 1.0], [2.0, 3.0]], k2:[[0.0, ...}", Tensor.from("tensor(m{},x[2],y[2]):{k1:[[0,1],[2,3]], k2:[[0,1],[2,3]], k3:[[0,1],[2,3]]}").toAbbreviatedString()); + assertEquals("{k1:[[0.0, 1.0], [2.0, 3.0]], k2:[[0.0, ...}", + Tensor.from("tensor(m{},x[2],y[2]):{k1:[[0,1],[2,3]], k2:[[0,1],[2,3]], k3:[[0,1],[2,3]]}").toAbbreviatedString(false, true)); + assertEquals("{{m:k1,x:0,y:0}:0.0, {m:k1,x:0,y:1}:1.0, {m:k1,x:1,y:0}:2.0, {m:k1,x:1,y:1}:3.0, {m:k2,x:0,y:0}:0.0, ...}", + Tensor.from("tensor(m{},x[2],y[2]):{k1:[[0,1],[2,3]], k2:[[0,1],[2,3]], k3:[[0,1],[2,3]]}").toAbbreviatedString(false, false)); } @Test |