diff options
author | Jon Bratseth <bratseth@gmail.com> | 2022-05-15 18:02:34 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-05-15 18:02:34 +0200 |
commit | b66953efc96e262f3d071f7aaa036492736dee8e (patch) | |
tree | e41ae09fc46e4e0ff1492377a5e13dbe2ac3ad9f | |
parent | 420759450bacbd454a90d02eea352436d47df760 (diff) | |
parent | 02d0faa424db6c3ede74dd685438cd234c693139 (diff) |
Merge pull request #22600 from vespa-engine/bratseth/constants-cleanup-2
Bratseth/constants cleanup 2
7 files changed, 35 insertions, 48 deletions
diff --git a/config-model/src/main/java/com/yahoo/config/model/deploy/DeployState.java b/config-model/src/main/java/com/yahoo/config/model/deploy/DeployState.java index 74d92bcfd02..3cd2499f968 100644 --- a/config-model/src/main/java/com/yahoo/config/model/deploy/DeployState.java +++ b/config-model/src/main/java/com/yahoo/config/model/deploy/DeployState.java @@ -41,6 +41,7 @@ import com.yahoo.vespa.model.container.search.QueryProfiles; import com.yahoo.vespa.model.container.search.QueryProfilesBuilder; import com.yahoo.vespa.model.container.search.SemanticRuleBuilder; import com.yahoo.vespa.model.container.search.SemanticRules; +import com.yahoo.yolean.Exceptions; import java.io.File; import java.io.FileNotFoundException; @@ -217,7 +218,7 @@ public class DeployState implements ConfigDefinitionStore { for (var entry : importedModels.getSkippedModels().entrySet()) { // TODO: Vespa 8: Throw IllegalArgumentException instead deployLogger.logApplicationPackage(Level.WARNING, "Skipping import of model " + entry.getKey() + " as an exception " + - "occurred during import. Error: " + entry.getValue()); + "occurred during import: " + entry.getValue()); } return importedModels; } 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 index c9b0ed2f628..8de86beacdb 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/derived/FileDistributedConstants.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/derived/FileDistributedConstants.java @@ -20,54 +20,30 @@ import java.util.function.Function; */ public class FileDistributedConstants { - private final FileRegistry fileRegistry; - - private final Map<String, DistributableConstant> constants = new LinkedHashMap<>(); + private final Map<String, DistributableConstant> constants; public FileDistributedConstants(FileRegistry fileRegistry, Collection<RankProfile.Constant> constants) { - this.fileRegistry = fileRegistry; + Map<String, DistributableConstant> distributableConstants = new LinkedHashMap<>(); for (var constant : constants) { - if (constant.valuePath().isPresent()) - add(new DistributableConstant(constant.name().simpleArgument().get(), - constant.type(), - constant.valuePath().get(), - constant.pathType().get())); + if ( ! constant.valuePath().isPresent()) continue; + + var distributableConstant = new DistributableConstant(constant.name().simpleArgument().get(), + constant.type(), + constant.valuePath().get(), + constant.pathType().get()); + distributableConstant.validate(); + distributableConstant.register(fileRegistry); + distributableConstants.put(distributableConstant.getName(), distributableConstant); } - } - - 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; - }); + this.constants = Collections.unmodifiableMap(distributableConstants); } /** Returns a read-only map of the constants in this indexed by name. */ - public Map<String, DistributableConstant> asMap() { - return Collections.unmodifiableMap(constants); - } + public Map<String, DistributableConstant> asMap() { return constants; } public static class DistributableConstant extends DistributableResource { - private TensorType tensorType; + private final TensorType tensorType; public DistributableConstant(String name, TensorType type, String fileName) { this(name, type, fileName, PathType.FILE); @@ -79,10 +55,6 @@ public class FileDistributedConstants { validate(); } - public void setType(TensorType type) { - this.tensorType = type; - } - public TensorType getTensorType() { return tensorType; } public String getType() { return tensorType.toString(); } diff --git a/config-model/src/test/cfg/application/stateless_eval/example.model b/config-model/src/test/cfg/application/stateless_eval/example.model index af1c85be4f0..1d2db15c3ba 100644 --- a/config-model/src/test/cfg/application/stateless_eval/example.model +++ b/config-model/src/test/cfg/application/stateless_eval/example.model @@ -7,6 +7,7 @@ model example { constants { constant1: tensor(x[3]):{{x:0}:0.5, {x:1}:1.5, {x:2}:2.5} constant2: 3.0 + #constant1asLarge tensor(x[3]): file:constant1asLarge.json } constant constant1asLarge { diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionWithOnnxModelTestCase.java b/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionWithOnnxModelTestCase.java index 2057bcaba04..1c23950d972 100644 --- a/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionWithOnnxModelTestCase.java +++ b/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionWithOnnxModelTestCase.java @@ -8,6 +8,7 @@ import com.yahoo.io.IOUtils; import com.yahoo.path.Path; import com.yahoo.vespa.config.search.RankProfilesConfig; import com.yahoo.vespa.config.search.core.OnnxModelsConfig; +import com.yahoo.vespa.config.search.core.RankingConstantsConfig; import com.yahoo.vespa.model.VespaModel; import com.yahoo.vespa.model.search.DocumentDatabase; import com.yahoo.vespa.model.search.IndexedSearchCluster; @@ -57,6 +58,15 @@ public class RankingExpressionWithOnnxModelTestCase { private void assertGeneratedConfig(VespaModel vespaModel) { DocumentDatabase db = ((IndexedSearchCluster)vespaModel.getSearchClusters().get(0)).getDocumentDbs().get(0); + + RankingConstantsConfig.Builder rankingConstantsConfigBuilder = new RankingConstantsConfig.Builder(); + db.getConfig(rankingConstantsConfigBuilder); + var rankingConstantsConfig = rankingConstantsConfigBuilder.build(); + assertEquals(1, rankingConstantsConfig.constant().size()); + assertEquals("my_constant", rankingConstantsConfig.constant(0).name()); + assertEquals("tensor(d0[2])", rankingConstantsConfig.constant(0).type()); + assertEquals("files/constant.json", rankingConstantsConfig.constant(0).fileref().value()); + OnnxModelsConfig.Builder builder = new OnnxModelsConfig.Builder(); ((OnnxModelsConfig.Producer) db).getConfig(builder); OnnxModelsConfig config = new OnnxModelsConfig(builder); diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/ml/ModelsEvaluatorTest.java b/config-model/src/test/java/com/yahoo/vespa/model/container/ml/ModelsEvaluatorTest.java index f246b87d9bf..bec72f78a65 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/container/ml/ModelsEvaluatorTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/container/ml/ModelsEvaluatorTest.java @@ -18,8 +18,7 @@ import static org.junit.Assume.assumeTrue; public class ModelsEvaluatorTest { @Test - public void testModelsEvaluatorTester() { - assumeTrue(OnnxEvaluator.isRuntimeAvailable()); + public void testModelsEvaluator() { ModelsEvaluator modelsEvaluator = ModelsEvaluatorTester.create("src/test/cfg/application/stateless_eval"); assertEquals(3, modelsEvaluator.models().size()); 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..bd07513b704 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,6 +1,7 @@ // 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.yahoo.config.FileReference; import com.yahoo.config.model.ApplicationPackageTester; import ai.vespa.rankingexpression.importer.configmodelview.MlModelImporter; import com.yahoo.config.model.deploy.DeployState; @@ -22,9 +23,9 @@ 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; +import static org.junit.Assert.assertTrue; /** * Helper for testing of imported models. @@ -74,6 +75,8 @@ public class ImportedModelTester { assertEquals(constantName, constant.getName()); assertTrue(constant.getFileName().endsWith(constantApplicationPackagePath.toString())); + assertTrue(model.fileReferences().contains(new FileReference(constant.getFileName()))); + if (expectedSize.isPresent()) { Path constantPath = applicationDir.append(constantApplicationPackagePath); assertTrue("Constant file '" + constantPath + "' has been written", diff --git a/model-integration/src/main/java/ai/vespa/rankingexpression/importer/configmodelview/ImportedMlModels.java b/model-integration/src/main/java/ai/vespa/rankingexpression/importer/configmodelview/ImportedMlModels.java index 367b840a728..1003ebd8f3f 100644 --- a/model-integration/src/main/java/ai/vespa/rankingexpression/importer/configmodelview/ImportedMlModels.java +++ b/model-integration/src/main/java/ai/vespa/rankingexpression/importer/configmodelview/ImportedMlModels.java @@ -3,6 +3,7 @@ package ai.vespa.rankingexpression.importer.configmodelview; import com.yahoo.concurrent.InThreadExecutorService; import com.yahoo.path.Path; +import com.yahoo.yolean.Exceptions; import java.io.File; import java.util.Arrays; @@ -55,7 +56,7 @@ public class ImportedMlModels { models.put(name, model); } } catch (InterruptedException | ExecutionException e) { - skippedModels.put(name, e.getMessage()); + skippedModels.put(name, Exceptions.toMessageString(e)); } }); importedModels = Collections.unmodifiableMap(models); @@ -97,7 +98,7 @@ public class ImportedMlModels { if (existing != null) { try { throw new IllegalArgumentException("The models in " + child + " and " + existing.get().source() + - " both resolve to the model name '" + name + "'"); + " both resolve to the model name '" + name + "'"); } catch (InterruptedException | ExecutionException e) {} } |