From a23fc5e8d4e9ef0f737041f6d4f2ebc50b38c40b Mon Sep 17 00:00:00 2001 From: Jon Bratseth Date: Tue, 6 Feb 2018 15:05:05 +0100 Subject: Type check all expressions --- .../com/yahoo/config/model/deploy/DeployState.java | 2 +- .../com/yahoo/searchdefinition/MapTypeContext.java | 39 ++++++++++++++++++++ .../com/yahoo/searchdefinition/RankProfile.java | 23 +++++++----- .../com/yahoo/searchdefinition/SearchBuilder.java | 15 +++----- .../com/yahoo/searchdefinition/TypeMapContext.java | 37 ------------------- .../searchdefinition/processing/Processing.java | 2 +- .../processing/RankingExpressionTypeValidator.java | 36 +++++++++++++------ .../src/test/derived/tensor/rank-profiles.cfg | 2 +- config-model/src/test/derived/tensor/tensor.sd | 2 +- config-model/src/test/examples/rankpropvars.sd | 8 ++--- config-model/src/test/examples/simple.sd | 2 +- .../searchdefinition/RankProfileTestCase.java | 8 ++--- .../RankingExpressionConstantsTestCase.java | 3 ++ .../RankingExpressionShadowingTestCase.java | 40 ++++++++++++++++++--- .../processing/RankProfileSearchFixture.java | 4 +-- .../RankingExpressionTypeValidatorTestCase.java | 42 ++++++++++++++++++++++ .../RankingExpressionWithTensorFlowTestCase.java | 18 +++++----- .../processing/TensorTransformTestCase.java | 13 +++---- 18 files changed, 196 insertions(+), 100 deletions(-) create mode 100644 config-model/src/main/java/com/yahoo/searchdefinition/MapTypeContext.java delete mode 100644 config-model/src/main/java/com/yahoo/searchdefinition/TypeMapContext.java create mode 100644 config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionTypeValidatorTestCase.java (limited to 'config-model/src') diff --git a/config-model/src/main/java/com/yahoo/config/model/deploy/DeployState.java b/config-model/src/main/java/com/yahoo/config/model/deploy/DeployState.java index eb0c6067fca..67724058e64 100644 --- a/config-model/src/main/java/com/yahoo/config/model/deploy/DeployState.java +++ b/config-model/src/main/java/com/yahoo/config/model/deploy/DeployState.java @@ -333,7 +333,7 @@ public class DeployState implements ConfigDefinitionStore { closeIgnoreException(reader.getReader()); } } - builder.build(logger, queryProfiles); + builder.build(logger); return SearchDocumentModel.fromBuilderAndNames(builder, names); } diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/MapTypeContext.java b/config-model/src/main/java/com/yahoo/searchdefinition/MapTypeContext.java new file mode 100644 index 00000000000..e0dc7a2f33c --- /dev/null +++ b/config-model/src/main/java/com/yahoo/searchdefinition/MapTypeContext.java @@ -0,0 +1,39 @@ +// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.searchdefinition; + +import com.yahoo.tensor.TensorType; +import com.yahoo.tensor.evaluation.TypeContext; + +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; + +/** + * A context which only contains type information. + * This returns empty tensor types (double) for unknown features which are not + * query, attribute or constant features, as we do not have information about which such + * features exist (but we know those that exist are doubles). + * + * @author bratseth + */ +public class MapTypeContext implements TypeContext { + + private final Map featureTypes = new HashMap<>(); + + public void setType(String name, TensorType type) { + featureTypes.put(FeatureNames.canonicalize(name), type); + } + + @Override + public TensorType getType(String name) { + if (FeatureNames.isConstantFeature(name) || + FeatureNames.isAttributeFeature(name) || + FeatureNames.isQueryFeature(name)) + return featureTypes.get(FeatureNames.canonicalize(name)); + else + return TensorType.empty; // we do not have type information for these. Correct would be either empty or null + } + + public Map bindings() { return Collections.unmodifiableMap(featureTypes); } + +} diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/RankProfile.java b/config-model/src/main/java/com/yahoo/searchdefinition/RankProfile.java index 238f22eb17c..77dc03ac4d1 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/RankProfile.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/RankProfile.java @@ -578,8 +578,11 @@ public class RankProfile implements Serializable, Cloneable { } /** - * Will take the parser-set textual ranking expressions and turn into objects + * Will take the parser-set textual ranking expressions and turn into ranking expression objects, + * if not already done */ + // TODO: There doesn't appear to be any good reason to defer parsing of ranking expressions + // until this is called. Simplify by parsing them right away. public void parseExpressions() { try { parseRankingExpressions(); @@ -597,20 +600,23 @@ public class RankProfile implements Serializable, Cloneable { for (Map.Entry e : getMacros().entrySet()) { String macroName = e.getKey(); Macro macro = e.getValue(); - RankingExpression expr = parseRankingExpression(macroName, macro.getTextualExpression()); - macro.setRankingExpression(expr); - macro.setTextualExpression(expr.getRoot().toString()); + if (macro.getRankingExpression() == null) { + RankingExpression expr = parseRankingExpression(macroName, macro.getTextualExpression()); + macro.setRankingExpression(expr); + macro.setTextualExpression(expr.getRoot().toString()); + } } } /** * Passes ranking expressions on to parser + * * @throws ParseException if either of the ranking expressions could not be parsed */ private void parseRankingExpressions() throws ParseException { - if (getFirstPhaseRankingString() != null) + if (getFirstPhaseRankingString() != null && firstPhaseRanking == null) setFirstPhaseRanking(parseRankingExpression("firstphase", getFirstPhaseRankingString())); - if (getSecondPhaseRankingString() != null) + if (getSecondPhaseRankingString() != null && secondPhaseRanking == null) setSecondPhaseRanking(parseRankingExpression("secondphase", getSecondPhaseRankingString())); } @@ -741,7 +747,7 @@ public class RankProfile implements Serializable, Cloneable { * referable from this rank profile. */ public TypeContext typeContext(QueryProfileRegistry queryProfiles) { - TypeMapContext context = new TypeMapContext(); + MapTypeContext context = new MapTypeContext(); // Add constants getSearch().getRankingConstants().forEach((k, v) -> context.setType(FeatureNames.asConstantFeature(k), v.getTensorType())); @@ -755,7 +761,8 @@ public class RankProfile implements Serializable, Cloneable { for (QueryProfileType queryProfileType : queryProfiles.getTypeRegistry().allComponents()) { for (FieldDescription field : queryProfileType.declaredFields().values()) { TensorType type = field.getType().asTensorType(); - String feature = FeatureNames.asQueryFeature(field.getName()); + if ( ! FeatureNames.isQueryFeature(field.getName())) continue; + String feature = FeatureNames.canonicalize(field.getName()); TensorType existingType = context.getType(feature); if (existingType != null) type = existingType.dimensionwiseGeneralizationWith(type).orElseThrow( () -> diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/SearchBuilder.java b/config-model/src/main/java/com/yahoo/searchdefinition/SearchBuilder.java index f63e24f65a6..469f29098ad 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/SearchBuilder.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/SearchBuilder.java @@ -195,11 +195,7 @@ public class SearchBuilder { * @throws IllegalStateException Thrown if this method has already been called. */ public void build() { - build(new BaseDeployLogger(), new QueryProfiles()); - } - - public void build(DeployLogger logger) { - build(logger, new QueryProfiles()); + build(new BaseDeployLogger()); } /** @@ -208,9 +204,8 @@ public class SearchBuilder { * * @throws IllegalStateException Thrown if this method has already been called. * @param deployLogger The logger to use during build - * @param queryProfiles The query profiles contained in the application this search is part of. */ - public void build(DeployLogger deployLogger, QueryProfiles queryProfiles) { + public void build(DeployLogger deployLogger) { if (isBuilt) throw new IllegalStateException("Model already built"); List built = new ArrayList<>(); @@ -238,7 +233,7 @@ public class SearchBuilder { for (Search search : new SearchOrderer().order(searchList)) { new FieldOperationApplierForSearch().process(search); // These two needed for a couple of old unit tests, ideally these are just read from app - process(search, deployLogger, queryProfiles); + process(search, deployLogger, new QueryProfiles(queryProfileRegistry)); built.add(search); } builder.addToModel(searchList); @@ -348,7 +343,7 @@ public class SearchBuilder { rankProfileRegistry, queryprofileRegistry); builder.importFile(fileName); - builder.build(deployLogger, new QueryProfiles()); + builder.build(deployLogger); return builder; } @@ -364,7 +359,7 @@ public class SearchBuilder { for (Iterator i = Files.list(new File(dir).toPath()).filter(p -> p.getFileName().toString().endsWith(".sd")).iterator(); i.hasNext(); ) { builder.importFile(i.next()); } - builder.build(new BaseDeployLogger(), new QueryProfiles()); + builder.build(new BaseDeployLogger()); return builder; } diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/TypeMapContext.java b/config-model/src/main/java/com/yahoo/searchdefinition/TypeMapContext.java deleted file mode 100644 index 1e569ac7016..00000000000 --- a/config-model/src/main/java/com/yahoo/searchdefinition/TypeMapContext.java +++ /dev/null @@ -1,37 +0,0 @@ -// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.searchdefinition; - -import com.yahoo.tensor.TensorType; -import com.yahoo.tensor.evaluation.TypeContext; - -import java.util.Collections; -import java.util.HashMap; -import java.util.Map; - -/** - * A context which only contains type information. - * This returns empty tensor types (double) for unknown features which are not - * query, attribute or constant features, as we do not have information about which such - * features exist (but we know those that exist are doubles). - * - * @author bratseth - */ -public class TypeMapContext implements TypeContext { - - private final Map featureTypes = new HashMap<>(); - - public void setType(String name, TensorType type) { - featureTypes.put(FeatureNames.canonicalize(name), type); - } - - @Override - public TensorType getType(String name) { - if (FeatureNames.isConstantFeature(name) || - FeatureNames.isAttributeFeature(name) || - FeatureNames.isQueryFeature(name)) - return featureTypes.get(FeatureNames.canonicalize(name)); - else - return TensorType.empty; // we do not have type information for these. Correct would be either empty or null - } - -} diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/processing/Processing.java b/config-model/src/main/java/com/yahoo/searchdefinition/processing/Processing.java index d9b995fbcd6..061a803cb48 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/processing/Processing.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/processing/Processing.java @@ -76,9 +76,9 @@ public class Processing { ImportedFieldsInSummayValidator::new, FastAccessValidator::new, ReservedMacroNames::new, + RankingExpressionTypeValidator::new, // These should be last. - RankingExpressionTypeValidator::new, IndexingValidation::new, IndexingValues::new); } diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/processing/RankingExpressionTypeValidator.java b/config-model/src/main/java/com/yahoo/searchdefinition/processing/RankingExpressionTypeValidator.java index 6a0dbce362d..494d8d56161 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/processing/RankingExpressionTypeValidator.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/processing/RankingExpressionTypeValidator.java @@ -3,6 +3,7 @@ package com.yahoo.searchdefinition.processing; import com.yahoo.config.application.api.DeployLogger; import com.yahoo.search.query.profile.QueryProfileRegistry; +import com.yahoo.searchdefinition.MapTypeContext; import com.yahoo.searchdefinition.RankProfile; import com.yahoo.searchdefinition.RankProfileRegistry; import com.yahoo.searchdefinition.Search; @@ -30,30 +31,45 @@ public class RankingExpressionTypeValidator extends Processor { validate(profile); } catch (IllegalArgumentException e) { - throw new IllegalArgumentException(search + ", " + profile + " is invalid", e); + throw new IllegalArgumentException("In " + search + ", " + profile, e); } } } /** Throws an IllegalArgumentException if the given rank profile does not produce valid type */ private void validate(RankProfile profile) { + profile.parseExpressions(); + System.out.println("Type checking " + profile + ":"); + System.out.println(" First-phase: " + profile.getFirstPhaseRanking()); TypeContext context = profile.typeContext(queryProfiles); for (RankProfile.Macro macro : profile.getMacros().values()) - if (macro.getRankingExpression() != null) - macro.getRankingExpression().type(context); // type infer to throw on type conflicts - ensureProducesDouble(profile.getFirstPhaseRanking(), "first-phase", context); - ensureProducesDouble(profile.getSecondPhaseRanking(), "second-phase", context); + ensureValid(macro.getRankingExpression(), "macro '" + macro.getName() + "'", context); + ensureValidDouble(profile.getFirstPhaseRanking(), "first-phase expression", context); + ensureValidDouble(profile.getSecondPhaseRanking(), "second-phase expression", context); } - private void ensureProducesDouble(RankingExpression expression, String expressionDescription, TypeContext context) { - if (expression == null) return; + private TensorType ensureValid(RankingExpression expression, String expressionDescription, TypeContext context) { + if (expression == null) return null; - TensorType type = expression.type(context); + TensorType type; + try { + type = expression.type(context); + } + catch (IllegalArgumentException e) { + throw new IllegalArgumentException("The " + expressionDescription + " is invalid", e); + } + System.out.println(" Type of " + expressionDescription + " " + expression.getRoot() + ": " + type); if (type == null) // Not expected to happen throw new IllegalStateException("Could not determine the type produced by " + expressionDescription); + return type; + } + + private void ensureValidDouble(RankingExpression expression, String expressionDescription, TypeContext context) { + if (expression == null) return; + TensorType type = ensureValid(expression, expressionDescription, context); if ( ! type.equals(TensorType.empty)) - throw new IllegalArgumentException(expressionDescription + " ranking expression must produce a double " + - "but produces " + type); + throw new IllegalArgumentException("The " + expressionDescription + " must produce a double " + + "(a tensor with no dimensions), but produces " + type); } } diff --git a/config-model/src/test/derived/tensor/rank-profiles.cfg b/config-model/src/test/derived/tensor/rank-profiles.cfg index 2b231e0cda2..b6ad5372c05 100644 --- a/config-model/src/test/derived/tensor/rank-profiles.cfg +++ b/config-model/src/test/derived/tensor/rank-profiles.cfg @@ -35,7 +35,7 @@ rankprofile[3].name "profile2" rankprofile[3].fef.property[0].name "vespa.rank.firstphase" rankprofile[3].fef.property[0].value "rankingExpression(firstphase)" rankprofile[3].fef.property[1].name "rankingExpression(firstphase).rankingScript" -rankprofile[3].fef.property[1].value "reduce(join(attribute(f4), tensor(x[2],y[2],z[3])((x==y)*(y==z)), f(a,b)(a * b)), sum, x)" +rankprofile[3].fef.property[1].value "reduce(reduce(join(attribute(f4), tensor(x[2],y[2],z[3])((x==y)*(y==z)), f(a,b)(a * b)), sum, x), sum)" rankprofile[3].fef.property[2].name "vespa.type.attribute.f2" rankprofile[3].fef.property[2].value "tensor(x[2],y[])" rankprofile[3].fef.property[3].name "vespa.type.attribute.f3" diff --git a/config-model/src/test/derived/tensor/tensor.sd b/config-model/src/test/derived/tensor/tensor.sd index a6a9a98db3a..3d64f6b807e 100644 --- a/config-model/src/test/derived/tensor/tensor.sd +++ b/config-model/src/test/derived/tensor/tensor.sd @@ -28,7 +28,7 @@ search tensor { rank-profile profile2 { first-phase { - expression: matmul(attribute(f4), diag(x[2],y[2],z[3]), x) + expression: sum(matmul(attribute(f4), diag(x[2],y[2],z[3]), x)) } } diff --git a/config-model/src/test/examples/rankpropvars.sd b/config-model/src/test/examples/rankpropvars.sd index 40f9e73f35a..28959edbc09 100644 --- a/config-model/src/test/examples/rankpropvars.sd +++ b/config-model/src/test/examples/rankpropvars.sd @@ -18,8 +18,8 @@ first-phase { second-phase { expression { if (attribute(artist) == query(testvar1), - 0.0 * fieldMatch(title) + 0.0 * attribute(popularity) + 0.0 * fieldMatch(artist), - 0.0 * attribute(popularity) + 0.0 * fieldMatch(artist) + 0.0 * fieldMatch(title)) + 0.0 * fieldMatch(title) + 0.0 * attribute(Popularity) + 0.0 * fieldMatch(artist), + 0.0 * attribute(Popularity) + 0.0 * fieldMatch(artist) + 0.0 * fieldMatch(title)) } } @@ -42,8 +42,8 @@ first-phase { second-phase { expression { if (attribute(artist) == query(testvar1), - 0.0 * fieldMatch(title) + 0.0 * attribute(popularity) + 0.0 * fieldMatch(artist), - 0.0 * attribute(popularity) + 0.0 * fieldMatch(artist) + 0.0 * fieldMatch(title)) + 0.0 * fieldMatch(title) + 0.0 * attribute(Popularity) + 0.0 * fieldMatch(artist), + 0.0 * attribute(Popularity) + 0.0 * fieldMatch(artist) + 0.0 * fieldMatch(title)) } } } diff --git a/config-model/src/test/examples/simple.sd b/config-model/src/test/examples/simple.sd index 4fda7f5039e..96b0fa98098 100644 --- a/config-model/src/test/examples/simple.sd +++ b/config-model/src/test/examples/simple.sd @@ -116,7 +116,7 @@ search simple { first-phase { keep-rank-count:200 rank-score-drop-limit: -13.0 - expression: attribute(year) + expression: attribute(popularity) } second-phase { rerank-count: 99 diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/RankProfileTestCase.java b/config-model/src/test/java/com/yahoo/searchdefinition/RankProfileTestCase.java index 442c8bd41bd..11093d9f008 100644 --- a/config-model/src/test/java/com/yahoo/searchdefinition/RankProfileTestCase.java +++ b/config-model/src/test/java/com/yahoo/searchdefinition/RankProfileTestCase.java @@ -135,13 +135,13 @@ public class RankProfileTestCase extends SearchDefinitionTestCase { @Test public void requireThatConfigIsDerivedForQueryFeatureTypeSettings() throws ParseException { RankProfileRegistry registry = new RankProfileRegistry(); - SearchBuilder builder = new SearchBuilder(registry); + SearchBuilder builder = new SearchBuilder(registry, setupQueryProfileTypes()); builder.importString("search test {\n" + " document test { } \n" + " rank-profile p1 {}\n" + " rank-profile p2 {}\n" + "}"); - builder.build(new BaseDeployLogger(), setupQueryProfileTypes()); + builder.build(new BaseDeployLogger()); Search search = builder.getSearch(); assertEquals(4, registry.allRankProfiles().size()); @@ -151,7 +151,7 @@ public class RankProfileTestCase extends SearchDefinitionTestCase { assertQueryFeatureTypeSettings(registry.getRankProfile(search, "p2"), search); } - private static QueryProfiles setupQueryProfileTypes() { + private static QueryProfileRegistry setupQueryProfileTypes() { QueryProfileRegistry registry = new QueryProfileRegistry(); QueryProfileTypeRegistry typeRegistry = registry.getTypeRegistry(); QueryProfileType type = new QueryProfileType(new ComponentId("testtype")); @@ -164,7 +164,7 @@ public class RankProfileTestCase extends SearchDefinitionTestCase { type.addField(new FieldDescription("ranking.features.query(numeric)", FieldType.fromString("integer", typeRegistry)), typeRegistry); typeRegistry.register(type); - return new QueryProfiles(registry); + return registry; } private static void assertQueryFeatureTypeSettings(RankProfile profile, Search search) { diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/RankingExpressionConstantsTestCase.java b/config-model/src/test/java/com/yahoo/searchdefinition/RankingExpressionConstantsTestCase.java index e94880e61c7..82b9f5ac043 100644 --- a/config-model/src/test/java/com/yahoo/searchdefinition/RankingExpressionConstantsTestCase.java +++ b/config-model/src/test/java/com/yahoo/searchdefinition/RankingExpressionConstantsTestCase.java @@ -207,6 +207,9 @@ public class RankingExpressionConstantsTestCase extends SearchDefinitionTestCase builder.importString( "search test {\n" + " document test { \n" + + " field rating_yelp type int {" + + " indexing: attribute" + + " }" + " }\n" + " \n" + " rank-profile test {\n" + diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/RankingExpressionShadowingTestCase.java b/config-model/src/test/java/com/yahoo/searchdefinition/RankingExpressionShadowingTestCase.java index 5100ac15c40..ed1b00e2875 100644 --- a/config-model/src/test/java/com/yahoo/searchdefinition/RankingExpressionShadowingTestCase.java +++ b/config-model/src/test/java/com/yahoo/searchdefinition/RankingExpressionShadowingTestCase.java @@ -2,7 +2,10 @@ package com.yahoo.searchdefinition; import com.yahoo.collections.Pair; +import com.yahoo.search.query.profile.QueryProfile; import com.yahoo.search.query.profile.QueryProfileRegistry; +import com.yahoo.search.query.profile.types.FieldDescription; +import com.yahoo.search.query.profile.types.QueryProfileType; import com.yahoo.searchdefinition.derived.AttributeFields; import com.yahoo.searchdefinition.derived.RawRankProfile; import com.yahoo.searchdefinition.parser.ParseException; @@ -149,11 +152,12 @@ public class RankingExpressionShadowingTestCase extends SearchDefinitionTestCase censorBindingHash(testRankProperties.get(4).toString())); } - @Test public void testNeuralNetworkSetup() throws ParseException { + // Note: the type assigned to query profile and constant tensors here is not the correct type RankProfileRegistry rankProfileRegistry = new RankProfileRegistry(); - SearchBuilder builder = new SearchBuilder(rankProfileRegistry); + QueryProfileRegistry queryProfiles = queryProfileWith("query(q)", "tensor(x[])"); + SearchBuilder builder = new SearchBuilder(rankProfileRegistry, queryProfiles); builder.importString( "search test {\n" + " document test { \n" + @@ -176,13 +180,28 @@ public class RankingExpressionShadowingTestCase extends SearchDefinitionTestCase " expression: sum(final_layer)\n" + " }\n" + " }\n" + - "\n" + + " constant W_hidden {\n" + + " type: tensor(x[])\n" + + " file: ignored.json\n" + + " }\n" + + " constant b_input {\n" + + " type: tensor(x[])\n" + + " file: ignored.json\n" + + " }\n" + + " constant W_final {\n" + + " type: tensor(x[])\n" + + " file: ignored.json\n" + + " }\n" + + " constant b_final {\n" + + " type: tensor(x[])\n" + + " file: ignored.json\n" + + " }\n" + "}\n"); builder.build(); Search s = builder.getSearch(); - RankProfile test = rankProfileRegistry.getRankProfile(s, "test").compile(new QueryProfileRegistry()); + RankProfile test = rankProfileRegistry.getRankProfile(s, "test").compile(queryProfiles); List> testRankProperties = new RawRankProfile(test, - new QueryProfileRegistry(), + queryProfiles, new AttributeFields(s)).configProperties(); assertEquals("(rankingExpression(relu).rankingScript,max(1.0,x))", testRankProperties.get(0).toString()); @@ -198,6 +217,17 @@ public class RankingExpressionShadowingTestCase extends SearchDefinitionTestCase testRankProperties.get(5).toString()); } + private QueryProfileRegistry queryProfileWith(String field, String type) { + QueryProfileType queryProfileType = new QueryProfileType("root"); + queryProfileType.addField(new FieldDescription(field, type)); + QueryProfileRegistry queryProfileRegistry = new QueryProfileRegistry(); + queryProfileRegistry.getTypeRegistry().register(queryProfileType); + QueryProfile profile = new QueryProfile("default"); + profile.setType(queryProfileType); + queryProfileRegistry.register(profile); + return queryProfileRegistry; + } + private String censorBindingHash(String s) { StringBuilder b = new StringBuilder(); boolean areInHash = false; diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankProfileSearchFixture.java b/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankProfileSearchFixture.java index 800697b3430..0ce6129ef7f 100644 --- a/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankProfileSearchFixture.java +++ b/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankProfileSearchFixture.java @@ -38,7 +38,8 @@ class RankProfileSearchFixture { RankProfileSearchFixture(ApplicationPackage applicationpackage, QueryProfileRegistry queryProfileRegistry, String rankProfiles, String constant, String field) throws ParseException { - SearchBuilder builder = new SearchBuilder(applicationpackage, rankProfileRegistry, new QueryProfileRegistry()); + this.queryProfileRegistry = queryProfileRegistry; + SearchBuilder builder = new SearchBuilder(applicationpackage, rankProfileRegistry, queryProfileRegistry); String sdContent = "search test {\n" + " " + (constant != null ? constant : "") + "\n" + " document test {\n" + @@ -50,7 +51,6 @@ class RankProfileSearchFixture { builder.importString(sdContent); builder.build(); search = builder.getSearch(); - this.queryProfileRegistry = queryProfileRegistry; } public void assertFirstPhaseExpression(String expExpression, String rankProfile) { diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionTypeValidatorTestCase.java b/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionTypeValidatorTestCase.java new file mode 100644 index 00000000000..5c654f09c51 --- /dev/null +++ b/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionTypeValidatorTestCase.java @@ -0,0 +1,42 @@ +// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.searchdefinition.processing; + +import com.yahoo.searchdefinition.RankProfileRegistry; +import com.yahoo.searchdefinition.SearchBuilder; +import com.yahoo.yolean.Exceptions; +import org.junit.Test; +import static com.yahoo.config.model.test.TestUtil.joinLines; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.fail; + +public class RankingExpressionTypeValidatorTestCase { + + @Test + public void tensorTypeValidation() throws Exception { + try { + RankProfileRegistry rankProfileRegistry = new RankProfileRegistry(); + SearchBuilder searchBuilder = new SearchBuilder(rankProfileRegistry); + searchBuilder.importString(joinLines( + "search test {", + " document test { ", + " field a type tensor(x[],y[]) {", + " indexing: attribute", + " }", + " }", + " rank-profile my_rank_profile {", + " first-phase {", + " expression: attribute(a)", + " }", + " }", + "}" + )); + searchBuilder.build(); + fail("Expected exception"); + } + catch (IllegalArgumentException expected) { + assertEquals("In search definition 'test', rank profile 'my_rank_profile': The first-phase expression must produce a double (a tensor with no dimensions), but produces tensor(x[],y[])", + Exceptions.toMessageString(expected)); + } + } + +} diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionWithTensorFlowTestCase.java b/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionWithTensorFlowTestCase.java index 5203e686681..464772fc10d 100644 --- a/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionWithTensorFlowTestCase.java +++ b/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionWithTensorFlowTestCase.java @@ -51,7 +51,7 @@ public class RankingExpressionWithTensorFlowTestCase { } @Test - public void testTensorFlowReference() throws ParseException { + public void testTensorFlowReference() { RankProfileSearchFixture search = fixtureWith("tensor(d0[2],d1[784])(0.0)", "tensorflow('mnist_softmax/saved')"); search.assertFirstPhaseExpression(vespaExpression, "my_profile"); @@ -60,7 +60,7 @@ public class RankingExpressionWithTensorFlowTestCase { } @Test - public void testTensorFlowReferenceWithConstantFeature() throws ParseException { + public void testTensorFlowReferenceWithConstantFeature() { RankProfileSearchFixture search = fixtureWith("constant(mytensor)", "tensorflow('mnist_softmax/saved')", "constant mytensor { file: ignored\ntype: tensor(d0[7],d1[784]) }", @@ -71,10 +71,10 @@ public class RankingExpressionWithTensorFlowTestCase { } @Test - public void testTensorFlowReferenceWithQueryFeature() throws ParseException { + public void testTensorFlowReferenceWithQueryFeature() { String queryProfile = ""; String queryProfileType = "" + - " " + + " " + ""; StoringApplicationPackage application = new StoringApplicationPackage(applicationDir, queryProfile, @@ -90,7 +90,7 @@ public class RankingExpressionWithTensorFlowTestCase { } @Test - public void testTensorFlowReferenceWithDocumentFeature() throws ParseException { + public void testTensorFlowReferenceWithDocumentFeature() { StoringApplicationPackage application = new StoringApplicationPackage(applicationDir); RankProfileSearchFixture search = fixtureWith("attribute(mytensor)", "tensorflow('mnist_softmax/saved')", @@ -103,10 +103,10 @@ public class RankingExpressionWithTensorFlowTestCase { } @Test - public void testTensorFlowReferenceWithFeatureCombination() throws ParseException { + public void testTensorFlowReferenceWithFeatureCombination() { String queryProfile = ""; String queryProfileType = "" + - " " + + " " + ""; StoringApplicationPackage application = new StoringApplicationPackage(applicationDir, queryProfile, @@ -122,7 +122,7 @@ public class RankingExpressionWithTensorFlowTestCase { } @Test - public void testNestedTensorFlowReference() throws ParseException { + public void testNestedTensorFlowReference() { RankProfileSearchFixture search = fixtureWith("tensor(d0[2],d1[784])(0.0)", "5 + sum(tensorflow('mnist_softmax/saved'))"); search.assertFirstPhaseExpression("5 + reduce(" + vespaExpression + ", sum)", "my_profile"); @@ -131,7 +131,7 @@ public class RankingExpressionWithTensorFlowTestCase { } @Test - public void testTensorFlowReferenceSpecifyingSignature() throws ParseException { + public void testTensorFlowReferenceSpecifyingSignature() { RankProfileSearchFixture search = fixtureWith("tensor(d0[2],d1[784])(0.0)", "tensorflow('mnist_softmax/saved', 'serving_default')"); search.assertFirstPhaseExpression(vespaExpression, "my_profile"); diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/processing/TensorTransformTestCase.java b/config-model/src/test/java/com/yahoo/searchdefinition/processing/TensorTransformTestCase.java index c18cfcfe1aa..c1d987ef3ad 100644 --- a/config-model/src/test/java/com/yahoo/searchdefinition/processing/TensorTransformTestCase.java +++ b/config-model/src/test/java/com/yahoo/searchdefinition/processing/TensorTransformTestCase.java @@ -108,7 +108,8 @@ public class TensorTransformTestCase extends SearchDefinitionTestCase { private List> buildSearch(String expression) throws ParseException { RankProfileRegistry rankProfileRegistry = new RankProfileRegistry(); - SearchBuilder builder = new SearchBuilder(rankProfileRegistry); + QueryProfileRegistry queryProfiles = setupQueryProfileTypes(); + SearchBuilder builder = new SearchBuilder(rankProfileRegistry, queryProfiles); builder.importString( "search test {\n" + " document test { \n" + @@ -167,16 +168,16 @@ public class TensorTransformTestCase extends SearchDefinitionTestCase { " }\n" + " }\n" + "}\n"); - builder.build(new BaseDeployLogger(), setupQueryProfileTypes()); + builder.build(new BaseDeployLogger()); Search s = builder.getSearch(); - RankProfile test = rankProfileRegistry.getRankProfile(s, "test").compile(new QueryProfileRegistry()); + RankProfile test = rankProfileRegistry.getRankProfile(s, "test").compile(queryProfiles); List> testRankProperties = new RawRankProfile(test, - new QueryProfileRegistry(), + queryProfiles, new AttributeFields(s)).configProperties(); return testRankProperties; } - private static QueryProfiles setupQueryProfileTypes() { + private static QueryProfileRegistry setupQueryProfileTypes() { QueryProfileRegistry registry = new QueryProfileRegistry(); QueryProfileTypeRegistry typeRegistry = registry.getTypeRegistry(); QueryProfileType type = new QueryProfileType(new ComponentId("testtype")); @@ -185,7 +186,7 @@ public class TensorTransformTestCase extends SearchDefinitionTestCase { type.addField(new FieldDescription("ranking.features.query(n)", FieldType.fromString("integer", typeRegistry)), typeRegistry); typeRegistry.register(type); - return new QueryProfiles(registry); + return registry; } private String censorBindingHash(String s) { -- cgit v1.2.3