diff options
author | Jon Bratseth <bratseth@gmail.com> | 2022-05-13 12:14:50 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-05-13 12:14:50 +0200 |
commit | 79537b954556ba6985f2c59b4bc44b971a385eb0 (patch) | |
tree | 810f8b8d2d070ac7e21704ea00ae3b20b62e5f2c /config-model | |
parent | 1df3b3c59251bd4fd1b099ae5cfb4c280313e76d (diff) | |
parent | 64b4e9c4236bbf924230dcc4750094da30496b63 (diff) |
Merge pull request #22589 from vespa-engine/revert-22582-bratseth/constants-cleanup-take-2
Revert "Bratseth/constants cleanup take 2"
Diffstat (limited to 'config-model')
36 files changed, 446 insertions, 466 deletions
diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/DefaultRankProfile.java b/config-model/src/main/java/com/yahoo/searchdefinition/DefaultRankProfile.java index 69850b21224..812726a609e 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/DefaultRankProfile.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/DefaultRankProfile.java @@ -1,10 +1,10 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.searchdefinition; -import com.yahoo.searchdefinition.derived.FileDistributedConstants; import com.yahoo.searchdefinition.document.ImmutableSDField; import java.util.LinkedHashSet; +import java.util.List; import java.util.Set; /** @@ -21,8 +21,8 @@ public class DefaultRankProfile extends RankProfile { * @param rankProfileRegistry the {@link com.yahoo.searchdefinition.RankProfileRegistry} * to use for storing and looking up rank profiles */ - public DefaultRankProfile(Schema schema, RankProfileRegistry rankProfileRegistry) { - super("default", schema, rankProfileRegistry); + public DefaultRankProfile(Schema schema, RankProfileRegistry rankProfileRegistry, RankingConstants rankingConstants) { + super("default", schema, rankProfileRegistry, rankingConstants); } /** Ignore self inheriting of default as some applications may use that for historical reasons. */ diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/DistributableResource.java b/config-model/src/main/java/com/yahoo/searchdefinition/DistributableResource.java index be2aa500b2b..11c55521100 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/DistributableResource.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/DistributableResource.java @@ -14,7 +14,7 @@ public class DistributableResource implements Comparable <DistributableResource> /** The search definition-unique name of this constant */ private final String name; - // TODO: Make path/pathType final + //TODO Make path/pathType final private PathType pathType; private String path; private FileReference fileReference = new FileReference(""); @@ -35,14 +35,14 @@ public class DistributableResource implements Comparable <DistributableResource> this.pathType = type; } - // TODO: Remove and make path/pathType final + //TODO Remove and make path/pathType final public void setFileName(String fileName) { Objects.requireNonNull(fileName, "Filename cannot be null"); this.path = fileName; this.pathType = PathType.FILE; } - // TODO: Remove and make path/pathType final + //TODO Remove and make path/pathType final public void setUri(String uri) { Objects.requireNonNull(uri, "uri cannot be null"); this.path = uri; @@ -65,7 +65,7 @@ public class DistributableResource implements Comparable <DistributableResource> } } - public void register(FileRegistry fileRegistry) { + void register(FileRegistry fileRegistry) { switch (pathType) { case FILE: fileReference = fileRegistry.addFile(path); @@ -91,5 +91,4 @@ public class DistributableResource implements Comparable <DistributableResource> public int compareTo(DistributableResource o) { return name.compareTo(o.getName()); } - } diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/DocumentsOnlyRankProfile.java b/config-model/src/main/java/com/yahoo/searchdefinition/DocumentsOnlyRankProfile.java index caaef63fd73..acab2b96772 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/DocumentsOnlyRankProfile.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/DocumentsOnlyRankProfile.java @@ -14,8 +14,9 @@ import java.util.List; */ public class DocumentsOnlyRankProfile extends RankProfile { - public DocumentsOnlyRankProfile(String name, Schema schema, RankProfileRegistry rankProfileRegistry) { - super(name, schema, rankProfileRegistry); + public DocumentsOnlyRankProfile(String name, Schema schema, RankProfileRegistry rankProfileRegistry, + RankingConstants rankingConstants) { + super(name, schema, rankProfileRegistry, rankingConstants); } @Override diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/ImmutableSchema.java b/config-model/src/main/java/com/yahoo/searchdefinition/ImmutableSchema.java index 1c643292a05..b2f46f4a309 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/ImmutableSchema.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/ImmutableSchema.java @@ -7,7 +7,6 @@ import com.yahoo.config.model.api.ModelContext; import com.yahoo.searchdefinition.document.ImmutableSDField; import com.yahoo.searchdefinition.document.SDDocumentType; import com.yahoo.searchdefinition.document.SDField; -import com.yahoo.searchlib.rankingexpression.Reference; import com.yahoo.vespa.documentmodel.SummaryField; import java.io.Reader; @@ -35,7 +34,7 @@ public interface ImmutableSchema { ApplicationPackage applicationPackage(); DeployLogger getDeployLogger(); ModelContext.Properties getDeployProperties(); - Map<Reference, RankProfile.Constant> constants(); + RankingConstants rankingConstants(); LargeRankExpressions rankExpressionFiles(); OnnxModels onnxModels(); Stream<ImmutableSDField> allImportedFields(); diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/RankExpressionBody.java b/config-model/src/main/java/com/yahoo/searchdefinition/RankExpressionBody.java index 90109026dce..6ba17123fb4 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/RankExpressionBody.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/RankExpressionBody.java @@ -4,6 +4,7 @@ package com.yahoo.searchdefinition; import com.yahoo.config.application.api.FileRegistry; import java.nio.ByteBuffer; +import java.util.Objects; import static java.util.Objects.requireNonNull; @@ -25,7 +26,7 @@ public class RankExpressionBody extends DistributableResource { } } - public void register(FileRegistry fileRegistry) { + void register(FileRegistry fileRegistry) { register(fileRegistry, blob); } 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 95e022fca9e..3abf4ec3596 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/RankProfile.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/RankProfile.java @@ -130,6 +130,7 @@ public class RankProfile implements Cloneable { /** Global onnx models not tied to a schema */ private final OnnxModels onnxModels; + private final RankingConstants rankingConstants; private final ApplicationPackage applicationPackage; private final DeployLogger deployLogger; @@ -141,10 +142,11 @@ public class RankProfile implements Cloneable { * @param rankProfileRegistry the {@link com.yahoo.searchdefinition.RankProfileRegistry} to use for storing * and looking up rank profiles. */ - public RankProfile(String name, Schema schema, RankProfileRegistry rankProfileRegistry) { + public RankProfile(String name, Schema schema, RankProfileRegistry rankProfileRegistry, RankingConstants rankingConstants) { this.name = Objects.requireNonNull(name, "name cannot be null"); this.schema = Objects.requireNonNull(schema, "schema cannot be null"); this.onnxModels = null; + this.rankingConstants = rankingConstants; this.rankProfileRegistry = rankProfileRegistry; this.applicationPackage = schema.applicationPackage(); this.deployLogger = schema.getDeployLogger(); @@ -156,10 +158,11 @@ public class RankProfile implements Cloneable { * @param name the name of the new profile */ public RankProfile(String name, ApplicationPackage applicationPackage, DeployLogger deployLogger, - RankProfileRegistry rankProfileRegistry, OnnxModels onnxModels) { + RankProfileRegistry rankProfileRegistry, RankingConstants rankingConstants, OnnxModels onnxModels) { this.name = Objects.requireNonNull(name, "name cannot be null"); this.schema = null; this.rankProfileRegistry = rankProfileRegistry; + this.rankingConstants = rankingConstants; this.onnxModels = onnxModels; this.applicationPackage = applicationPackage; this.deployLogger = deployLogger; @@ -175,6 +178,11 @@ public class RankProfile implements Cloneable { return applicationPackage; } + /** Returns the ranking constants of the owner of this */ + public RankingConstants rankingConstants() { + return rankingConstants; + } + public Map<String, OnnxModel> onnxModels() { return schema != null ? schema.onnxModels().asMap() : onnxModels.asMap(); } @@ -407,25 +415,24 @@ public class RankProfile implements Cloneable { return finalSettings; } - public void add(Constant constant) { - constants.put(constant.name(), constant); + public void addConstant(Reference name, Constant value) { + constants.put(name, value); } /** Returns an unmodifiable view of the constants available in this */ - public Map<Reference, Constant> constants() { + public Map<Reference, Constant> getConstants() { + if (inherited().isEmpty()) return new HashMap<>(constants); + Map<Reference, Constant> allConstants = new HashMap<>(); for (var inheritedProfile : inherited()) { - for (var constant : inheritedProfile.constants().values()) { - if (allConstants.containsKey(constant.name())) - throw new IllegalArgumentException(constant + "' is present in " + + for (var constant : inheritedProfile.getConstants().entrySet()) { + if (allConstants.containsKey(constant.getKey())) + throw new IllegalArgumentException("Constant '" + constant.getKey() + "' is present in " + inheritedProfile + " inherited by " + this + ", but is also present in another profile inherited by it"); - allConstants.put(constant.name(), constant); + allConstants.put(constant.getKey(), constant.getValue()); } } - - if (schema != null) - allConstants.putAll(schema.constants()); allConstants.putAll(constants); return allConstants; } @@ -935,7 +942,7 @@ public class RankProfile implements Cloneable { } private void compileThis(QueryProfileRegistry queryProfiles, ImportedMlModels importedModels) { - checkNameCollisions(getFunctions(), constants()); + checkNameCollisions(getFunctions(), getConstants()); ExpressionTransforms expressionTransforms = new ExpressionTransforms(); Map<Reference, TensorType> featureTypes = featureTypes(); @@ -943,8 +950,8 @@ public class RankProfile implements Cloneable { Map<String, RankingExpressionFunction> inlineFunctions = compileFunctions(this::getInlineFunctions, queryProfiles, featureTypes, importedModels, Collections.emptyMap(), expressionTransforms); - firstPhaseRanking = compile(this.getFirstPhase(), queryProfiles, featureTypes, importedModels, constants(), inlineFunctions, expressionTransforms); - secondPhaseRanking = compile(this.getSecondPhase(), queryProfiles, featureTypes, importedModels, constants(), inlineFunctions, expressionTransforms); + firstPhaseRanking = compile(this.getFirstPhase(), queryProfiles, featureTypes, importedModels, getConstants(), inlineFunctions, expressionTransforms); + secondPhaseRanking = compile(this.getSecondPhase(), queryProfiles, featureTypes, importedModels, getConstants(), inlineFunctions, expressionTransforms); // Function compiling second pass: compile all functions and insert previously compiled inline functions // TODO: This merges all functions from inherited profiles too and erases inheritance information. Not good. @@ -979,7 +986,7 @@ public class RankProfile implements Cloneable { while (null != (entry = findUncompiledFunction(functions.get(), compiledFunctions.keySet()))) { RankingExpressionFunction rankingExpressionFunction = entry.getValue(); RankingExpressionFunction compiled = compile(rankingExpressionFunction, queryProfiles, featureTypes, - importedModels, constants(), inlineFunctions, + importedModels, getConstants(), inlineFunctions, expressionTransforms); compiledFunctions.put(entry.getKey(), compiled); } @@ -1039,7 +1046,9 @@ public class RankProfile implements Cloneable { Map<Reference, TensorType> featureTypes) { MapEvaluationTypeContext context = new MapEvaluationTypeContext(getExpressionFunctions(), featureTypes); - constants().forEach((k, v) -> context.setType(k, v.type())); + // Add small and large constants, respectively + getConstants().forEach((k, v) -> context.setType(k, v.type())); + rankingConstants().asMap().forEach((k, v) -> context.setType(FeatureNames.asConstantFeature(k), v.getTensorType())); // Add query features from all rank profile types for (QueryProfileType queryProfileType : queryProfiles.getTypeRegistry().allComponents()) { @@ -1418,32 +1427,23 @@ public class RankProfile implements Cloneable { private final Optional<Tensor> value; private final Optional<String> valuePath; - // Always set only if valuePath is set - private final Optional<DistributableResource.PathType> pathType; - public Constant(Reference name, Tensor value) { - this(name, value.type(), Optional.of(value), Optional.empty(), Optional.empty()); + this(name, value.type(), Optional.of(value), Optional.empty()); } public Constant(Reference name, TensorType type, String valuePath) { - this(name, type, Optional.empty(), Optional.of(valuePath), Optional.of(DistributableResource.PathType.FILE)); - } - - public Constant(Reference name, TensorType type, String valuePath, DistributableResource.PathType pathType) { - this(name, type, Optional.empty(), Optional.of(valuePath), Optional.of(pathType)); + this(name, type, Optional.empty(), Optional.of(valuePath)); } - private Constant(Reference name, TensorType type, Optional<Tensor> value, - Optional<String> valuePath, Optional<DistributableResource.PathType> pathType) { - this.name = Objects.requireNonNull(name); - this.type = Objects.requireNonNull(type); - this.value = Objects.requireNonNull(value); - this.valuePath = Objects.requireNonNull(valuePath); - this.pathType = Objects.requireNonNull(pathType); - + private Constant(Reference name, TensorType type, Optional<Tensor> value, Optional<String> valuePath) { if (type.dimensions().stream().anyMatch(d -> d.isIndexed() && d.size().isEmpty())) throw new IllegalArgumentException("Illegal type of constant " + name + " type " + type + ": Dense tensor dimensions must have a size"); + + this.name = name; + this.type = type; + this.value = value; + this.valuePath = valuePath; } public Reference name() { return name; } @@ -1455,9 +1455,6 @@ public class RankProfile implements Cloneable { /** Returns the path to the value of this, if its value is empty. */ public Optional<String> valuePath() { return valuePath; } - /** Returns the path type, if valuePath is set. */ - public Optional<DistributableResource.PathType> pathType() { return pathType; } - @Override public boolean equals(Object o) { if (o == this) return true; @@ -1467,19 +1464,18 @@ public class RankProfile implements Cloneable { if ( ! other.type().equals(this.type())) return false; if ( ! other.value().equals(this.value())) return false; if ( ! other.valuePath().equals(this.valuePath())) return false; - if ( ! other.pathType().equals(this.pathType())) return false; return true; } @Override public int hashCode() { - return Objects.hash(name, type, value, valuePath, pathType); + return Objects.hash(name, type, value, valuePath); } @Override public String toString() { - return "constant '" + name + "' " + type + ":" + - (value().isPresent() ? value.get().toAbbreviatedString() : " file:" + valuePath.get()); + return "constant '" + name + "' " + type + + (value().isPresent() ? ":" + value.get().toAbbreviatedString() : "file:" + valuePath.get()); } } diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/RankProfileRegistry.java b/config-model/src/main/java/com/yahoo/searchdefinition/RankProfileRegistry.java index 2e815b7b503..bb2dc04cd2a 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/RankProfileRegistry.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/RankProfileRegistry.java @@ -31,8 +31,8 @@ public class RankProfileRegistry { public static RankProfileRegistry createRankProfileRegistryWithBuiltinRankProfiles(Schema schema) { RankProfileRegistry rankProfileRegistry = new RankProfileRegistry(); - rankProfileRegistry.add(new DefaultRankProfile(schema, rankProfileRegistry)); - rankProfileRegistry.add(new UnrankedRankProfile(schema, rankProfileRegistry)); + rankProfileRegistry.add(new DefaultRankProfile(schema, rankProfileRegistry, schema.rankingConstants())); + rankProfileRegistry.add(new UnrankedRankProfile(schema, rankProfileRegistry, schema.rankingConstants())); return rankProfileRegistry; } diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/RankingConstant.java b/config-model/src/main/java/com/yahoo/searchdefinition/RankingConstant.java new file mode 100644 index 00000000000..7a2765de852 --- /dev/null +++ b/config-model/src/main/java/com/yahoo/searchdefinition/RankingConstant.java @@ -0,0 +1,52 @@ +// Copyright Yahoo. 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; + +/** + * A global ranking constant distributed using file distribution. + * Ranking constants must be sent to some services to be useful - this is done + * by calling the sentTo method during the prepare phase of building models. + * + * @author arnej + * @author bratseth + */ +public class RankingConstant extends DistributableResource { + + private TensorType tensorType = null; + + public RankingConstant(String name) { + super(name); + } + + public RankingConstant(String name, TensorType type, String fileName) { + this(name, type, fileName, PathType.FILE); + } + public RankingConstant(String name, TensorType type, String fileName, PathType pathType) { + super(name, fileName, pathType); + this.tensorType = type; + validate(); + } + + public void setType(TensorType type) { + this.tensorType = type; + } + + public TensorType getTensorType() { return tensorType; } + public String getType() { return tensorType.toString(); } + + public void validate() { + super.validate(); + if (tensorType == null) + throw new IllegalArgumentException("Ranking constant '" + getName() + "' must have a type."); + if (tensorType.dimensions().stream().anyMatch(d -> d.isIndexed() && d.size().isEmpty())) + throw new IllegalArgumentException("Illegal type in field " + getName() + " type " + tensorType + + ": Dense tensor dimensions must have a size"); + } + + @Override + public String toString() { + return super.toString() + "' of type '" + tensorType + "'"; + } + +} diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/RankingConstants.java b/config-model/src/main/java/com/yahoo/searchdefinition/RankingConstants.java new file mode 100644 index 00000000000..e10da716335 --- /dev/null +++ b/config-model/src/main/java/com/yahoo/searchdefinition/RankingConstants.java @@ -0,0 +1,90 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.searchdefinition; + +import com.yahoo.config.application.api.FileRegistry; + +import java.util.Collections; +import java.util.HashMap; +import java.util.LinkedHashMap; +import java.util.Map; +import java.util.Optional; +import java.util.concurrent.ConcurrentHashMap; +import java.util.function.Function; + +/** + * Constant values for ranking/model execution tied to a schema, or globally to an application package + * + * @author bratseth + */ +public class RankingConstants { + + private final FileRegistry fileRegistry; + + /** The schema this belongs to, or empty if it is global */ + private final Optional<Schema> owner; + + private final Map<String, RankingConstant> constants = new LinkedHashMap<>(); + + private final Object mutex = new Object(); + + public RankingConstants(FileRegistry fileRegistry, Optional<Schema> owner) { + this.fileRegistry = fileRegistry; + this.owner = owner; + } + + public void add(RankingConstant constant) { + synchronized (mutex) { + constant.validate(); + constant.register(fileRegistry); + String name = constant.getName(); + RankingConstant prev = constants.putIfAbsent(name, constant); + if (prev != null) + throw new IllegalArgumentException("Constant '" + name + "' defined twice"); + } + } + + public void putIfAbsent(RankingConstant constant) { + synchronized (mutex) { + constant.validate(); + constant.register(fileRegistry); + String name = constant.getName(); + constants.putIfAbsent(name, constant); + } + } + + public void computeIfAbsent(String name, Function<? super String, ? extends RankingConstant> createConstant) { + synchronized (mutex) { + constants.computeIfAbsent(name, key -> { + RankingConstant constant = createConstant.apply(key); + constant.validate(); + constant.register(fileRegistry); + return constant; + }); + } + } + + /** Returns the ranking constant with the given name, or null if not present */ + public RankingConstant get(String name) { + synchronized (mutex) { + var constant = constants.get(name); + if (constant != null) return constant; + if (owner.isPresent() && owner.get().inherited().isPresent()) + return owner.get().inherited().get().rankingConstants().get(name); + return null; + } + } + + /** Returns a read-only map of the ranking constants in this indexed by name */ + public Map<String, RankingConstant> asMap() { + synchronized (mutex) { + // Shortcuts + if (owner.isEmpty() || owner.get().inherited().isEmpty()) return Collections.unmodifiableMap(constants); + if (constants.isEmpty()) return owner.get().inherited().get().rankingConstants().asMap(); + + var allConstants = new LinkedHashMap<>(owner.get().inherited().get().rankingConstants().asMap()); + allConstants.putAll(constants); + return Collections.unmodifiableMap(allConstants); + } + } + +} diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/Schema.java b/config-model/src/main/java/com/yahoo/searchdefinition/Schema.java index db105edc9d4..203a109acd5 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/Schema.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/Schema.java @@ -19,7 +19,6 @@ import com.yahoo.searchdefinition.document.SDField; import com.yahoo.searchdefinition.document.Stemming; import com.yahoo.searchdefinition.document.TemporaryImportedFields; import com.yahoo.searchdefinition.document.annotation.SDAnnotationType; -import com.yahoo.searchlib.rankingexpression.Reference; import com.yahoo.vespa.documentmodel.DocumentSummary; import com.yahoo.vespa.documentmodel.SummaryField; @@ -86,9 +85,7 @@ public class Schema implements ImmutableSchema { /** External rank expression files of this */ private final LargeRankExpressions largeRankExpressions; - /** Constants that will be available in all rank profiles. */ - // TODO: Remove on Vespa 9: Should always be in a rank profile - private final Map<Reference, RankProfile.Constant> constants = new LinkedHashMap<>(); + private final RankingConstants rankingConstants; private final OnnxModels onnxModels; @@ -150,6 +147,7 @@ public class Schema implements ImmutableSchema { this.properties = properties; this.documentsOnly = documentsOnly; largeRankExpressions = new LargeRankExpressions(fileRegistry); + rankingConstants = new RankingConstants(fileRegistry, Optional.of(this)); onnxModels = new OnnxModels(fileRegistry, Optional.of(this)); } @@ -226,19 +224,8 @@ public class Schema implements ImmutableSchema { @Override public LargeRankExpressions rankExpressionFiles() { return largeRankExpressions; } - public void add(RankProfile.Constant constant) { - constants.put(constant.name(), constant); - } - @Override - public Map<Reference, RankProfile.Constant> constants() { - if (inherited().isEmpty()) return Collections.unmodifiableMap(constants); - if (constants.isEmpty()) return inherited().get().constants(); - - Map<Reference, RankProfile.Constant> allConstants = new LinkedHashMap<>(inherited().get().constants()); - allConstants.putAll(constants); - return allConstants; - } + public RankingConstants rankingConstants() { return rankingConstants; } @Override public OnnxModels onnxModels() { return onnxModels; } diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/UnrankedRankProfile.java b/config-model/src/main/java/com/yahoo/searchdefinition/UnrankedRankProfile.java index 0e4578878df..acf034362ca 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/UnrankedRankProfile.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/UnrankedRankProfile.java @@ -1,7 +1,6 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.searchdefinition; -import com.yahoo.searchdefinition.derived.FileDistributedConstants; import com.yahoo.searchlib.rankingexpression.RankingExpression; import com.yahoo.searchlib.rankingexpression.parser.ParseException; @@ -12,8 +11,8 @@ import com.yahoo.searchlib.rankingexpression.parser.ParseException; */ public class UnrankedRankProfile extends RankProfile { - public UnrankedRankProfile(Schema schema, RankProfileRegistry rankProfileRegistry) { - super("unranked", schema, rankProfileRegistry); + public UnrankedRankProfile(Schema schema, RankProfileRegistry rankProfileRegistry, RankingConstants rankingConstants) { + super("unranked", schema, rankProfileRegistry, rankingConstants); try { RankingExpression exp = new RankingExpression("value(0)"); this.setFirstPhaseRanking(exp); diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/derived/DerivedConfiguration.java b/config-model/src/main/java/com/yahoo/searchdefinition/derived/DerivedConfiguration.java index 50029613b2e..8dec0e8339a 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/derived/DerivedConfiguration.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/derived/DerivedConfiguration.java @@ -12,7 +12,6 @@ import com.yahoo.searchdefinition.RankProfileRegistry; import com.yahoo.searchdefinition.Schema; import com.yahoo.searchdefinition.derived.validation.Validation; import com.yahoo.vespa.config.search.AttributesConfig; -import com.yahoo.vespa.config.search.core.RankingConstantsConfig; import com.yahoo.vespa.model.container.search.QueryProfiles; import java.io.IOException; @@ -82,7 +81,7 @@ public class DerivedConfiguration implements AttributesConfig.Producer { summaries = new Summaries(schema, deployState.getDeployLogger(), deployState.getProperties().featureFlags()); summaryMap = new SummaryMap(schema); juniperrc = new Juniperrc(schema); - rankProfileList = new RankProfileList(schema, schema.constants(), schema.rankExpressionFiles(), + rankProfileList = new RankProfileList(schema, schema.rankingConstants(), schema.rankExpressionFiles(), schema.onnxModels(), attributeFields, deployState); indexingScript = new IndexingScript(schema); indexInfo = new IndexInfo(schema); @@ -128,12 +127,6 @@ public class DerivedConfiguration implements AttributesConfig.Producer { exportCfg(new QueryProfiles(queryProfileRegistry, (level, message) -> {}).getConfig(), toDirectory + "/" + "query-profiles.cfg"); } - public void exportConstants(String toDirectory) throws IOException { - RankingConstantsConfig.Builder b = new RankingConstantsConfig.Builder(); - rankProfileList.getConfig(b); - exportCfg(b.build(), toDirectory + "/" + "ranking-constants.cfg"); - } - private static void exportCfg(ConfigInstance instance, String fileName) throws IOException { Writer writer = null; try { diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/derived/FileDistributedConstants.java b/config-model/src/main/java/com/yahoo/searchdefinition/derived/FileDistributedConstants.java deleted file mode 100644 index c9b0ed2f628..00000000000 --- a/config-model/src/main/java/com/yahoo/searchdefinition/derived/FileDistributedConstants.java +++ /dev/null @@ -1,106 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.searchdefinition.derived; - -import com.yahoo.config.application.api.FileRegistry; -import com.yahoo.searchdefinition.DistributableResource; -import com.yahoo.searchdefinition.RankProfile; -import com.yahoo.tensor.TensorType; - -import java.util.Collection; -import java.util.Collections; -import java.util.LinkedHashMap; -import java.util.Map; -import java.util.function.Function; - -/** - * Constant values for ranking/model execution tied to a rank profile, - * to be distributed as files. - * - * @author bratseth - */ -public class FileDistributedConstants { - - private final FileRegistry fileRegistry; - - private final Map<String, DistributableConstant> constants = new LinkedHashMap<>(); - - public FileDistributedConstants(FileRegistry fileRegistry, Collection<RankProfile.Constant> constants) { - this.fileRegistry = fileRegistry; - for (var constant : constants) { - if (constant.valuePath().isPresent()) - add(new DistributableConstant(constant.name().simpleArgument().get(), - constant.type(), - constant.valuePath().get(), - constant.pathType().get())); - } - } - - public void add(DistributableConstant constant) { - constant.validate(); - constant.register(fileRegistry); - String name = constant.getName(); - DistributableConstant prev = constants.putIfAbsent(name, constant); - if ( prev != null ) - throw new IllegalArgumentException("Constant '" + name + "' defined twice"); - } - - public void putIfAbsent(DistributableConstant constant) { - constant.validate(); - constant.register(fileRegistry); - String name = constant.getName(); - constants.putIfAbsent(name, constant); - } - - public void computeIfAbsent(String name, Function<? super String, ? extends DistributableConstant> createConstant) { - constants.computeIfAbsent(name, key -> { - DistributableConstant constant = createConstant.apply(key); - constant.validate(); - constant.register(fileRegistry); - return constant; - }); - } - - /** Returns a read-only map of the constants in this indexed by name. */ - public Map<String, DistributableConstant> asMap() { - return Collections.unmodifiableMap(constants); - } - - public static class DistributableConstant extends DistributableResource { - - private TensorType tensorType; - - public DistributableConstant(String name, TensorType type, String fileName) { - this(name, type, fileName, PathType.FILE); - } - - public DistributableConstant(String name, TensorType type, String fileName, PathType pathType) { - super(name, fileName, pathType); - this.tensorType = type; - validate(); - } - - public void setType(TensorType type) { - this.tensorType = type; - } - - public TensorType getTensorType() { return tensorType; } - public String getType() { return tensorType.toString(); } - - public void validate() { - super.validate(); - if (tensorType == null) - throw new IllegalArgumentException("Ranking constant '" + getName() + "' must have a type."); - if (tensorType.dimensions().stream().anyMatch(d -> d.isIndexed() && d.size().isEmpty())) - throw new IllegalArgumentException("Illegal type in field " + getName() + " type " + tensorType + - ": Dense tensor dimensions must have a size"); - } - - @Override - public String toString() { - return super.toString() + "' of type '" + tensorType + "'"; - } - - } - -} - diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/derived/RankProfileList.java b/config-model/src/main/java/com/yahoo/searchdefinition/derived/RankProfileList.java index 770ca358ea4..a55c97f4ebb 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/derived/RankProfileList.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/derived/RankProfileList.java @@ -9,7 +9,8 @@ import com.yahoo.searchdefinition.OnnxModel; import com.yahoo.searchdefinition.OnnxModels; import com.yahoo.searchdefinition.LargeRankExpressions; import com.yahoo.searchdefinition.RankProfileRegistry; -import com.yahoo.searchlib.rankingexpression.Reference; +import com.yahoo.searchdefinition.RankingConstant; +import com.yahoo.searchdefinition.RankingConstants; import com.yahoo.vespa.config.search.RankProfilesConfig; import com.yahoo.searchdefinition.RankProfile; import com.yahoo.searchdefinition.Schema; @@ -18,8 +19,6 @@ import com.yahoo.vespa.config.search.core.RankingConstantsConfig; import com.yahoo.vespa.config.search.core.RankingExpressionsConfig; import java.util.ArrayList; -import java.util.Collection; -import java.util.HashMap; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -39,14 +38,14 @@ public class RankProfileList extends Derived implements RankProfilesConfig.Produ private static final Logger log = Logger.getLogger(RankProfileList.class.getName()); private final Map<String, RawRankProfile> rankProfiles = new java.util.LinkedHashMap<>(); - private final FileDistributedConstants constants; + private final RankingConstants rankingConstants; private final LargeRankExpressions largeRankExpressions; private final OnnxModels onnxModels; public static final RankProfileList empty = new RankProfileList(); private RankProfileList() { - constants = new FileDistributedConstants(null, List.of()); + rankingConstants = new RankingConstants(null, Optional.empty()); largeRankExpressions = new LargeRankExpressions(null); onnxModels = new OnnxModels(null, Optional.empty()); } @@ -58,45 +57,18 @@ public class RankProfileList extends Derived implements RankProfilesConfig.Produ * @param attributeFields the attribute fields to create a ranking for */ public RankProfileList(Schema schema, - Map<Reference, RankProfile.Constant> constantsFromSchema, + RankingConstants rankingConstants, LargeRankExpressions largeRankExpressions, OnnxModels onnxModels, AttributeFields attributeFields, DeployState deployState) { setName(schema == null ? "default" : schema.getName()); - this.constants = deriveFileDistributedConstants(schema, constantsFromSchema, deployState); + this.rankingConstants = rankingConstants; this.largeRankExpressions = largeRankExpressions; this.onnxModels = onnxModels; // as ONNX models come from parsing rank expressions deriveRankProfiles(schema, attributeFields, deployState); } - private static FileDistributedConstants deriveFileDistributedConstants(Schema schema, - Map<Reference, RankProfile.Constant> constantsFromSchema, - DeployState deployState) { - Map<Reference, RankProfile.Constant> allFileConstants = new HashMap<>(); - addFileConstants(constantsFromSchema.values(), allFileConstants, schema != null ? schema.toString() : "[global]"); - for (var profile : deployState.rankProfileRegistry().rankProfilesOf(schema)) - addFileConstants(profile.constants().values(), allFileConstants, profile.toString()); - for (var profile : deployState.rankProfileRegistry().rankProfilesOf(null)) - addFileConstants(profile.constants().values(), allFileConstants, profile.toString()); - return new FileDistributedConstants(deployState.getFileRegistry(), allFileConstants.values()); - } - - private static void addFileConstants(Collection<RankProfile.Constant> source, - Map<Reference, RankProfile.Constant> destination, - String sourceName) { - for (var constant : source) { - if (constant.valuePath().isEmpty()) continue; - var existing = destination.get(constant.name()); - if ( existing != null && ! constant.equals(existing)) - throw new IllegalArgumentException("Duplicate " + constant + " in " + sourceName + - ": Value reference constants must be unique across all rank profiles"); - destination.put(constant.name(), constant); - } - } - - public FileDistributedConstants constants() { return constants; } - private boolean areDependenciesReady(RankProfile rank, RankProfileRegistry registry) { return rank.inheritedNames().isEmpty() || rankProfiles.keySet().containsAll(rank.inheritedNames()) || @@ -106,7 +78,7 @@ public class RankProfileList extends Derived implements RankProfilesConfig.Produ private void deriveRankProfiles(Schema schema, AttributeFields attributeFields, DeployState deployState) { - if (schema != null) { // profiles belonging to a schema have a default profile + if (schema != null) { // profiles belonging to a search have a default profile RawRankProfile rawRank = new RawRankProfile(deployState.rankProfileRegistry().get(schema, "default"), largeRankExpressions, deployState.getQueryProfiles().getRegistry(), @@ -134,7 +106,6 @@ public class RankProfileList extends Derived implements RankProfilesConfig.Produ ready.forEach(rank -> remaining.remove(rank.name())); } } - private void processRankProfiles(List<RankProfile> ready, QueryProfileRegistry queryProfiles, ImportedMlModels importedModels, @@ -189,7 +160,7 @@ public class RankProfileList extends Derived implements RankProfilesConfig.Produ } public void getConfig(RankingConstantsConfig.Builder builder) { - for (var constant : constants.asMap().values()) { + for (RankingConstant constant : rankingConstants.asMap().values()) { if ("".equals(constant.getFileReference())) log.warning("Illegal file reference " + constant); // Let tests pass ... we should find a better way else diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/parser/ConvertParsedRanking.java b/config-model/src/main/java/com/yahoo/searchdefinition/parser/ConvertParsedRanking.java index f772c5fe903..b04bfca3f76 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/parser/ConvertParsedRanking.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/parser/ConvertParsedRanking.java @@ -5,6 +5,8 @@ import com.yahoo.searchdefinition.RankProfile; import com.yahoo.searchdefinition.RankProfileRegistry; import com.yahoo.searchdefinition.Schema; import com.yahoo.searchdefinition.document.RankType; +import com.yahoo.searchlib.rankingexpression.evaluation.TensorValue; +import com.yahoo.searchlib.rankingexpression.evaluation.Value; import java.util.List; @@ -30,7 +32,7 @@ public class ConvertParsedRanking { if (name.equals("default")) { return rankProfileRegistry.get(schema, "default"); } - return new RankProfile(name, schema, rankProfileRegistry); + return new RankProfile(name, schema, rankProfileRegistry, schema.rankingConstants()); } void convertRankProfile(Schema schema, ParsedRankProfile parsed) { @@ -40,8 +42,8 @@ public class ConvertParsedRanking { parsed.isStrict().ifPresent(value -> profile.setStrict(value)); - for (var constant : parsed.getConstants().values()) - profile.add(constant); + for (var constant : parsed.getConstants().entrySet()) + profile.addConstant(constant.getKey(), constant.getValue()); for (var input : parsed.getInputs().entrySet()) profile.addInput(input.getKey(), input.getValue()); diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/parser/ConvertParsedSchemas.java b/config-model/src/main/java/com/yahoo/searchdefinition/parser/ConvertParsedSchemas.java index e56245d1332..d3c24fa9924 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/parser/ConvertParsedSchemas.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/parser/ConvertParsedSchemas.java @@ -28,6 +28,7 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.logging.Level; /** * Class converting a collection of schemas from the intermediate format. @@ -207,14 +208,14 @@ public class ConvertParsedSchemas { if (documentsOnly) { return; // skip ranking-only content, not used for document type generation } - for (var constant : parsed.getConstants()) { - schema.add(constant); + for (var rankingConstant : parsed.getRankingConstants()) { + schema.rankingConstants().add(rankingConstant); } for (var onnxModel : parsed.getOnnxModels()) { schema.onnxModels().add(onnxModel); } - rankProfileRegistry.add(new DefaultRankProfile(schema, rankProfileRegistry)); - rankProfileRegistry.add(new UnrankedRankProfile(schema, rankProfileRegistry)); + rankProfileRegistry.add(new DefaultRankProfile(schema, rankProfileRegistry, schema.rankingConstants())); + rankProfileRegistry.add(new UnrankedRankProfile(schema, rankProfileRegistry, schema.rankingConstants())); var rankConverter = new ConvertParsedRanking(rankProfileRegistry); for (var rankProfile : parsed.getRankProfiles()) { rankConverter.convertRankProfile(schema, rankProfile); diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/parser/ParsedRankProfile.java b/config-model/src/main/java/com/yahoo/searchdefinition/parser/ParsedRankProfile.java index 0ade3bfd76b..bead0215b7d 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/parser/ParsedRankProfile.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/parser/ParsedRankProfile.java @@ -101,9 +101,9 @@ class ParsedRankProfile extends ParsedBlock { this.inheritedSummaryFeatures = other; } - void add(RankProfile.Constant constant) { - verifyThat(! constants.containsKey(constant.name()), "already has constant", constant.name()); - constants.put(constant.name(), constant); + void addConstant(Reference name, RankProfile.Constant value) { + verifyThat(! constants.containsKey(name), "already has constant", name); + constants.put(name, value); } void addInput(Reference name, RankProfile.Input input) { diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/parser/ParsedSchema.java b/config-model/src/main/java/com/yahoo/searchdefinition/parser/ParsedSchema.java index 2bc10554b25..e4f312e98e3 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/parser/ParsedSchema.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/parser/ParsedSchema.java @@ -2,9 +2,8 @@ package com.yahoo.searchdefinition.parser; import com.yahoo.searchdefinition.OnnxModel; -import com.yahoo.searchdefinition.RankProfile; +import com.yahoo.searchdefinition.RankingConstant; import com.yahoo.searchdefinition.document.Stemming; -import com.yahoo.searchlib.rankingexpression.Reference; import java.util.ArrayList; import java.util.LinkedHashMap; @@ -17,7 +16,7 @@ import java.util.Optional; * one schema (.sd) file, using simple data structures * as far as possible. * - * Do not put complicated logic here! + * Do not put advanced logic here! * * @author arnej27959 */ @@ -40,7 +39,7 @@ public class ParsedSchema extends ParsedBlock { private Stemming defaultStemming = null; private final List<ImportedField> importedFields = new ArrayList<>(); private final List<OnnxModel> onnxModels = new ArrayList<>(); - private final Map<Reference, RankProfile.Constant> constants = new LinkedHashMap<>(); + private final List<RankingConstant> rankingConstants = new ArrayList<>(); private final List<String> inherited = new ArrayList<>(); private final List<String> inheritedByDocument = new ArrayList<>(); private final Map<String, ParsedSchema> resolvedInherits = new LinkedHashMap<>(); @@ -71,12 +70,12 @@ public class ParsedSchema extends ParsedBlock { List<ParsedFieldSet> getFieldSets() { return List.copyOf(fieldSets.values()); } List<ParsedIndex> getIndexes() { return List.copyOf(extraIndexes.values()); } List<ParsedStruct> getStructs() { return List.copyOf(extraStructs.values()); } + List<RankingConstant> getRankingConstants() { return List.copyOf(rankingConstants); } List<String> getInherited() { return List.copyOf(inherited); } List<String> getInheritedByDocument() { return List.copyOf(inheritedByDocument); } List<ParsedRankProfile> getRankProfiles() { return List.copyOf(rankProfiles.values()); } List<ParsedSchema> getResolvedInherits() { return List.copyOf(resolvedInherits.values()); } List<ParsedSchema> getAllResolvedInherits() { return List.copyOf(allResolvedInherits.values()); } - List<RankProfile.Constant> getConstants() { return List.copyOf(constants.values()); } void addAnnotation(ParsedAnnotation annotation) { String annName = annotation.name(); @@ -133,8 +132,8 @@ public class ParsedSchema extends ParsedBlock { rankProfiles.put(rpName, profile); } - void add(RankProfile.Constant constant) { - constants.put(constant.name(), constant); + void addRankingConstant(RankingConstant constant) { + rankingConstants.add(constant); } void addStruct(ParsedStruct struct) { diff --git a/config-model/src/main/java/com/yahoo/vespa/model/VespaModel.java b/config-model/src/main/java/com/yahoo/vespa/model/VespaModel.java index f79f3d9c82c..6c04ebbc31e 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/VespaModel.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/VespaModel.java @@ -35,7 +35,7 @@ import com.yahoo.searchdefinition.OnnxModel; import com.yahoo.searchdefinition.OnnxModels; import com.yahoo.searchdefinition.RankProfile; import com.yahoo.searchdefinition.RankProfileRegistry; -import com.yahoo.searchdefinition.derived.FileDistributedConstants; +import com.yahoo.searchdefinition.RankingConstants; import com.yahoo.searchdefinition.derived.AttributeFields; import com.yahoo.searchdefinition.derived.RankProfileList; import com.yahoo.searchdefinition.document.SDField; @@ -125,6 +125,9 @@ public final class VespaModel extends AbstractConfigProducerRoot implements Seri /** The global rank profiles of this model */ private final RankProfileList rankProfileList; + /** The global ranking constants of this model */ + private final RankingConstants rankingConstants; + /** The validation overrides of this. This is never null. */ private final ValidationOverrides validationOverrides; @@ -170,15 +173,16 @@ public final class VespaModel extends AbstractConfigProducerRoot implements Seri version = deployState.getVespaVersion(); wantedNodeVersion = deployState.getWantedNodeVespaVersion(); fileReferencesRepository = new FileReferencesRepository(deployState.getFileRegistry()); + rankingConstants = new RankingConstants(deployState.getFileRegistry(), Optional.empty()); validationOverrides = deployState.validationOverrides(); applicationPackage = deployState.getApplicationPackage(); provisioned = deployState.provisioned(); VespaModelBuilder builder = new VespaDomBuilder(); root = builder.getRoot(VespaModel.ROOT_CONFIGID, deployState, this); - createGlobalRankProfiles(deployState, deployState.getFileRegistry()); + createGlobalRankProfiles(deployState, rankingConstants, deployState.getFileRegistry()); rankProfileList = new RankProfileList(null, // null search -> global - Map.of(), + rankingConstants, new LargeRankExpressions(deployState.getFileRegistry()), new OnnxModels(deployState.getFileRegistry(), Optional.empty()), AttributeFields.empty, @@ -251,6 +255,9 @@ public final class VespaModel extends AbstractConfigProducerRoot implements Seri /** Returns the application package owning this */ public ApplicationPackage applicationPackage() { return applicationPackage; } + /** Returns the global ranking constants of this */ + public RankingConstants rankingConstants() { return rankingConstants; } + /** Creates a mutable model with no services instantiated */ public static VespaModel createIncomplete(DeployState deployState) throws IOException, SAXException { return new VespaModel(new NullConfigModelRegistry(), deployState, false); @@ -275,7 +282,7 @@ public final class VespaModel extends AbstractConfigProducerRoot implements Seri * Creates a rank profile not attached to any search definition, for each imported model in the application package, * and adds it to the given rank profile registry. */ - private void createGlobalRankProfiles(DeployState deployState, FileRegistry fileRegistry) { + private void createGlobalRankProfiles(DeployState deployState, RankingConstants rankingConstants, FileRegistry fileRegistry) { var importedModels = deployState.getImportedModels().all(); DeployLogger deployLogger = deployState.getDeployLogger(); RankProfileRegistry rankProfileRegistry = deployState.rankProfileRegistry(); @@ -285,11 +292,11 @@ public final class VespaModel extends AbstractConfigProducerRoot implements Seri for (ImportedMlModel model : importedModels) { // Due to automatic naming not guaranteeing unique names, there must be a 1-1 between OnnxModels and global RankProfiles. OnnxModels onnxModels = onnxModelInfoFromSource(model, fileRegistry); - RankProfile profile = new RankProfile(model.name(), applicationPackage, deployLogger, rankProfileRegistry, onnxModels); + RankProfile profile = new RankProfile(model.name(), applicationPackage, deployLogger, rankProfileRegistry, rankingConstants, onnxModels); rankProfileRegistry.add(profile); futureModels.add(deployState.getExecutor().submit(() -> { ConvertedModel convertedModel = ConvertedModel.fromSource(applicationPackage, new ModelName(model.name()), - model.name(), profile, queryProfiles.getRegistry(), model); + model.name(), profile, queryProfiles.getRegistry(), model); convertedModel.expressions().values().forEach(f -> profile.addFunction(f, false)); return convertedModel; })); @@ -302,7 +309,7 @@ public final class VespaModel extends AbstractConfigProducerRoot implements Seri if (modelName.contains(".")) continue; // Name space: Not a global profile // Due to automatic naming not guaranteeing unique names, there must be a 1-1 between OnnxModels and global RankProfiles. OnnxModels onnxModels = onnxModelInfoFromStore(modelName, fileRegistry); - RankProfile profile = new RankProfile(modelName, applicationPackage, deployLogger, rankProfileRegistry, onnxModels); + RankProfile profile = new RankProfile(modelName, applicationPackage, deployLogger, rankProfileRegistry, rankingConstants, onnxModels); rankProfileRegistry.add(profile); futureModels.add(deployState.getExecutor().submit(() -> { ConvertedModel convertedModel = ConvertedModel.fromStore(applicationPackage, new ModelName(modelName), modelName, profile); diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/ConstantValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/ConstantValidator.java deleted file mode 100644 index a0fb7078b13..00000000000 --- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/ConstantValidator.java +++ /dev/null @@ -1,83 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.model.application.validation; - -import com.yahoo.config.application.api.ApplicationFile; -import com.yahoo.config.application.api.ApplicationPackage; -import com.yahoo.config.model.application.provider.FilesApplicationPackage; -import com.yahoo.config.model.deploy.DeployState; -import com.yahoo.path.Path; -import com.yahoo.searchdefinition.DistributableResource; -import com.yahoo.searchdefinition.RankProfile; -import com.yahoo.searchdefinition.Schema; -import com.yahoo.vespa.model.VespaModel; -import com.yahoo.vespa.model.application.validation.ConstantTensorJsonValidator.InvalidConstantTensorException; - -import java.io.FileNotFoundException; - -/** - * RankingConstantsValidator validates all constant tensors (ranking constants) bundled with an application package - * - * @author Vegard Sjonfjell - */ -public class ConstantValidator extends Validator { - - @Override - public void validate(VespaModel model, DeployState deployState) { - var exceptionMessageCollector = new ExceptionMessageCollector("Invalid constant tensor file(s):"); - for (Schema schema : deployState.getSchemas()) { - for (var constant : schema.constants().values()) - validate(constant, deployState.getApplicationPackage(), exceptionMessageCollector); - for (var profile : deployState.rankProfileRegistry().rankProfilesOf(schema)) { - for (var constant : profile.constants().values()) - validate(constant, deployState.getApplicationPackage(), exceptionMessageCollector); - } - } - - if (exceptionMessageCollector.exceptionsOccurred) - throw new IllegalArgumentException(exceptionMessageCollector.combinedMessage); - } - - private void validate(RankProfile.Constant constant, - ApplicationPackage applicationPackage, - ExceptionMessageCollector exceptionMessageCollector) { - try { - validate(constant, applicationPackage); - } catch (InvalidConstantTensorException | FileNotFoundException exception) { - exceptionMessageCollector.add(constant, exception); - } - } - - private void validate(RankProfile.Constant rankingConstant, ApplicationPackage application) throws FileNotFoundException { - // Only validate file backed constants: - if (rankingConstant.valuePath().isEmpty()) return; - if (rankingConstant.pathType().get() != DistributableResource.PathType.FILE) return; - - String constantFile = rankingConstant.valuePath().get(); - if (application.getFileReference(Path.fromString("")).getAbsolutePath().endsWith(FilesApplicationPackage.preprocessed) - && constantFile.startsWith(FilesApplicationPackage.preprocessed)) { - constantFile = constantFile.substring(FilesApplicationPackage.preprocessed.length()); - } - - ApplicationFile tensorApplicationFile = application.getFile(Path.fromString(constantFile)); - new ConstantTensorJsonValidator().validate(constantFile, - rankingConstant.type(), - tensorApplicationFile.createReader()); - } - - private static class ExceptionMessageCollector { - - String combinedMessage; - boolean exceptionsOccurred = false; - - ExceptionMessageCollector(String messagePrelude) { - this.combinedMessage = messagePrelude; - } - - public void add(RankProfile.Constant constant, Exception exception) { - exceptionsOccurred = true; - combinedMessage += "\n" + constant.name() + " " + constant.type() + ": file:" + constant.valuePath().get() + - ": " + exception.getMessage(); - } - } - -} diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/RankingConstantsValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/RankingConstantsValidator.java new file mode 100644 index 00000000000..c2f71c78ce3 --- /dev/null +++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/RankingConstantsValidator.java @@ -0,0 +1,81 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.model.application.validation; + +import com.yahoo.config.application.api.ApplicationFile; +import com.yahoo.config.application.api.ApplicationPackage; +import com.yahoo.config.model.application.provider.FilesApplicationPackage; +import com.yahoo.config.model.deploy.DeployState; +import com.yahoo.path.Path; +import com.yahoo.searchdefinition.RankingConstant; +import com.yahoo.searchdefinition.Schema; +import com.yahoo.vespa.model.VespaModel; +import com.yahoo.vespa.model.application.validation.ConstantTensorJsonValidator.InvalidConstantTensorException; + +import java.io.FileNotFoundException; + +/** + * RankingConstantsValidator validates all constant tensors (ranking constants) bundled with an application package + * + * @author Vegard Sjonfjell + */ +public class RankingConstantsValidator extends Validator { + + private static class ExceptionMessageCollector { + + String combinedMessage; + boolean exceptionsOccurred = false; + + ExceptionMessageCollector(String messagePrelude) { + this.combinedMessage = messagePrelude; + } + + public ExceptionMessageCollector add(Throwable throwable, String rcName, String rcFilename) { + exceptionsOccurred = true; + combinedMessage += String.format("\nRanking constant '%s' (%s): %s", rcName, rcFilename, throwable.getMessage()); + return this; + } + } + + static class TensorValidationException extends IllegalArgumentException { + TensorValidationException(String message) { + super(message); + } + } + + @Override + public void validate(VespaModel model, DeployState deployState) { + ApplicationPackage applicationPackage = deployState.getApplicationPackage(); + ExceptionMessageCollector exceptionMessageCollector = new ExceptionMessageCollector("Invalid constant tensor file(s):"); + + for (Schema schema : deployState.getSchemas()) { + for (RankingConstant rc : schema.rankingConstants().asMap().values()) { + try { + validateRankingConstant(rc, applicationPackage); + } catch (InvalidConstantTensorException | FileNotFoundException ex) { + exceptionMessageCollector.add(ex, rc.getName(), rc.getFileName()); + } + } + } + + if (exceptionMessageCollector.exceptionsOccurred) { + throw new TensorValidationException(exceptionMessageCollector.combinedMessage); + } + } + + private void validateRankingConstant(RankingConstant rankingConstant, ApplicationPackage application) throws FileNotFoundException { + // TODO: Handle validation of URI soon too. + if (rankingConstant.getPathType() == RankingConstant.PathType.FILE) { + String constantFile = rankingConstant.getFileName(); + if (application.getFileReference(Path.fromString("")).getAbsolutePath().endsWith(FilesApplicationPackage.preprocessed) + && constantFile.startsWith(FilesApplicationPackage.preprocessed)) { + constantFile = constantFile.substring(FilesApplicationPackage.preprocessed.length()); + } + + ApplicationFile tensorApplicationFile = application.getFile(Path.fromString(constantFile)); + new ConstantTensorJsonValidator().validate(constantFile, + rankingConstant.getTensorType(), + tensorApplicationFile.createReader()); + } + } + +} diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/Validation.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/Validation.java index ce61d3edc3b..4ba809a58ba 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/Validation.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/Validation.java @@ -77,7 +77,7 @@ public class Validation { new NoPrefixForIndexes().validate(model, deployState); new DeploymentSpecValidator().validate(model, deployState); new ValidationOverridesValidator().validate(model, deployState); - new ConstantValidator().validate(model, deployState); + new RankingConstantsValidator().validate(model, deployState); new SecretStoreValidator().validate(model, deployState); new EndpointCertificateSecretsValidator().validate(model, deployState); new AccessControlFilterValidator().validate(model, deployState); diff --git a/config-model/src/main/java/com/yahoo/vespa/model/ml/ConvertedModel.java b/config-model/src/main/java/com/yahoo/vespa/model/ml/ConvertedModel.java index 255ce3a95f6..7bc6f95aafe 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/ml/ConvertedModel.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/ml/ConvertedModel.java @@ -13,6 +13,7 @@ import com.yahoo.path.Path; import com.yahoo.search.query.profile.QueryProfileRegistry; import com.yahoo.searchdefinition.FeatureNames; import com.yahoo.searchdefinition.RankProfile; +import com.yahoo.searchdefinition.RankingConstant; import com.yahoo.searchdefinition.expressiontransforms.RankProfileTransformContext; import com.yahoo.searchlib.rankingexpression.ExpressionFunction; import com.yahoo.searchlib.rankingexpression.RankingExpression; @@ -265,11 +266,11 @@ public class ConvertedModel { private static Map<String, ExpressionFunction> convertStored(ModelStore store, RankProfile profile) { for (Pair<String, Tensor> constant : store.readSmallConstants()) { var name = FeatureNames.asConstantFeature(constant.getFirst()); - profile.add(new RankProfile.Constant(name, constant.getSecond())); + profile.addConstant(name, new RankProfile.Constant(name, constant.getSecond())); } - for (RankProfile.Constant constant : store.readLargeConstants()) { - profile.add(constant); + for (RankingConstant constant : store.readLargeConstants()) { + profile.rankingConstants().putIfAbsent(constant); } for (Pair<String, RankingExpression> function : store.readFunctions()) { @@ -298,7 +299,7 @@ public class ConvertedModel { Tensor constantValue = Tensor.from(constantValueString); store.writeSmallConstant(constantName, constantValue); Reference name = FeatureNames.asConstantFeature(constantName); - profile.add(new RankProfile.Constant(name, constantValue)); + profile.addConstant(name, new RankProfile.Constant(name, constantValue)); } private static void transformLargeConstant(ModelStore store, @@ -317,11 +318,10 @@ public class ConvertedModel { constantsReplacedByFunctions.add(constantName); // will replace constant(constantName) by constantName later } else { - var constantReference = FeatureNames.asConstantFeature(constantName); - if ( ! profile.constants().containsKey(constantReference)) { - Path constantPath = store.writeLargeConstant(constantName, constantValue); - profile.add(new RankProfile.Constant(constantReference, constantValue.type(), constantPath.toString())); - } + profile.rankingConstants().computeIfAbsent(constantName, name -> { + Path constantPath = store.writeLargeConstant(name, constantValue); + return new RankingConstant(name, constantValue.type(), constantPath.toString()); + }); } } @@ -548,17 +548,15 @@ public class ConvertedModel { } /** - * Reads the information about all the large constants stored in the application package + * Reads the information about all the large (aka ranking) constants stored in the application package * (the constant value itself is replicated with file distribution). */ - List<RankProfile.Constant> readLargeConstants() { + List<RankingConstant> readLargeConstants() { try { - List<RankProfile.Constant> constants = new ArrayList<>(); + List<RankingConstant> constants = new ArrayList<>(); for (ApplicationFile constantFile : application.getFile(modelFiles.largeConstantsInfoPath()).listFiles()) { String[] parts = IOUtils.readAll(constantFile.createReader()).split(":"); - constants.add(new RankProfile.Constant(FeatureNames.asConstantFeature(parts[0]), - TensorType.fromSpec(parts[1]), - parts[2])); + constants.add(new RankingConstant(parts[0], TensorType.fromSpec(parts[1]), parts[2])); } return constants; } diff --git a/config-model/src/main/javacc/IntermediateParser.jj b/config-model/src/main/javacc/IntermediateParser.jj index 873196d8bda..1fabe9c57c4 100644 --- a/config-model/src/main/javacc/IntermediateParser.jj +++ b/config-model/src/main/javacc/IntermediateParser.jj @@ -24,6 +24,7 @@ import com.yahoo.searchdefinition.OnnxModel; import com.yahoo.searchdefinition.RankProfile.DiversitySettings; import com.yahoo.searchdefinition.RankProfile.MatchPhaseSettings; import com.yahoo.searchdefinition.RankProfile; +import com.yahoo.searchdefinition.RankingConstant; import com.yahoo.searchdefinition.Schema; import com.yahoo.searchdefinition.document.Case; import com.yahoo.searchdefinition.document.MatchType; @@ -1773,9 +1774,7 @@ void rankingConstant(ParsedSchema schema) : <RBRACE> ) { - if (type == null) throw new IllegalArgumentException("constant '" + name + "' must have a type"); - if (path == null) throw new IllegalArgumentException("constant '" + name + "' must have a file"); - schema.add(new RankProfile.Constant(FeatureNames.asConstantFeature(name), type, path, pathType)); + schema.addRankingConstant(new RankingConstant(name, type, path, pathType)); } } @@ -2454,9 +2453,9 @@ void constant(ParsedSchema schema, ParsedRankProfile profile) : LOOKAHEAD(4) ( ( type = valueType(name) )? <COLON> (<NL>)* ( value = tensorValue(type) | valuePath = fileItem()) { if (value != null) - profile.add(new RankProfile.Constant(name, value)); + profile.addConstant(name, new RankProfile.Constant(name, value)); else - profile.add(new RankProfile.Constant(name, type, valuePath)); + schema.addRankingConstant(new RankingConstant(name.simpleArgument().get(), type, valuePath, DistributableResource.PathType.FILE)); // TODO JON: Move to RankProfile } ) | // Deprecated forms (TODO: Add warning on Vespa 8): @@ -2486,7 +2485,7 @@ void constantValue(ParsedRankProfile profile, Reference name) : } { <COLON> ( value = <DOUBLE> | value = <INTEGER> | value = <IDENTIFIER> ) - { profile.add(new RankProfile.Constant(name, Tensor.from(value.image))); } + { profile.addConstant(name, new RankProfile.Constant(name, Tensor.from(value.image))); } } // Deprecated form @@ -2499,7 +2498,8 @@ void constantTensor(ParsedRankProfile profile, Reference name) : <LBRACE> (<NL>)* (( tensorString = tensorValuePrefixedByValue() | type = tensorTypeWithPrefix(constantTensorErrorMessage(profile.name(), name)) ) (<NL>)* )* <RBRACE> - { profile.add(new RankProfile.Constant(name, type != null ? Tensor.from(type, tensorString) : Tensor.from(tensorString))); + { profile.addConstant(name, + new RankProfile.Constant(name, type != null ? Tensor.from(type, tensorString) : Tensor.from(tensorString))); } } diff --git a/config-model/src/test/derived/schemainheritance/parent.sd b/config-model/src/test/derived/schemainheritance/parent.sd index 41c2d89fff5..51b11dad444 100644 --- a/config-model/src/test/derived/schemainheritance/parent.sd +++ b/config-model/src/test/derived/schemainheritance/parent.sd @@ -23,9 +23,10 @@ schema parent { indexing: input pf1 | lowercase | index | attribute | summary } rank-profile parent_profile { - constants { - parent_constant tensor<float>(x{},y{}): file:constants/my_constant_tensor_file.json - } + } + constant parent_constant { + file: constants/my_constant_tensor_file.json + type: tensor<float>(x{},y{}) } onnx-model parent_model { file: small_constants_and_functions.onnx diff --git a/config-model/src/test/derived/schemainheritance/ranking-constants.cfg b/config-model/src/test/derived/schemainheritance/ranking-constants.cfg deleted file mode 100644 index 9b34e3f1837..00000000000 --- a/config-model/src/test/derived/schemainheritance/ranking-constants.cfg +++ /dev/null @@ -1,6 +0,0 @@ -constant[].name "parent_constant" -constant[].fileref "constants/my_constant_tensor_file.json" -constant[].type "tensor<float>(x{},y{})" -constant[].name "child_constant" -constant[].fileref "constants/my_constant_tensor_file.json" -constant[].type "tensor<float>(x{},y{})" diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/RankProfileRegistryTest.java b/config-model/src/test/java/com/yahoo/searchdefinition/RankProfileRegistryTest.java index cbbb53d8ec8..d07f4513c3c 100644 --- a/config-model/src/test/java/com/yahoo/searchdefinition/RankProfileRegistryTest.java +++ b/config-model/src/test/java/com/yahoo/searchdefinition/RankProfileRegistryTest.java @@ -36,7 +36,7 @@ public class RankProfileRegistryTest { public void testRankProfileDuplicateNameIsIllegal() { Schema schema = new Schema("foo", MockApplicationPackage.createEmpty()); RankProfileRegistry rankProfileRegistry = RankProfileRegistry.createRankProfileRegistryWithBuiltinRankProfiles(schema); - RankProfile barRankProfile = new RankProfile("bar", schema, rankProfileRegistry); + RankProfile barRankProfile = new RankProfile("bar", schema, rankProfileRegistry, schema.rankingConstants()); rankProfileRegistry.add(barRankProfile); rankProfileRegistry.add(barRankProfile); } @@ -48,7 +48,7 @@ public class RankProfileRegistryTest { for (String rankProfileName : RankProfileRegistry.overridableRankProfileNames) { assertNull(rankProfileRegistry.get(schema, rankProfileName).getFunctions().get("foo")); - RankProfile rankProfileWithAddedFunction = new RankProfile(rankProfileName, schema, rankProfileRegistry); + RankProfile rankProfileWithAddedFunction = new RankProfile(rankProfileName, schema, rankProfileRegistry, schema.rankingConstants()); rankProfileWithAddedFunction.addFunction(new ExpressionFunction("foo", RankingExpression.from("1+2")), true); rankProfileRegistry.add(rankProfileWithAddedFunction); assertNotNull(rankProfileRegistry.get(schema, rankProfileName).getFunctions().get("foo")); 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 50f930dde86..03b504bb821 100644 --- a/config-model/src/test/java/com/yahoo/searchdefinition/RankProfileTestCase.java +++ b/config-model/src/test/java/com/yahoo/searchdefinition/RankProfileTestCase.java @@ -52,7 +52,7 @@ public class RankProfileTestCase extends AbstractSchemaTestCase { a.setRankType(RankType.IDENTITY); document.addField("b", DataType.STRING); schema.addDocument(document); - RankProfile child = new RankProfile("child", schema, rankProfileRegistry); + RankProfile child = new RankProfile("child", schema, rankProfileRegistry, schema.rankingConstants()); child.inherit("default"); rankProfileRegistry.add(child); diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/RankingConstantTest.java b/config-model/src/test/java/com/yahoo/searchdefinition/RankingConstantTest.java index 3aab1a7bbe6..6eb74dede62 100644 --- a/config-model/src/test/java/com/yahoo/searchdefinition/RankingConstantTest.java +++ b/config-model/src/test/java/com/yahoo/searchdefinition/RankingConstantTest.java @@ -30,7 +30,7 @@ public class RankingConstantTest { RankProfileRegistry rankProfileRegistry = new RankProfileRegistry(); ApplicationBuilder schemaBuilder = new ApplicationBuilder(rankProfileRegistry); schemaBuilder.addSchema(joinLines( - "schema test {", + "search test {", " document test { }", " rank-profile my_rank_profile {", " first-phase {", @@ -46,12 +46,12 @@ public class RankingConstantTest { schemaBuilder.build(true); Schema schema = schemaBuilder.getSchema(); - Iterator<RankProfile.Constant> constantIterator = schema.constants().values().iterator(); - RankProfile.Constant constant = constantIterator.next(); - assertEquals(TENSOR_NAME, constant.name().simpleArgument().get()); - assertEquals(TENSOR_FILE, constant.valuePath().get()); - assertEquals(TENSOR_TYPE, constant.type().toString()); - assertEquals(DistributableResource.PathType.FILE, constant.pathType().get()); + Iterator<RankingConstant> constantIterator = schema.rankingConstants().asMap().values().iterator(); + RankingConstant constant = constantIterator.next(); + assertEquals(TENSOR_NAME, constant.getName()); + assertEquals(TENSOR_FILE, constant.getFileName()); + assertEquals(TENSOR_TYPE, constant.getType()); + assertEquals(RankingConstant.PathType.FILE, constant.getPathType()); assertFalse(constantIterator.hasNext()); } @@ -63,7 +63,7 @@ public class RankingConstantTest { thrown.expect(IllegalArgumentException.class); thrown.expectMessage("must have a type"); schemaBuilder.addSchema(joinLines( - "schema test {", + "search test {", " document test { }", " constant foo {", " file: bar.baz", @@ -79,7 +79,7 @@ public class RankingConstantTest { thrown.expect(IllegalArgumentException.class); thrown.expectMessage("must have a file"); schemaBuilder.addSchema(joinLines( - "schema test {", + "search test {", " document test { }", " constant foo {", " type: tensor(x[])", @@ -93,7 +93,7 @@ public class RankingConstantTest { RankProfileRegistry rankProfileRegistry = new RankProfileRegistry(); ApplicationBuilder schemaBuilder = new ApplicationBuilder(rankProfileRegistry); schemaBuilder.addSchema(joinLines( - "schema test {", + "search test {", " document test { }", " constant foo {", " type: tensor(x{})", @@ -103,8 +103,8 @@ public class RankingConstantTest { )); schemaBuilder.build(true); Schema schema = schemaBuilder.getSchema(); - RankProfile.Constant constant = schema.constants().values().iterator().next(); - assertEquals("simplename", constant.valuePath().get()); + RankingConstant constant = schema.rankingConstants().asMap().values().iterator().next(); + assertEquals("simplename", constant.getFileName()); } @Test @@ -112,7 +112,7 @@ public class RankingConstantTest { RankProfileRegistry rankProfileRegistry = new RankProfileRegistry(); ApplicationBuilder schemaBuilder = new ApplicationBuilder(rankProfileRegistry); schemaBuilder.addSchema(joinLines( - "schema test {", + "search test {", " document test { }", " constant foo {", " type: tensor(x{})", @@ -122,9 +122,9 @@ public class RankingConstantTest { )); schemaBuilder.build(true); Schema schema = schemaBuilder.getSchema(); - RankProfile.Constant constant = schema.constants().values().iterator().next(); - assertEquals(DistributableResource.PathType.URI, constant.pathType().get()); - assertEquals("http://somewhere.far.away/in/another-galaxy", constant.valuePath().get()); + RankingConstant constant = schema.rankingConstants().asMap().values().iterator().next(); + assertEquals(RankingConstant.PathType.URI, constant.getPathType()); + assertEquals("http://somewhere.far.away/in/another-galaxy", constant.getUri()); } @Test @@ -132,7 +132,7 @@ public class RankingConstantTest { RankProfileRegistry rankProfileRegistry = new RankProfileRegistry(); ApplicationBuilder schemaBuilder = new ApplicationBuilder(rankProfileRegistry); schemaBuilder.addSchema(joinLines( - "schema test {", + "search test {", " document test { }", " constant foo {", " type: tensor(x{})", @@ -142,9 +142,9 @@ public class RankingConstantTest { )); schemaBuilder.build(true); Schema schema = schemaBuilder.getSchema(); - RankProfile.Constant constant = schema.constants().values().iterator().next(); - assertEquals(DistributableResource.PathType.URI, constant.pathType().get()); - assertEquals("https://somewhere.far.away:4443/in/another-galaxy", constant.valuePath().get()); + RankingConstant constant = schema.rankingConstants().asMap().values().iterator().next(); + assertEquals(RankingConstant.PathType.URI, constant.getPathType()); + assertEquals("https://somewhere.far.away:4443/in/another-galaxy", constant.getUri()); } @Test @@ -152,7 +152,7 @@ public class RankingConstantTest { RankProfileRegistry rankProfileRegistry = new RankProfileRegistry(); ApplicationBuilder schemaBuilder = new ApplicationBuilder(rankProfileRegistry); schemaBuilder.addSchema(joinLines( - "schema test {", + "search test {", " document test { }", " constant foo {", " type: tensor(x{})", @@ -162,9 +162,9 @@ public class RankingConstantTest { )); schemaBuilder.build(true); Schema schema = schemaBuilder.getSchema(); - RankProfile.Constant constant = schema.constants().values().iterator().next(); - assertEquals(DistributableResource.PathType.URI, constant.pathType().get()); - assertEquals("http://somewhere.far.away:4080/in/another-galaxy", constant.valuePath().get()); + RankingConstant constant = schema.rankingConstants().asMap().values().iterator().next(); + assertEquals(RankingConstant.PathType.URI, constant.getPathType()); + assertEquals("http://somewhere.far.away:4080/in/another-galaxy", constant.getUri()); } @Test @@ -172,7 +172,7 @@ public class RankingConstantTest { RankProfileRegistry rankProfileRegistry = new RankProfileRegistry(); ApplicationBuilder schemaBuilder = new ApplicationBuilder(rankProfileRegistry); schemaBuilder.addSchema(joinLines( - "schema test {", + "search test {", " document test { }", " constant foo {", " type: tensor(x{})", @@ -182,9 +182,9 @@ public class RankingConstantTest { )); schemaBuilder.build(true); Schema schema = schemaBuilder.getSchema(); - RankProfile.Constant constant = schema.constants().values().iterator().next(); - assertEquals(DistributableResource.PathType.URI, constant.pathType().get()); - assertEquals("http:somewhere.far.away/in/another-galaxy", constant.valuePath().get()); + RankingConstant constant = schema.rankingConstants().asMap().values().iterator().next(); + assertEquals(RankingConstant.PathType.URI, constant.getPathType()); + assertEquals("http:somewhere.far.away/in/another-galaxy", constant.getUri()); } @Test @@ -196,7 +196,7 @@ public class RankingConstantTest { "<URI_PATH> ..."; try { schemaBuilder.addSchema(joinLines( - "schema test {", + "search test {", " document test { }", " constant foo {", " type: tensor(x{})", diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/SchemaTestCase.java b/config-model/src/test/java/com/yahoo/searchdefinition/SchemaTestCase.java index 65f4dab3650..4b40da9e289 100644 --- a/config-model/src/test/java/com/yahoo/searchdefinition/SchemaTestCase.java +++ b/config-model/src/test/java/com/yahoo/searchdefinition/SchemaTestCase.java @@ -122,9 +122,10 @@ public class SchemaTestCase { " indexing: input pf1 | lowercase | index | attribute | summary" + " }" + " rank-profile child1_profile inherits parent_profile {" + - " constants {" + - " child1_constant tensor<float>(x{},y{}): file:constants/my_constant_tensor_file.json" + - " }" + + " }" + + " constant child1_constant {" + + " file: constants/my_constant_tensor_file.json" + + " type: tensor<float>(x{},y{})" + " }" + " onnx-model child1_model {" + " file: models/my_model.onnx" + @@ -187,13 +188,11 @@ public class SchemaTestCase { assertNotNull(child1.getExtraField("child1_field")); assertNotNull(builder.getRankProfileRegistry().get(child1, "parent_profile")); assertNotNull(builder.getRankProfileRegistry().get(child1, "child1_profile")); - var child1profile = builder.getRankProfileRegistry().get(child1, "child1_profile"); assertEquals("parent_profile", builder.getRankProfileRegistry().get(child1, "child1_profile").inheritedNames().get(0)); - assertNotNull(child1.constants().get(FeatureNames.asConstantFeature("parent_constant"))); - assertNotNull(child1profile.constants().get(FeatureNames.asConstantFeature("child1_constant"))); - assertTrue(child1.constants().containsKey(FeatureNames.asConstantFeature("parent_constant"))); - assertTrue(child1profile.constants().containsKey(FeatureNames.asConstantFeature("child1_constant"))); - assertTrue(child1profile.constants().containsKey(FeatureNames.asConstantFeature("parent_constant"))); + assertNotNull(child1.rankingConstants().get("parent_constant")); + assertNotNull(child1.rankingConstants().get("child1_constant")); + assertTrue(child1.rankingConstants().asMap().containsKey("parent_constant")); + assertTrue(child1.rankingConstants().asMap().containsKey("child1_constant")); assertNotNull(child1.onnxModels().get("parent_model")); assertNotNull(child1.onnxModels().get("child1_model")); assertTrue(child1.onnxModels().asMap().containsKey("parent_model")); @@ -225,10 +224,10 @@ public class SchemaTestCase { assertNotNull(builder.getRankProfileRegistry().get(child2, "parent_profile")); assertNotNull(builder.getRankProfileRegistry().get(child2, "child2_profile")); assertEquals("parent_profile", builder.getRankProfileRegistry().get(child2, "child2_profile").inheritedNames().get(0)); - assertNotNull(child2.constants().get(FeatureNames.asConstantFeature("parent_constant"))); - assertNotNull(child2.constants().get(FeatureNames.asConstantFeature("child2_constant"))); - assertTrue(child2.constants().containsKey(FeatureNames.asConstantFeature("parent_constant"))); - assertTrue(child2.constants().containsKey(FeatureNames.asConstantFeature("child2_constant"))); + assertNotNull(child2.rankingConstants().get("parent_constant")); + assertNotNull(child2.rankingConstants().get("child2_constant")); + assertTrue(child2.rankingConstants().asMap().containsKey("parent_constant")); + assertTrue(child2.rankingConstants().asMap().containsKey("child2_constant")); assertNotNull(child2.onnxModels().get("parent_model")); assertNotNull(child2.onnxModels().get("child2_model")); assertTrue(child2.onnxModels().asMap().containsKey("parent_model")); @@ -318,8 +317,8 @@ public class SchemaTestCase { builder.build(true); var application = builder.application(); - assertInheritedFromParent(application.schemas().get("child"), builder.getRankProfileRegistry()); - assertInheritedFromParent(application.schemas().get("grandchild"), builder.getRankProfileRegistry()); + assertInheritedFromParent(application.schemas().get("child"), application, builder.getRankProfileRegistry()); + assertInheritedFromParent(application.schemas().get("grandchild"), application, builder.getRankProfileRegistry()); } @Test @@ -420,15 +419,15 @@ public class SchemaTestCase { } } - private void assertInheritedFromParent(Schema schema, RankProfileRegistry rankProfileRegistry) { + private void assertInheritedFromParent(Schema schema, Application application, RankProfileRegistry rankProfileRegistry) { assertEquals("pf1", schema.fieldSets().userFieldSets().get("parent_set").getFieldNames().stream().findFirst().get()); assertEquals(Stemming.NONE, schema.getStemming()); assertEquals(Stemming.BEST, schema.getIndex("parent_index").getStemming()); assertNotNull(schema.getField("parent_field")); assertNotNull(schema.getExtraField("parent_field")); assertNotNull(rankProfileRegistry.get(schema, "parent_profile")); - assertNotNull(schema.constants().get(FeatureNames.asConstantFeature("parent_constant"))); - assertTrue(schema.constants().containsKey(FeatureNames.asConstantFeature("parent_constant"))); + assertNotNull(schema.rankingConstants().get("parent_constant")); + assertTrue(schema.rankingConstants().asMap().containsKey("parent_constant")); assertNotNull(schema.onnxModels().get("parent_model")); assertTrue(schema.onnxModels().asMap().containsKey("parent_model")); assertNotNull(schema.getSummary("parent_summary")); diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/derived/AbstractExportingTestCase.java b/config-model/src/test/java/com/yahoo/searchdefinition/derived/AbstractExportingTestCase.java index 615f6be5b4f..992d770d851 100644 --- a/config-model/src/test/java/com/yahoo/searchdefinition/derived/AbstractExportingTestCase.java +++ b/config-model/src/test/java/com/yahoo/searchdefinition/derived/AbstractExportingTestCase.java @@ -71,7 +71,6 @@ public abstract class AbstractExportingTestCase extends AbstractSchemaTestCase { .produce(builder.getModel(), new DocumentmanagerConfig.Builder()), path); DerivedConfiguration.exportDocuments(new DocumentTypes().produce(builder.getModel(), new DocumenttypesConfig.Builder()), path); DerivedConfiguration.exportQueryProfiles(builder.getQueryProfileRegistry(), path); - config.exportConstants(path); return config; } diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/derived/LiteralBoostTestCase.java b/config-model/src/test/java/com/yahoo/searchdefinition/derived/LiteralBoostTestCase.java index 70545c15ca0..1e7dd3a2405 100644 --- a/config-model/src/test/java/com/yahoo/searchdefinition/derived/LiteralBoostTestCase.java +++ b/config-model/src/test/java/com/yahoo/searchdefinition/derived/LiteralBoostTestCase.java @@ -38,7 +38,7 @@ public class LiteralBoostTestCase extends AbstractExportingTestCase { SDField field1 = document.addField("a", DataType.STRING); field1.parseIndexingScript("{ index }"); field1.setLiteralBoost(20); - RankProfile other = new RankProfile("other", schema, rankProfileRegistry); + RankProfile other = new RankProfile("other", schema, rankProfileRegistry, schema.rankingConstants()); rankProfileRegistry.add(other); other.addRankSetting(new RankProfile.RankSetting("a", RankProfile.RankSetting.Type.LITERALBOOST, 333)); @@ -70,7 +70,7 @@ public class LiteralBoostTestCase extends AbstractExportingTestCase { schema.addDocument(document); SDField field1 = document.addField("a", DataType.STRING); field1.parseIndexingScript("{ index }"); - RankProfile other = new RankProfile("other", schema, rankProfileRegistry); + RankProfile other = new RankProfile("other", schema, rankProfileRegistry, schema.rankingConstants()); rankProfileRegistry.add(other); other.addRankSetting(new RankProfile.RankSetting("a", RankProfile.RankSetting.Type.LITERALBOOST, 333)); diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionWithOnnxTestCase.java b/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionWithOnnxTestCase.java index bd6310ffd48..0929e54a360 100644 --- a/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionWithOnnxTestCase.java +++ b/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionWithOnnxTestCase.java @@ -10,7 +10,10 @@ import com.yahoo.path.Path; import com.yahoo.search.query.profile.QueryProfileRegistry; import com.yahoo.searchdefinition.FeatureNames; import com.yahoo.searchdefinition.parser.ParseException; +import com.yahoo.searchlib.rankingexpression.evaluation.Value; import com.yahoo.tensor.TensorType; +import com.yahoo.vespa.model.VespaModel; +import com.yahoo.vespa.model.ml.ImportedModelTester; import com.yahoo.yolean.Exceptions; import org.junit.After; import org.junit.Test; @@ -20,8 +23,10 @@ import java.io.FileReader; import java.io.IOException; import java.io.UncheckedIOException; import java.util.ArrayList; +import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.Optional; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; @@ -231,7 +236,7 @@ public class RankingExpressionWithOnnxTestCase { search.assertFirstPhaseExpression(vespaExpressionWithoutConstant, "my_profile_child"); assertNull("Constant overridden by function is not added", - search.search().constants().get(name + "_Variable")); + search.search().rankingConstants().get( name + "_Variable")); // At this point the expression is stored - copy application to another location which do not have a models dir Path storedApplicationDirectory = applicationDir.getParentPath().append("copy"); @@ -246,7 +251,7 @@ public class RankingExpressionWithOnnxTestCase { searchFromStored.assertFirstPhaseExpression(vespaExpressionWithoutConstant, "my_profile"); searchFromStored.assertFirstPhaseExpression(vespaExpressionWithoutConstant, "my_profile_child"); assertNull("Constant overridden by function is not added", - searchFromStored.search().constants().get(name + "_Variable")); + searchFromStored.search().rankingConstants().get( name + "_Variable")); } finally { IOUtils.recursiveDeleteDir(storedApplicationDirectory.toFile()); } @@ -323,7 +328,7 @@ public class RankingExpressionWithOnnxTestCase { } private void assertSmallConstant(String name, TensorType type, RankProfileSearchFixture search) { - var value = search.compiledRankProfile("my_profile").constants().get(FeatureNames.asConstantFeature(name)); + var value = search.compiledRankProfile("my_profile").getConstants().get(FeatureNames.asConstantFeature(name)); assertNotNull(value); assertEquals(type, value.type()); } diff --git a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/ConstantValidatorTest.java b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/ConstantValidatorTest.java deleted file mode 100644 index ff45038a051..00000000000 --- a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/ConstantValidatorTest.java +++ /dev/null @@ -1,34 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.model.application.validation; - -import com.yahoo.vespa.model.test.utils.VespaModelCreatorWithFilePkg; -import org.junit.Test; - -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.fail; - -public class ConstantValidatorTest { - - @Test - public void ensure_that_valid_ranking_constants_do_not_fail() { - new VespaModelCreatorWithFilePkg("src/test/cfg/application/validation/ranking_constants_ok/").create(); - } - - @Test - public void ensure_that_failing_ranking_constants_fails() { - try { - new VespaModelCreatorWithFilePkg("src/test/cfg/application/validation/ranking_constants_fail/").create(); - fail(); - } catch (IllegalArgumentException e) { - String[] lines = e.getMessage().split("\n"); - assertStartsWith("constant(constant_tensor_2) tensor(x[6]): file:tensors/constant_tensor_2.json: Tensor label is not a string", lines[1]); - assertStartsWith("constant(constant_tensor_3) tensor(cpp{},d{}): file:tensors/constant_tensor_3.json: Tensor dimension 'cd' does not exist", lines[2]); - assertStartsWith("constant(constant_tensor_4) tensor(x{},y{}): file:tensors/constant_tensor_4.json: Tensor dimension 'z' does not exist", lines[3]); - } - } - - private void assertStartsWith(String prefix, String value) { - assertEquals(prefix, value.substring(0, prefix.length())); - } - -} diff --git a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/RankingConstantsValidatorTest.java b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/RankingConstantsValidatorTest.java new file mode 100644 index 00000000000..f6fa73dce74 --- /dev/null +++ b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/RankingConstantsValidatorTest.java @@ -0,0 +1,30 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.model.application.validation; + +import com.yahoo.vespa.model.test.utils.VespaModelCreatorWithFilePkg; +import org.junit.Test; + +import static com.yahoo.vespa.model.application.validation.RankingConstantsValidator.TensorValidationException; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + +public class RankingConstantsValidatorTest { + + @Test + public void ensure_that_valid_ranking_constants_do_not_fail() { + new VespaModelCreatorWithFilePkg("src/test/cfg/application/validation/ranking_constants_ok/").create(); + } + + @Test + public void ensure_that_failing_ranking_constants_fails() { + try { + new VespaModelCreatorWithFilePkg("src/test/cfg/application/validation/ranking_constants_fail/").create(); + fail(); + } catch (TensorValidationException e) { + assertTrue(e.getMessage().contains("Ranking constant 'constant_tensor_2' (tensors/constant_tensor_2.json): Tensor label is not a string (VALUE_NUMBER_INT)")); + assertTrue(e.getMessage().contains("Ranking constant 'constant_tensor_3' (tensors/constant_tensor_3.json): Tensor dimension 'cd' does not exist")); + assertTrue(e.getMessage().contains("Ranking constant 'constant_tensor_4' (tensors/constant_tensor_4.json): Tensor dimension 'z' does not exist")); + } + } + +} diff --git a/config-model/src/test/java/com/yahoo/vespa/model/ml/ImportedModelTester.java b/config-model/src/test/java/com/yahoo/vespa/model/ml/ImportedModelTester.java index 556d91acb70..a1fc13adf5c 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/ml/ImportedModelTester.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/ml/ImportedModelTester.java @@ -1,12 +1,14 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.model.ml; +import com.google.common.collect.ImmutableList; import com.yahoo.config.model.ApplicationPackageTester; import ai.vespa.rankingexpression.importer.configmodelview.MlModelImporter; import com.yahoo.config.model.deploy.DeployState; import com.yahoo.io.GrowableByteBuffer; import com.yahoo.io.IOUtils; import com.yahoo.path.Path; +import com.yahoo.searchdefinition.RankingConstant; import ai.vespa.rankingexpression.importer.lightgbm.LightGBMImporter; import ai.vespa.rankingexpression.importer.onnx.OnnxImporter; import ai.vespa.rankingexpression.importer.tensorflow.TensorFlowImporter; @@ -19,12 +21,10 @@ import org.xml.sax.SAXException; import java.io.IOException; import java.io.UncheckedIOException; -import java.util.List; import java.util.Optional; import static junit.framework.TestCase.assertTrue; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotNull; /** * Helper for testing of imported models. @@ -34,11 +34,11 @@ import static org.junit.Assert.assertNotNull; */ public class ImportedModelTester { - private final List<MlModelImporter> importers = List.of(new TensorFlowImporter(), - new OnnxImporter(), - new LightGBMImporter(), - new XGBoostImporter(), - new VespaImporter()); + private final ImmutableList<MlModelImporter> importers = ImmutableList.of(new TensorFlowImporter(), + new OnnxImporter(), + new LightGBMImporter(), + new XGBoostImporter(), + new VespaImporter()); private final String modelName; private final Path applicationDir; @@ -69,10 +69,9 @@ public class ImportedModelTester { public void assertLargeConstant(String constantName, VespaModel model, Optional<Long> expectedSize) { try { Path constantApplicationPackagePath = Path.fromString("models.generated/" + modelName + "/constants").append(constantName + ".tbf"); - var constant = model.rankProfileList().constants().asMap().get(constantName); - assertNotNull(constant); - assertEquals(constantName, constant.getName()); - assertTrue(constant.getFileName().endsWith(constantApplicationPackagePath.toString())); + RankingConstant rankingConstant = model.rankingConstants().get(constantName); + assertEquals(constantName, rankingConstant.getName()); + assertTrue(rankingConstant.getFileName().endsWith(constantApplicationPackagePath.toString())); if (expectedSize.isPresent()) { Path constantPath = applicationDir.append(constantApplicationPackagePath); |