diff options
author | Jon Bratseth <bratseth@gmail.com> | 2022-05-12 19:50:47 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-05-12 19:50:47 +0200 |
commit | 9128b9c93fd58be98409c59892470263f249ec0b (patch) | |
tree | 23637dc005deb2410a773f9561d23b59d8b5461e | |
parent | 012433e354febca577396a2361f3278e89692a44 (diff) | |
parent | ad5d1f2ce6399f2614bfe77d489e98d2adf75c1c (diff) |
Merge pull request #22576 from vespa-engine/bratseth/constants-cleanup
Create distributable constants on deriving
30 files changed, 338 insertions, 337 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 812726a609e..69850b21224 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, RankingConstants rankingConstants) { - super("default", schema, rankProfileRegistry, rankingConstants); + public DefaultRankProfile(Schema schema, RankProfileRegistry rankProfileRegistry) { + super("default", schema, rankProfileRegistry); } /** 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 11c55521100..be2aa500b2b 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> } } - void register(FileRegistry fileRegistry) { + public void register(FileRegistry fileRegistry) { switch (pathType) { case FILE: fileReference = fileRegistry.addFile(path); @@ -91,4 +91,5 @@ 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 acab2b96772..caaef63fd73 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/DocumentsOnlyRankProfile.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/DocumentsOnlyRankProfile.java @@ -14,9 +14,8 @@ import java.util.List; */ public class DocumentsOnlyRankProfile extends RankProfile { - public DocumentsOnlyRankProfile(String name, Schema schema, RankProfileRegistry rankProfileRegistry, - RankingConstants rankingConstants) { - super(name, schema, rankProfileRegistry, rankingConstants); + public DocumentsOnlyRankProfile(String name, Schema schema, RankProfileRegistry rankProfileRegistry) { + super(name, schema, rankProfileRegistry); } @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 b2f46f4a309..1c643292a05 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/ImmutableSchema.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/ImmutableSchema.java @@ -7,6 +7,7 @@ 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; @@ -34,7 +35,7 @@ public interface ImmutableSchema { ApplicationPackage applicationPackage(); DeployLogger getDeployLogger(); ModelContext.Properties getDeployProperties(); - RankingConstants rankingConstants(); + Map<Reference, RankProfile.Constant> constants(); 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 6ba17123fb4..90109026dce 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/RankExpressionBody.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/RankExpressionBody.java @@ -4,7 +4,6 @@ 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; @@ -26,7 +25,7 @@ public class RankExpressionBody extends DistributableResource { } } - void register(FileRegistry fileRegistry) { + public 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 3abf4ec3596..24f6b1390fd 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/RankProfile.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/RankProfile.java @@ -130,7 +130,6 @@ 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; @@ -142,11 +141,10 @@ 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, RankingConstants rankingConstants) { + public RankProfile(String name, Schema schema, RankProfileRegistry rankProfileRegistry) { 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(); @@ -158,11 +156,10 @@ public class RankProfile implements Cloneable { * @param name the name of the new profile */ public RankProfile(String name, ApplicationPackage applicationPackage, DeployLogger deployLogger, - RankProfileRegistry rankProfileRegistry, RankingConstants rankingConstants, OnnxModels onnxModels) { + RankProfileRegistry rankProfileRegistry, 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; @@ -178,11 +175,6 @@ 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(); } @@ -415,24 +407,25 @@ public class RankProfile implements Cloneable { return finalSettings; } - public void addConstant(Reference name, Constant value) { - constants.put(name, value); + public void add(Constant constant) { + constants.put(constant.name(), constant); } /** Returns an unmodifiable view of the constants available in this */ 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.getConstants().entrySet()) { - if (allConstants.containsKey(constant.getKey())) - throw new IllegalArgumentException("Constant '" + constant.getKey() + "' is present in " + + for (var constant : inheritedProfile.getConstants().values()) { + if (allConstants.containsKey(constant.name())) + throw new IllegalArgumentException(constant + "' is present in " + inheritedProfile + " inherited by " + this + ", but is also present in another profile inherited by it"); - allConstants.put(constant.getKey(), constant.getValue()); + allConstants.put(constant.name(), constant); } } + + if (schema != null) + allConstants.putAll(schema.constants()); allConstants.putAll(constants); return allConstants; } @@ -1046,9 +1039,7 @@ public class RankProfile implements Cloneable { Map<Reference, TensorType> featureTypes) { MapEvaluationTypeContext context = new MapEvaluationTypeContext(getExpressionFunctions(), featureTypes); - // 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()) { @@ -1427,23 +1418,32 @@ 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()); + this(name, value.type(), Optional.of(value), Optional.empty(), Optional.empty()); } public Constant(Reference name, TensorType type, String valuePath) { - this(name, type, Optional.empty(), Optional.of(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)); } - private Constant(Reference name, TensorType type, Optional<Tensor> value, Optional<String> 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); + 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,6 +1455,9 @@ 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; @@ -1464,18 +1467,19 @@ 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); + return Objects.hash(name, type, value, valuePath, pathType); } @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 bb2dc04cd2a..2e815b7b503 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, schema.rankingConstants())); - rankProfileRegistry.add(new UnrankedRankProfile(schema, rankProfileRegistry, schema.rankingConstants())); + rankProfileRegistry.add(new DefaultRankProfile(schema, rankProfileRegistry)); + rankProfileRegistry.add(new UnrankedRankProfile(schema, rankProfileRegistry)); 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 deleted file mode 100644 index 7a2765de852..00000000000 --- a/config-model/src/main/java/com/yahoo/searchdefinition/RankingConstant.java +++ /dev/null @@ -1,52 +0,0 @@ -// 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 deleted file mode 100644 index e10da716335..00000000000 --- a/config-model/src/main/java/com/yahoo/searchdefinition/RankingConstants.java +++ /dev/null @@ -1,90 +0,0 @@ -// 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 203a109acd5..db105edc9d4 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/Schema.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/Schema.java @@ -19,6 +19,7 @@ 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; @@ -85,7 +86,9 @@ public class Schema implements ImmutableSchema { /** External rank expression files of this */ private final LargeRankExpressions largeRankExpressions; - private final RankingConstants rankingConstants; + /** 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 OnnxModels onnxModels; @@ -147,7 +150,6 @@ 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)); } @@ -224,8 +226,19 @@ public class Schema implements ImmutableSchema { @Override public LargeRankExpressions rankExpressionFiles() { return largeRankExpressions; } + public void add(RankProfile.Constant constant) { + constants.put(constant.name(), constant); + } + @Override - public RankingConstants rankingConstants() { return rankingConstants; } + 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; + } @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 acf034362ca..0e4578878df 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/UnrankedRankProfile.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/UnrankedRankProfile.java @@ -1,6 +1,7 @@ // 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; @@ -11,8 +12,8 @@ import com.yahoo.searchlib.rankingexpression.parser.ParseException; */ public class UnrankedRankProfile extends RankProfile { - public UnrankedRankProfile(Schema schema, RankProfileRegistry rankProfileRegistry, RankingConstants rankingConstants) { - super("unranked", schema, rankProfileRegistry, rankingConstants); + public UnrankedRankProfile(Schema schema, RankProfileRegistry rankProfileRegistry) { + super("unranked", schema, rankProfileRegistry); 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 8dec0e8339a..31b6d4c3201 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 @@ -81,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.rankingConstants(), schema.rankExpressionFiles(), + rankProfileList = new RankProfileList(schema, schema.constants(), schema.rankExpressionFiles(), schema.onnxModels(), attributeFields, deployState); indexingScript = new IndexingScript(schema); indexInfo = new IndexInfo(schema); 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 new file mode 100644 index 00000000000..c9b0ed2f628 --- /dev/null +++ b/config-model/src/main/java/com/yahoo/searchdefinition/derived/FileDistributedConstants.java @@ -0,0 +1,106 @@ +// 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 a55c97f4ebb..745d9cfeb96 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,8 +9,7 @@ import com.yahoo.searchdefinition.OnnxModel; import com.yahoo.searchdefinition.OnnxModels; import com.yahoo.searchdefinition.LargeRankExpressions; import com.yahoo.searchdefinition.RankProfileRegistry; -import com.yahoo.searchdefinition.RankingConstant; -import com.yahoo.searchdefinition.RankingConstants; +import com.yahoo.searchlib.rankingexpression.Reference; import com.yahoo.vespa.config.search.RankProfilesConfig; import com.yahoo.searchdefinition.RankProfile; import com.yahoo.searchdefinition.Schema; @@ -19,6 +18,8 @@ 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; @@ -38,14 +39,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 RankingConstants rankingConstants; + private final FileDistributedConstants constants; private final LargeRankExpressions largeRankExpressions; private final OnnxModels onnxModels; public static final RankProfileList empty = new RankProfileList(); private RankProfileList() { - rankingConstants = new RankingConstants(null, Optional.empty()); + constants = new FileDistributedConstants(null, List.of()); largeRankExpressions = new LargeRankExpressions(null); onnxModels = new OnnxModels(null, Optional.empty()); } @@ -57,18 +58,43 @@ public class RankProfileList extends Derived implements RankProfilesConfig.Produ * @param attributeFields the attribute fields to create a ranking for */ public RankProfileList(Schema schema, - RankingConstants rankingConstants, + Map<Reference, RankProfile.Constant> constantsFromSchema, LargeRankExpressions largeRankExpressions, OnnxModels onnxModels, AttributeFields attributeFields, DeployState deployState) { setName(schema == null ? "default" : schema.getName()); - this.rankingConstants = rankingConstants; + this.constants = deriveFileDistributedConstants(schema, constantsFromSchema, deployState); 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.getConstants().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()) || @@ -78,7 +104,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 search have a default profile + if (schema != null) { // profiles belonging to a schema have a default profile RawRankProfile rawRank = new RawRankProfile(deployState.rankProfileRegistry().get(schema, "default"), largeRankExpressions, deployState.getQueryProfiles().getRegistry(), @@ -106,6 +132,7 @@ 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, @@ -160,7 +187,7 @@ public class RankProfileList extends Derived implements RankProfilesConfig.Produ } public void getConfig(RankingConstantsConfig.Builder builder) { - for (RankingConstant constant : rankingConstants.asMap().values()) { + for (var constant : constants.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 b04bfca3f76..f772c5fe903 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,8 +5,6 @@ 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; @@ -32,7 +30,7 @@ public class ConvertParsedRanking { if (name.equals("default")) { return rankProfileRegistry.get(schema, "default"); } - return new RankProfile(name, schema, rankProfileRegistry, schema.rankingConstants()); + return new RankProfile(name, schema, rankProfileRegistry); } void convertRankProfile(Schema schema, ParsedRankProfile parsed) { @@ -42,8 +40,8 @@ public class ConvertParsedRanking { parsed.isStrict().ifPresent(value -> profile.setStrict(value)); - for (var constant : parsed.getConstants().entrySet()) - profile.addConstant(constant.getKey(), constant.getValue()); + for (var constant : parsed.getConstants().values()) + profile.add(constant); 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 d3c24fa9924..e56245d1332 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,7 +28,6 @@ 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. @@ -208,14 +207,14 @@ public class ConvertParsedSchemas { if (documentsOnly) { return; // skip ranking-only content, not used for document type generation } - for (var rankingConstant : parsed.getRankingConstants()) { - schema.rankingConstants().add(rankingConstant); + for (var constant : parsed.getConstants()) { + schema.add(constant); } for (var onnxModel : parsed.getOnnxModels()) { schema.onnxModels().add(onnxModel); } - rankProfileRegistry.add(new DefaultRankProfile(schema, rankProfileRegistry, schema.rankingConstants())); - rankProfileRegistry.add(new UnrankedRankProfile(schema, rankProfileRegistry, schema.rankingConstants())); + rankProfileRegistry.add(new DefaultRankProfile(schema, rankProfileRegistry)); + rankProfileRegistry.add(new UnrankedRankProfile(schema, rankProfileRegistry)); 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 bead0215b7d..0ade3bfd76b 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 addConstant(Reference name, RankProfile.Constant value) { - verifyThat(! constants.containsKey(name), "already has constant", name); - constants.put(name, value); + void add(RankProfile.Constant constant) { + verifyThat(! constants.containsKey(constant.name()), "already has constant", constant.name()); + constants.put(constant.name(), constant); } 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 e4f312e98e3..2bc10554b25 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,8 +2,9 @@ package com.yahoo.searchdefinition.parser; import com.yahoo.searchdefinition.OnnxModel; -import com.yahoo.searchdefinition.RankingConstant; +import com.yahoo.searchdefinition.RankProfile; import com.yahoo.searchdefinition.document.Stemming; +import com.yahoo.searchlib.rankingexpression.Reference; import java.util.ArrayList; import java.util.LinkedHashMap; @@ -16,7 +17,7 @@ import java.util.Optional; * one schema (.sd) file, using simple data structures * as far as possible. * - * Do not put advanced logic here! + * Do not put complicated logic here! * * @author arnej27959 */ @@ -39,7 +40,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 List<RankingConstant> rankingConstants = new ArrayList<>(); + private final Map<Reference, RankProfile.Constant> constants = new LinkedHashMap<>(); private final List<String> inherited = new ArrayList<>(); private final List<String> inheritedByDocument = new ArrayList<>(); private final Map<String, ParsedSchema> resolvedInherits = new LinkedHashMap<>(); @@ -70,12 +71,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(); @@ -132,8 +133,8 @@ public class ParsedSchema extends ParsedBlock { rankProfiles.put(rpName, profile); } - void addRankingConstant(RankingConstant constant) { - rankingConstants.add(constant); + void add(RankProfile.Constant constant) { + constants.put(constant.name(), 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 6c04ebbc31e..f79f3d9c82c 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.RankingConstants; +import com.yahoo.searchdefinition.derived.FileDistributedConstants; import com.yahoo.searchdefinition.derived.AttributeFields; import com.yahoo.searchdefinition.derived.RankProfileList; import com.yahoo.searchdefinition.document.SDField; @@ -125,9 +125,6 @@ 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; @@ -173,16 +170,15 @@ 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, rankingConstants, deployState.getFileRegistry()); + createGlobalRankProfiles(deployState, deployState.getFileRegistry()); rankProfileList = new RankProfileList(null, // null search -> global - rankingConstants, + Map.of(), new LargeRankExpressions(deployState.getFileRegistry()), new OnnxModels(deployState.getFileRegistry(), Optional.empty()), AttributeFields.empty, @@ -255,9 +251,6 @@ 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); @@ -282,7 +275,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, RankingConstants rankingConstants, FileRegistry fileRegistry) { + private void createGlobalRankProfiles(DeployState deployState, FileRegistry fileRegistry) { var importedModels = deployState.getImportedModels().all(); DeployLogger deployLogger = deployState.getDeployLogger(); RankProfileRegistry rankProfileRegistry = deployState.rankProfileRegistry(); @@ -292,11 +285,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, rankingConstants, onnxModels); + RankProfile profile = new RankProfile(model.name(), applicationPackage, deployLogger, rankProfileRegistry, 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; })); @@ -309,7 +302,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, rankingConstants, onnxModels); + RankProfile profile = new RankProfile(modelName, applicationPackage, deployLogger, rankProfileRegistry, 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/RankingConstantsValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/RankingConstantsValidator.java index c2f71c78ce3..af2a781a70c 100644 --- 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 @@ -6,7 +6,8 @@ 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.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; @@ -48,11 +49,11 @@ public class RankingConstantsValidator extends Validator { ExceptionMessageCollector exceptionMessageCollector = new ExceptionMessageCollector("Invalid constant tensor file(s):"); for (Schema schema : deployState.getSchemas()) { - for (RankingConstant rc : schema.rankingConstants().asMap().values()) { + for (RankProfile.Constant rc : schema.constants().values()) { try { validateRankingConstant(rc, applicationPackage); } catch (InvalidConstantTensorException | FileNotFoundException ex) { - exceptionMessageCollector.add(ex, rc.getName(), rc.getFileName()); + exceptionMessageCollector.add(ex, rc.name().toString(), rc.valuePath().get()); } } } @@ -62,20 +63,21 @@ public class RankingConstantsValidator extends Validator { } } - 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()); - } + private void validateRankingConstant(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; - ApplicationFile tensorApplicationFile = application.getFile(Path.fromString(constantFile)); - new ConstantTensorJsonValidator().validate(constantFile, - rankingConstant.getTensorType(), - tensorApplicationFile.createReader()); + 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()); } } 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 7bc6f95aafe..89d85a9b417 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,7 +13,6 @@ 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; @@ -266,11 +265,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.addConstant(name, new RankProfile.Constant(name, constant.getSecond())); + profile.add(new RankProfile.Constant(name, constant.getSecond())); } - for (RankingConstant constant : store.readLargeConstants()) { - profile.rankingConstants().putIfAbsent(constant); + for (RankProfile.Constant constant : store.readLargeConstants()) { + profile.add(constant); } for (Pair<String, RankingExpression> function : store.readFunctions()) { @@ -299,7 +298,7 @@ public class ConvertedModel { Tensor constantValue = Tensor.from(constantValueString); store.writeSmallConstant(constantName, constantValue); Reference name = FeatureNames.asConstantFeature(constantName); - profile.addConstant(name, new RankProfile.Constant(name, constantValue)); + profile.add(new RankProfile.Constant(name, constantValue)); } private static void transformLargeConstant(ModelStore store, @@ -318,10 +317,11 @@ public class ConvertedModel { constantsReplacedByFunctions.add(constantName); // will replace constant(constantName) by constantName later } else { - profile.rankingConstants().computeIfAbsent(constantName, name -> { - Path constantPath = store.writeLargeConstant(name, constantValue); - return new RankingConstant(name, constantValue.type(), constantPath.toString()); - }); + var constantReference = FeatureNames.asConstantFeature(constantName); + if ( ! profile.getConstants().containsKey(constantReference)) { + Path constantPath = store.writeLargeConstant(constantName, constantValue); + profile.add(new RankProfile.Constant(constantReference, constantValue.type(), constantPath.toString())); + } } } @@ -548,15 +548,17 @@ public class ConvertedModel { } /** - * Reads the information about all the large (aka ranking) constants stored in the application package + * Reads the information about all the large constants stored in the application package * (the constant value itself is replicated with file distribution). */ - List<RankingConstant> readLargeConstants() { + List<RankProfile.Constant> readLargeConstants() { try { - List<RankingConstant> constants = new ArrayList<>(); + List<RankProfile.Constant> constants = new ArrayList<>(); for (ApplicationFile constantFile : application.getFile(modelFiles.largeConstantsInfoPath()).listFiles()) { String[] parts = IOUtils.readAll(constantFile.createReader()).split(":"); - constants.add(new RankingConstant(parts[0], TensorType.fromSpec(parts[1]), parts[2])); + constants.add(new RankProfile.Constant(FeatureNames.asConstantFeature(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 1fabe9c57c4..873196d8bda 100644 --- a/config-model/src/main/javacc/IntermediateParser.jj +++ b/config-model/src/main/javacc/IntermediateParser.jj @@ -24,7 +24,6 @@ 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; @@ -1774,7 +1773,9 @@ void rankingConstant(ParsedSchema schema) : <RBRACE> ) { - schema.addRankingConstant(new RankingConstant(name, type, path, pathType)); + 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)); } } @@ -2453,9 +2454,9 @@ void constant(ParsedSchema schema, ParsedRankProfile profile) : LOOKAHEAD(4) ( ( type = valueType(name) )? <COLON> (<NL>)* ( value = tensorValue(type) | valuePath = fileItem()) { if (value != null) - profile.addConstant(name, new RankProfile.Constant(name, value)); + profile.add(new RankProfile.Constant(name, value)); else - schema.addRankingConstant(new RankingConstant(name.simpleArgument().get(), type, valuePath, DistributableResource.PathType.FILE)); // TODO JON: Move to RankProfile + profile.add(new RankProfile.Constant(name, type, valuePath)); } ) | // Deprecated forms (TODO: Add warning on Vespa 8): @@ -2485,7 +2486,7 @@ void constantValue(ParsedRankProfile profile, Reference name) : } { <COLON> ( value = <DOUBLE> | value = <INTEGER> | value = <IDENTIFIER> ) - { profile.addConstant(name, new RankProfile.Constant(name, Tensor.from(value.image))); } + { profile.add(new RankProfile.Constant(name, Tensor.from(value.image))); } } // Deprecated form @@ -2498,8 +2499,7 @@ void constantTensor(ParsedRankProfile profile, Reference name) : <LBRACE> (<NL>)* (( tensorString = tensorValuePrefixedByValue() | type = tensorTypeWithPrefix(constantTensorErrorMessage(profile.name(), name)) ) (<NL>)* )* <RBRACE> - { profile.addConstant(name, - new RankProfile.Constant(name, type != null ? Tensor.from(type, tensorString) : Tensor.from(tensorString))); + { profile.add(new RankProfile.Constant(name, type != null ? Tensor.from(type, tensorString) : Tensor.from(tensorString))); } } 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 d07f4513c3c..cbbb53d8ec8 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, schema.rankingConstants()); + RankProfile barRankProfile = new RankProfile("bar", schema, rankProfileRegistry); 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, schema.rankingConstants()); + RankProfile rankProfileWithAddedFunction = new RankProfile(rankProfileName, schema, rankProfileRegistry); 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 03b504bb821..50f930dde86 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, schema.rankingConstants()); + RankProfile child = new RankProfile("child", schema, rankProfileRegistry); 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 6eb74dede62..3aab1a7bbe6 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( - "search test {", + "schema 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<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()); + 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()); assertFalse(constantIterator.hasNext()); } @@ -63,7 +63,7 @@ public class RankingConstantTest { thrown.expect(IllegalArgumentException.class); thrown.expectMessage("must have a type"); schemaBuilder.addSchema(joinLines( - "search test {", + "schema 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( - "search test {", + "schema 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( - "search test {", + "schema test {", " document test { }", " constant foo {", " type: tensor(x{})", @@ -103,8 +103,8 @@ public class RankingConstantTest { )); schemaBuilder.build(true); Schema schema = schemaBuilder.getSchema(); - RankingConstant constant = schema.rankingConstants().asMap().values().iterator().next(); - assertEquals("simplename", constant.getFileName()); + RankProfile.Constant constant = schema.constants().values().iterator().next(); + assertEquals("simplename", constant.valuePath().get()); } @Test @@ -112,7 +112,7 @@ public class RankingConstantTest { RankProfileRegistry rankProfileRegistry = new RankProfileRegistry(); ApplicationBuilder schemaBuilder = new ApplicationBuilder(rankProfileRegistry); schemaBuilder.addSchema(joinLines( - "search test {", + "schema test {", " document test { }", " constant foo {", " type: tensor(x{})", @@ -122,9 +122,9 @@ public class RankingConstantTest { )); schemaBuilder.build(true); Schema schema = schemaBuilder.getSchema(); - RankingConstant constant = schema.rankingConstants().asMap().values().iterator().next(); - assertEquals(RankingConstant.PathType.URI, constant.getPathType()); - assertEquals("http://somewhere.far.away/in/another-galaxy", constant.getUri()); + 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()); } @Test @@ -132,7 +132,7 @@ public class RankingConstantTest { RankProfileRegistry rankProfileRegistry = new RankProfileRegistry(); ApplicationBuilder schemaBuilder = new ApplicationBuilder(rankProfileRegistry); schemaBuilder.addSchema(joinLines( - "search test {", + "schema test {", " document test { }", " constant foo {", " type: tensor(x{})", @@ -142,9 +142,9 @@ public class RankingConstantTest { )); schemaBuilder.build(true); Schema schema = schemaBuilder.getSchema(); - 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()); + 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()); } @Test @@ -152,7 +152,7 @@ public class RankingConstantTest { RankProfileRegistry rankProfileRegistry = new RankProfileRegistry(); ApplicationBuilder schemaBuilder = new ApplicationBuilder(rankProfileRegistry); schemaBuilder.addSchema(joinLines( - "search test {", + "schema test {", " document test { }", " constant foo {", " type: tensor(x{})", @@ -162,9 +162,9 @@ public class RankingConstantTest { )); schemaBuilder.build(true); Schema schema = schemaBuilder.getSchema(); - 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()); + 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()); } @Test @@ -172,7 +172,7 @@ public class RankingConstantTest { RankProfileRegistry rankProfileRegistry = new RankProfileRegistry(); ApplicationBuilder schemaBuilder = new ApplicationBuilder(rankProfileRegistry); schemaBuilder.addSchema(joinLines( - "search test {", + "schema test {", " document test { }", " constant foo {", " type: tensor(x{})", @@ -182,9 +182,9 @@ public class RankingConstantTest { )); schemaBuilder.build(true); Schema schema = schemaBuilder.getSchema(); - RankingConstant constant = schema.rankingConstants().asMap().values().iterator().next(); - assertEquals(RankingConstant.PathType.URI, constant.getPathType()); - assertEquals("http:somewhere.far.away/in/another-galaxy", constant.getUri()); + 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()); } @Test @@ -196,7 +196,7 @@ public class RankingConstantTest { "<URI_PATH> ..."; try { schemaBuilder.addSchema(joinLines( - "search test {", + "schema 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 4b40da9e289..eb9b6e3a733 100644 --- a/config-model/src/test/java/com/yahoo/searchdefinition/SchemaTestCase.java +++ b/config-model/src/test/java/com/yahoo/searchdefinition/SchemaTestCase.java @@ -189,10 +189,10 @@ public class SchemaTestCase { assertNotNull(builder.getRankProfileRegistry().get(child1, "parent_profile")); assertNotNull(builder.getRankProfileRegistry().get(child1, "child1_profile")); assertEquals("parent_profile", builder.getRankProfileRegistry().get(child1, "child1_profile").inheritedNames().get(0)); - 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.constants().get(FeatureNames.asConstantFeature("parent_constant"))); + assertNotNull(child1.constants().get(FeatureNames.asConstantFeature("child1_constant"))); + assertTrue(child1.constants().containsKey(FeatureNames.asConstantFeature("parent_constant"))); + assertTrue(child1.constants().containsKey(FeatureNames.asConstantFeature("child1_constant"))); assertNotNull(child1.onnxModels().get("parent_model")); assertNotNull(child1.onnxModels().get("child1_model")); assertTrue(child1.onnxModels().asMap().containsKey("parent_model")); @@ -224,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.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.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.onnxModels().get("parent_model")); assertNotNull(child2.onnxModels().get("child2_model")); assertTrue(child2.onnxModels().asMap().containsKey("parent_model")); @@ -317,8 +317,8 @@ public class SchemaTestCase { builder.build(true); var application = builder.application(); - assertInheritedFromParent(application.schemas().get("child"), application, builder.getRankProfileRegistry()); - assertInheritedFromParent(application.schemas().get("grandchild"), application, builder.getRankProfileRegistry()); + assertInheritedFromParent(application.schemas().get("child"), builder.getRankProfileRegistry()); + assertInheritedFromParent(application.schemas().get("grandchild"), builder.getRankProfileRegistry()); } @Test @@ -419,15 +419,15 @@ public class SchemaTestCase { } } - private void assertInheritedFromParent(Schema schema, Application application, RankProfileRegistry rankProfileRegistry) { + private void assertInheritedFromParent(Schema schema, 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.rankingConstants().get("parent_constant")); - assertTrue(schema.rankingConstants().asMap().containsKey("parent_constant")); + assertNotNull(schema.constants().get(FeatureNames.asConstantFeature("parent_constant"))); + assertTrue(schema.constants().containsKey(FeatureNames.asConstantFeature("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/LiteralBoostTestCase.java b/config-model/src/test/java/com/yahoo/searchdefinition/derived/LiteralBoostTestCase.java index 1e7dd3a2405..70545c15ca0 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, schema.rankingConstants()); + RankProfile other = new RankProfile("other", schema, rankProfileRegistry); 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, schema.rankingConstants()); + RankProfile other = new RankProfile("other", schema, rankProfileRegistry); 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 0929e54a360..44857835ccf 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,10 +10,7 @@ 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; @@ -23,10 +20,8 @@ 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; @@ -236,7 +231,7 @@ public class RankingExpressionWithOnnxTestCase { search.assertFirstPhaseExpression(vespaExpressionWithoutConstant, "my_profile_child"); assertNull("Constant overridden by function is not added", - search.search().rankingConstants().get( name + "_Variable")); + search.search().constants().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"); @@ -251,7 +246,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().rankingConstants().get( name + "_Variable")); + searchFromStored.search().constants().get(name + "_Variable")); } finally { IOUtils.recursiveDeleteDir(storedApplicationDirectory.toFile()); } 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 index f6fa73dce74..efb6969d6c7 100644 --- 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 @@ -2,6 +2,7 @@ package com.yahoo.vespa.model.application.validation; import com.yahoo.vespa.model.test.utils.VespaModelCreatorWithFilePkg; +import com.yahoo.yolean.Exceptions; import org.junit.Test; import static com.yahoo.vespa.model.application.validation.RankingConstantsValidator.TensorValidationException; @@ -21,9 +22,9 @@ public class RankingConstantsValidatorTest { 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")); + assertTrue(e.getMessage().contains("Ranking constant '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(constant_tensor_3)' (tensors/constant_tensor_3.json): Tensor dimension 'cd' does not exist")); + assertTrue(e.getMessage().contains("Ranking constant '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 a1fc13adf5c..556d91acb70 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,14 +1,12 @@ // 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; @@ -21,10 +19,12 @@ 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.assertEquals; */ public class ImportedModelTester { - private final ImmutableList<MlModelImporter> importers = ImmutableList.of(new TensorFlowImporter(), - new OnnxImporter(), - new LightGBMImporter(), - new XGBoostImporter(), - new VespaImporter()); + private final List<MlModelImporter> importers = List.of(new TensorFlowImporter(), + new OnnxImporter(), + new LightGBMImporter(), + new XGBoostImporter(), + new VespaImporter()); private final String modelName; private final Path applicationDir; @@ -69,9 +69,10 @@ 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"); - RankingConstant rankingConstant = model.rankingConstants().get(constantName); - assertEquals(constantName, rankingConstant.getName()); - assertTrue(rankingConstant.getFileName().endsWith(constantApplicationPackagePath.toString())); + var constant = model.rankProfileList().constants().asMap().get(constantName); + assertNotNull(constant); + assertEquals(constantName, constant.getName()); + assertTrue(constant.getFileName().endsWith(constantApplicationPackagePath.toString())); if (expectedSize.isPresent()) { Path constantPath = applicationDir.append(constantApplicationPackagePath); |