From c86640d12de051ab1ba39d0adf679699d4cbc0ed Mon Sep 17 00:00:00 2001 From: Jon Marius Venstad Date: Fri, 17 Aug 2018 19:32:43 +0200 Subject: Revert "Revert "Revert "Convert all outputs""" --- .../provider/FilesApplicationPackage.java | 3 +- .../config/application/api/ApplicationPackage.java | 6 +- .../searchdefinition/RankProfileRegistry.java | 4 +- .../expressiontransforms/ConvertedModel.java | 381 +++++++++------------ .../expressiontransforms/OnnxFeatureConverter.java | 8 +- .../TensorFlowFeatureConverter.java | 8 +- .../XgboostFeatureConverter.java | 5 +- .../processing/RankProfileSearchFixture.java | 28 +- .../RankingExpressionWithOnnxTestCase.java | 21 +- .../RankingExpressionWithTensorFlowTestCase.java | 41 +-- .../RankingExpressionWithTensorTestCase.java | 9 +- .../RankingExpressionWithXgboostTestCase.java | 4 +- .../server/zookeeper/ZKApplicationPackage.java | 13 +- .../vespa/config/server/zookeeper/ZKLiveApp.java | 12 +- vespajlib/src/main/java/com/yahoo/path/Path.java | 1 - 15 files changed, 210 insertions(+), 334 deletions(-) diff --git a/config-application-package/src/main/java/com/yahoo/config/model/application/provider/FilesApplicationPackage.java b/config-application-package/src/main/java/com/yahoo/config/model/application/provider/FilesApplicationPackage.java index 7ca9bcf48f3..36009682022 100644 --- a/config-application-package/src/main/java/com/yahoo/config/model/application/provider/FilesApplicationPackage.java +++ b/config-application-package/src/main/java/com/yahoo/config/model/application/provider/FilesApplicationPackage.java @@ -45,7 +45,6 @@ import java.security.MessageDigest; import java.util.*; import java.util.jar.JarFile; import java.util.logging.Logger; -import java.util.stream.Collectors; import static com.yahoo.text.Lowercase.toLowerCase; @@ -165,7 +164,7 @@ public class FilesApplicationPackage implements ApplicationPackage { return metaData; } - private List getFiles(Path relativePath, String namePrefix, String suffix, boolean recurse) { + private List getFiles(Path relativePath,String namePrefix,String suffix,boolean recurse) { try { List readers=new ArrayList<>(); File dir = new File(appDir, relativePath.getRelative()); diff --git a/config-model-api/src/main/java/com/yahoo/config/application/api/ApplicationPackage.java b/config-model-api/src/main/java/com/yahoo/config/application/api/ApplicationPackage.java index a71a0878d3d..dd54fe11c39 100644 --- a/config-model-api/src/main/java/com/yahoo/config/application/api/ApplicationPackage.java +++ b/config-model-api/src/main/java/com/yahoo/config/application/api/ApplicationPackage.java @@ -88,14 +88,12 @@ public interface ApplicationPackage { /** * Contents of services.xml. Caller must close reader after use. - * * @return a Reader, or null if no services.xml/vespa-services.xml present */ Reader getServices(); /** * Contents of hosts.xml. Caller must close reader after use. - * * @return a Reader, or null if no hosts.xml/vespa-hosts.xml present */ Reader getHosts(); @@ -162,8 +160,8 @@ public interface ApplicationPackage { * Gets a file from the root of the application package * * - * @param relativePath the relative path of the file within this application package. - * @return information abut the file + * @param relativePath The relative path of the file within this application package. + * @return reader for file * @throws IllegalArgumentException if the given path does not exist */ ApplicationFile getFile(Path relativePath); 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 6311751bb88..7b4d70d85b1 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/RankProfileRegistry.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/RankProfileRegistry.java @@ -52,11 +52,11 @@ public class RankProfileRegistry { } private void checkForDuplicateRankProfile(RankProfile rankProfile) { - String rankProfileName = rankProfile.getName(); + final String rankProfileName = rankProfile.getName(); RankProfile existingRangProfileWithSameName = rankProfiles.get(rankProfile.getSearch()).get(rankProfileName); if (existingRangProfileWithSameName == null) return; - if ( ! overridableRankProfileNames.contains(rankProfileName)) { + if (!overridableRankProfileNames.contains(rankProfileName)) { throw new IllegalArgumentException("Cannot add rank profile '" + rankProfileName + "' in search definition '" + rankProfile.getSearch().getName() + "', since it already exists"); } diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/expressiontransforms/ConvertedModel.java b/config-model/src/main/java/com/yahoo/searchdefinition/expressiontransforms/ConvertedModel.java index 3bd96c9db26..867740c7912 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/expressiontransforms/ConvertedModel.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/expressiontransforms/ConvertedModel.java @@ -1,11 +1,11 @@ package com.yahoo.searchdefinition.expressiontransforms; +import com.google.common.base.Joiner; import com.yahoo.collections.Pair; 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.io.IOUtils; -import com.yahoo.io.reader.NamedReader; import com.yahoo.path.Path; import com.yahoo.search.query.profile.QueryProfileRegistry; import com.yahoo.searchdefinition.FeatureNames; @@ -39,14 +39,11 @@ import com.yahoo.tensor.serialization.TypedBinaryFormat; import java.io.BufferedReader; import java.io.File; -import java.io.FileNotFoundException; import java.io.IOException; -import java.io.Reader; import java.io.StringReader; import java.io.UncheckedIOException; import java.util.ArrayList; import java.util.Collections; -import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; @@ -65,142 +62,69 @@ import java.util.stream.Collectors; */ public class ConvertedModel { - private final String modelName; - private final Path modelPath; + private final ExpressionNode convertedExpression; - /** - * The ranking expressions of this, indexed by their name. which is a 1-3 part string separated by dots - * where the first part is always the model name, the second the signature or (if none) - * expression name (if more than one), and the third is the output name (if any). - */ - private final Map expressions; - - public ConvertedModel(Path modelPath, + public ConvertedModel(FeatureArguments arguments, RankProfileTransformContext context, - ModelImporter modelImporter) { - this.modelPath = modelPath; - this.modelName = toModelName(modelPath); - ModelStore store = new ModelStore(context.rankProfile().getSearch().sourceApplication(), modelPath); + ModelImporter modelImporter, + Map importedModels) { + ModelStore store = new ModelStore(context.rankProfile().getSearch().sourceApplication(), arguments); if ( ! store.hasStoredModel()) // not converted yet - access from models/ directory - expressions = importModel(store, context.rankProfile(), context.queryProfiles(), modelImporter); + convertedExpression = importModel(store, context.rankProfile(), context.queryProfiles(), modelImporter, importedModels); else - expressions = transformFromStoredModel(store, context.rankProfile()); + convertedExpression = transformFromStoredModel(store, context.rankProfile()); } - private Map importModel(ModelStore store, - RankProfile profile, - QueryProfileRegistry queryProfiles, - ModelImporter modelImporter) { - ImportedModel model = modelImporter.importModel(store.modelFiles.modelName(), store.modelDir()); + private ExpressionNode importModel(ModelStore store, + RankProfile profile, + QueryProfileRegistry queryProfiles, + ModelImporter modelImporter, + Map importedModels) { + ImportedModel model = importedModels.computeIfAbsent(store.arguments().modelPath(), + k -> modelImporter.importModel(store.arguments().modelName(), + store.modelDir())); return transformFromImportedModel(model, store, profile, queryProfiles); } - /** Returns the expression matching the given arguments */ - public ExpressionNode expression(FeatureArguments arguments) { - if (expressions.isEmpty()) - throw new IllegalArgumentException("No expressions available in " + this); - - RankingExpression expression = expressions.get(arguments.toName()); - if (expression != null) return expression.getRoot(); - - if ( ! arguments.signature().isPresent()) { - if (expressions.size() > 1) - throw new IllegalArgumentException("Multiple candidate expressions " + missingExpressionMessageSuffix()); - return expressions.values().iterator().next().getRoot(); - } - - if ( ! arguments.output().isPresent()) { - List> entriesWithTheRightPrefix = - expressions.entrySet().stream().filter(entry -> entry.getKey().startsWith(modelName + "." + arguments.signature().get() + ".")).collect(Collectors.toList()); - if (entriesWithTheRightPrefix.size() < 1) - throw new IllegalArgumentException("No expressions named '" + arguments.signature().get() + - missingExpressionMessageSuffix()); - if (entriesWithTheRightPrefix.size() > 1) - throw new IllegalArgumentException("Multiple candidate expression named '" + arguments.signature().get() + - missingExpressionMessageSuffix()); - return entriesWithTheRightPrefix.get(0).getValue().getRoot(); - } - - throw new IllegalArgumentException("No expression '" + arguments.toName() + missingExpressionMessageSuffix()); - } - - private String missingExpressionMessageSuffix() { - return "' in model '" + this.modelPath + "'. " + - "Available expressions: " + expressions.keySet().stream().collect(Collectors.joining(", ")); - } + public ExpressionNode expression() { return convertedExpression; } - private Map transformFromImportedModel(ImportedModel model, - ModelStore store, - RankProfile profile, - QueryProfileRegistry queryProfiles) { + private ExpressionNode transformFromImportedModel(ImportedModel model, + ModelStore store, + RankProfile profile, + QueryProfileRegistry queryProfiles) { // Add constants Set constantsReplacedByMacros = new HashSet<>(); model.smallConstants().forEach((k, v) -> transformSmallConstant(store, profile, k, v)); model.largeConstants().forEach((k, v) -> transformLargeConstant(store, profile, queryProfiles, constantsReplacedByMacros, k, v)); - // Add macros - addGeneratedMacros(model, profile); - - // Add expressions - Map expressions = new HashMap<>(); - for (Map.Entry signatureEntry : model.signatures().entrySet()) { - for (Map.Entry outputEntry : signatureEntry.getValue().outputs().entrySet()) { - addExpression(model.expressions().get(outputEntry.getValue()), - modelName + "." + signatureEntry.getKey() + "." + outputEntry.getKey(), - constantsReplacedByMacros, - model, store, profile, queryProfiles, - expressions); + // Find the specified expression + ImportedModel.Signature signature = chooseSignature(model, store.arguments().signature()); + String output = chooseOutput(signature, store.arguments().output()); + if (signature.skippedOutputs().containsKey(output)) { + String message = "Could not import model output '" + output + "'"; + if (!signature.skippedOutputs().get(output).isEmpty()) { + message += ": " + signature.skippedOutputs().get(output); } - if (signatureEntry.getValue().outputs().isEmpty()) { // fallback: Signature without outputs - addExpression(model.expressions().get(signatureEntry.getKey()), - modelName + "." + signatureEntry.getKey(), - constantsReplacedByMacros, - model, store, profile, queryProfiles, - expressions); + if (!signature.importWarnings().isEmpty()) { + message += ": " + String.join(", ", signature.importWarnings()); } + throw new IllegalArgumentException(message); } - if (model.signatures().isEmpty()) { // fallback: Model without signatures - if (model.expressions().size() == 1) { // Use just model name - addExpression(model.expressions().values().iterator().next(), - modelName, - constantsReplacedByMacros, - model, store, profile, queryProfiles, - expressions); - } - else { - for (Map.Entry expressionEntry : model.expressions().entrySet()) { - addExpression(expressionEntry.getValue(), - modelName + "." + expressionEntry.getKey(), - constantsReplacedByMacros, - model, store, profile, queryProfiles, - expressions); - } - } - } - - // Transform and save macro - must come after reading expressions due to optimization transforms - model.macros().forEach((k, v) -> transformGeneratedMacro(store, constantsReplacedByMacros, k, v)); - return expressions; - } - - private void addExpression(RankingExpression expression, - String expressionName, - Set constantsReplacedByMacros, - ImportedModel model, - ModelStore store, - RankProfile profile, - QueryProfileRegistry queryProfiles, - Map expressions) { + RankingExpression expression = model.expressions().get(output); expression = replaceConstantsByMacros(expression, constantsReplacedByMacros); verifyRequiredMacros(expression, model, profile, queryProfiles); + addGeneratedMacros(model, profile); reduceBatchDimensions(expression, model, profile, queryProfiles); - store.writeExpression(expressionName, expression); - expressions.put(expressionName, expression); + + model.macros().forEach((k, v) -> transformGeneratedMacro(store, constantsReplacedByMacros, k, v)); + + store.writeConverted(expression); + return expression.getRoot(); } - private Map transformFromStoredModel(ModelStore store, RankProfile profile) { + ExpressionNode transformFromStoredModel(ModelStore store, RankProfile profile) { for (Pair constant : store.readSmallConstants()) profile.addConstant(constant.getFirst(), asValue(constant.getSecond())); @@ -213,7 +137,60 @@ public class ConvertedModel { addGeneratedMacroToProfile(profile, macro.getFirst(), macro.getSecond()); } - return store.readExpressions(); + return store.readConverted().getRoot(); + } + + /** + * Returns the specified, existing signature, or the only signature if none is specified. + * Throws IllegalArgumentException in all other cases. + */ + private ImportedModel.Signature chooseSignature(ImportedModel importResult, Optional signatureName) { + if ( ! signatureName.isPresent()) { + if (importResult.signatures().size() == 0) + throw new IllegalArgumentException("No signatures are available"); + if (importResult.signatures().size() > 1) + throw new IllegalArgumentException("Model has multiple signatures (" + + Joiner.on(", ").join(importResult.signatures().keySet()) + + "), one must be specified " + + "as a second argument to tensorflow()"); + return importResult.signatures().values().stream().findFirst().get(); + } + else { + ImportedModel.Signature signature = importResult.signatures().get(signatureName.get()); + if (signature == null) + throw new IllegalArgumentException("Model does not have the specified signature '" + + signatureName.get() + "'"); + return signature; + } + } + + /** + * Returns the specified, existing output expression, or the only output expression if no output name is specified. + * Throws IllegalArgumentException in all other cases. + */ + private String chooseOutput(ImportedModel.Signature signature, Optional outputName) { + if ( ! outputName.isPresent()) { + if (signature.outputs().size() == 0) + throw new IllegalArgumentException("No outputs are available" + skippedOutputsDescription(signature)); + if (signature.outputs().size() > 1) + throw new IllegalArgumentException(signature + " has multiple outputs (" + + Joiner.on(", ").join(signature.outputs().keySet()) + + "), one must be specified " + + "as a third argument to tensorflow()"); + return signature.outputs().get(signature.outputs().keySet().stream().findFirst().get()); + } + else { + String output = signature.outputs().get(outputName.get()); + if (output == null) { + if (signature.skippedOutputs().containsKey(outputName.get())) + throw new IllegalArgumentException("Could not use output '" + outputName.get() + "': " + + signature.skippedOutputs().get(outputName.get())); + else + throw new IllegalArgumentException("Model does not have the specified output '" + + outputName.get() + "'"); + } + return output; + } } private void transformSmallConstant(ModelStore store, RankProfile profile, String constantName, Tensor constantValue) { @@ -250,15 +227,22 @@ public class ConvertedModel { } private void addGeneratedMacroToProfile(RankProfile profile, String macroName, RankingExpression expression) { - if (profile.getMacros().containsKey(macroName)) + if (profile.getMacros().containsKey(macroName)) { throw new IllegalArgumentException("Generated macro '" + macroName + "' already exists."); - + } profile.addMacro(macroName, false); // todo: inline if only used once RankProfile.Macro macro = profile.getMacros().get(macroName); macro.setRankingExpression(expression); macro.setTextualExpression(expression.getRoot().toString()); } + private String skippedOutputsDescription(ImportedModel.Signature signature) { + if (signature.skippedOutputs().isEmpty()) return ""; + StringBuilder b = new StringBuilder(": "); + signature.skippedOutputs().forEach((k, v) -> b.append("Skipping output '").append(k).append("': ").append(v)); + return b.toString(); + } + /** * Verify that the macros referred in the given expression exists in the given rank profile, * and return tensors of the types specified in requiredMacros. @@ -391,8 +375,8 @@ public class ConvertedModel { /** * If batch dimensions have been reduced away above, bring them back here * for any following computation of the tensor. + * Todo: determine when this is not necessary! */ - // TODO: determine when this is not necessary! private ExpressionNode expandBatchDimensionsAtOutput(ExpressionNode node, TensorType before, TensorType after) { if (after.equals(before)) { return node; @@ -468,29 +452,24 @@ public class ConvertedModel { return new TensorValue(tensor); } - private static String toModelName(Path modelPath) { - return modelPath.toString().replace("/", "_"); - } - - @Override - public String toString() { return "model '" + modelName + "'"; } - /** * Provides read/write access to the correct directories of the application package given by the feature arguments */ static class ModelStore { private final ApplicationPackage application; - private final ModelFiles modelFiles; + private final FeatureArguments arguments; - ModelStore(ApplicationPackage application, Path modelPath) { + ModelStore(ApplicationPackage application, FeatureArguments arguments) { this.application = application; - this.modelFiles = new ModelFiles(modelPath); + this.arguments = arguments; } + public FeatureArguments arguments() { return arguments; } + public boolean hasStoredModel() { try { - return application.getFileReference(modelFiles.storedModelPath()).exists(); + return application.getFile(arguments.expressionPath()).exists(); } catch (UnsupportedOperationException e) { return false; @@ -501,49 +480,40 @@ public class ConvertedModel { * Returns the directory which contains the source model to use for these arguments */ public File modelDir() { - return application.getFileReference(ApplicationPackage.MODELS_DIR.append(modelFiles.modelPath())); + return application.getFileReference(ApplicationPackage.MODELS_DIR.append(arguments.modelPath())); } /** * Adds this expression to the application package, such that it can be read later. - * - * @param name the name of this ranking expression - may have 1-3 parts separated by dot where the first part - * is always the model name */ - void writeExpression(String name, RankingExpression expression) { - application.getFile(modelFiles.expressionPath(name)) + void writeConverted(RankingExpression expression) { + application.getFile(arguments.expressionPath()) .writeFile(new StringReader(expression.getRoot().toString())); } - Map readExpressions() { - Map expressions = new HashMap<>(); - ApplicationFile expressionPath = application.getFile(modelFiles.expressionsPath()); - if ( ! expressionPath.exists() || ! expressionPath.isDirectory()) return Collections.emptyMap(); - for (ApplicationFile expressionFile : expressionPath.listFiles()) { - try { - String name = expressionFile.getPath().getName(); - expressions.put(name, new RankingExpression(name, expressionFile.createReader())); - } - catch (FileNotFoundException e) { - throw new IllegalStateException("Expression file removed while reading: " + expressionFile, e); - } - catch (ParseException e) { - throw new IllegalStateException("Invalid stored expression in " + expressionFile, e); - } + /** Reads the previously stored ranking expression for these arguments */ + RankingExpression readConverted() { + try { + return new RankingExpression(application.getFile(arguments.expressionPath()).createReader()); + } + catch (IOException e) { + throw new UncheckedIOException("Could not read " + arguments.expressionPath(), e); + } + catch (ParseException e) { + throw new IllegalStateException("Could not parse " + arguments.expressionPath(), e); } - return expressions; } /** Adds this macro expression to the application package to it can be read later. */ void writeMacro(String name, RankingExpression expression) { - application.getFile(modelFiles.macrosPath()).appendFile(name + "\t" + + application.getFile(arguments.macrosPath()).appendFile(name + "\t" + expression.getRoot().toString() + "\n"); } /** Reads the previously stored macro expressions for these arguments */ List> readMacros() { try { - ApplicationFile file = application.getFile(modelFiles.macrosPath()); + ApplicationFile file = application.getFile(arguments.macrosPath()); if (!file.exists()) return Collections.emptyList(); List> macros = new ArrayList<>(); @@ -557,7 +527,7 @@ public class ConvertedModel { macros.add(new Pair<>(name, expression)); } catch (ParseException e) { - throw new IllegalStateException("Could not parse " + name, e); + throw new IllegalStateException("Could not parse " + arguments.expressionPath(), e); } } return macros; @@ -574,7 +544,7 @@ public class ConvertedModel { List readLargeConstants() { try { List constants = new ArrayList<>(); - for (ApplicationFile constantFile : application.getFile(modelFiles.largeConstantsPath()).listFiles()) { + for (ApplicationFile constantFile : application.getFile(arguments.largeConstantsPath()).listFiles()) { String[] parts = IOUtils.readAll(constantFile.createReader()).split(":"); constants.add(new RankingConstant(parts[0], TensorType.fromSpec(parts[1]), parts[2])); } @@ -592,13 +562,13 @@ public class ConvertedModel { * @return the path to the stored constant, relative to the application package root */ Path writeLargeConstant(String name, Tensor constant) { - Path constantsPath = ApplicationPackage.MODELS_GENERATED_DIR.append(modelFiles.modelPath()).append("constants"); + Path constantsPath = ApplicationPackage.MODELS_GENERATED_DIR.append(arguments.modelPath).append("constants"); // "tbf" ending for "typed binary format" - recognized by the nodes receiving the file: Path constantPath = constantsPath.append(name + ".tbf"); // Remember the constant in a file we replicate in ZooKeeper - application.getFile(modelFiles.largeConstantsPath().append(name + ".constant")) + application.getFile(arguments.largeConstantsPath().append(name + ".constant")) .writeFile(new StringReader(name + ":" + constant.type() + ":" + correct(constantPath))); // Write content explicitly as a file on the file system as this is distributed using file distribution @@ -609,7 +579,7 @@ public class ConvertedModel { private List> readSmallConstants() { try { - ApplicationFile file = application.getFile(modelFiles.smallConstantsPath()); + ApplicationFile file = application.getFile(arguments.smallConstantsPath()); if (!file.exists()) return Collections.emptyList(); List> constants = new ArrayList<>(); @@ -634,7 +604,7 @@ public class ConvertedModel { */ public void writeSmallConstant(String name, Tensor constant) { // Secret file format for remembering constants: - application.getFile(modelFiles.smallConstantsPath()).appendFile(name + "\t" + + application.getFile(arguments.smallConstantsPath()).appendFile(name + "\t" + constant.type().toString() + "\t" + constant.toString() + "\n"); } @@ -658,24 +628,26 @@ public class ConvertedModel { } } - private void close(Reader reader) { - try { - if (reader != null) - reader.close(); - } - catch (IOException e) { - // ignore - } - } - } - static class ModelFiles { + /** Encapsulates the arguments to the import feature */ + static class FeatureArguments { Path modelPath; - public ModelFiles(Path modelPath) { + /** Optional arguments */ + Optional signature, output; + + public FeatureArguments(Arguments arguments) { + this(Path.fromString(asString(arguments.expressions().get(0))), + optionalArgument(1, arguments), + optionalArgument(2, arguments)); + } + + public FeatureArguments(Path modelPath, Optional signature, Optional output) { this.modelPath = modelPath; + this.signature = signature; + this.output = output; } /** Returns modelPath with slashes replaced by underscores */ @@ -683,66 +655,37 @@ public class ConvertedModel { /** Returns relative path to this model below the "models/" dir in the application package */ public Path modelPath() { return modelPath; } + public Optional signature() { return signature; } + public Optional output() { return output; } - public Path storedModelPath() { - return ApplicationPackage.MODELS_GENERATED_REPLICATED_DIR.append(modelPath()); - } - - public Path expressionPath(String name) { - return storedModelPath().append("expressions").append(name); - } - - public Path expressionsPath() { - return storedModelPath().append("expressions"); - } - + /** Path to the small constants file */ public Path smallConstantsPath() { - return storedModelPath().append("constants.txt"); + return ApplicationPackage.MODELS_GENERATED_DIR.append(modelPath).append("constants.txt"); } /** Path to the large (ranking) constants directory */ public Path largeConstantsPath() { - return storedModelPath().append("constants"); + return ApplicationPackage.MODELS_GENERATED_REPLICATED_DIR.append(modelPath).append("constants"); } /** Path to the macros file */ public Path macrosPath() { - return storedModelPath().append("macros.txt"); - } - - } - - /** Encapsulates the arguments of a specific model output */ - static class FeatureArguments { - - private final String modelName; - private final Path modelPath; - - /** Optional arguments */ - private final Optional signature, output; - - public FeatureArguments(Arguments arguments) { - this(Path.fromString(asString(arguments.expressions().get(0))), - optionalArgument(1, arguments), - optionalArgument(2, arguments)); + return ApplicationPackage.MODELS_GENERATED_REPLICATED_DIR.append(modelPath).append("macros.txt"); } - public FeatureArguments(Path modelPath, Optional signature, Optional output) { - this.modelPath = modelPath; - this.modelName = toModelName(modelPath); - this.signature = signature; - this.output = output; + public Path expressionPath() { + return ApplicationPackage.MODELS_GENERATED_REPLICATED_DIR + .append(modelPath).append("expressions").append(expressionFileName()); } - public Path modelPath() { return modelPath; } - - public Optional signature() { return signature; } - public Optional output() { return output; } - - public String toName() { - return modelName + - (signature.isPresent() ? "." + signature.get() : "") + - (output.isPresent() ? "." + output.get() : ""); + private String expressionFileName() { + StringBuilder fileName = new StringBuilder(); + signature.ifPresent(s -> fileName.append(s).append(".")); + output.ifPresent(s -> fileName.append(s).append(".")); + if (fileName.length() == 0) // single signature and output + fileName.append("single."); + fileName.append("expression"); + return fileName.toString(); } private static Optional optionalArgument(int argumentIndex, Arguments arguments) { @@ -751,7 +694,7 @@ public class ConvertedModel { return Optional.of(asString(arguments.expressions().get(argumentIndex))); } - public static String asString(ExpressionNode node) { + private static String asString(ExpressionNode node) { if ( ! (node instanceof ConstantNode)) throw new IllegalArgumentException("Expected a constant string as argument, but got '" + node); return stripQuotes(((ConstantNode)node).sourceString()); diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/expressiontransforms/OnnxFeatureConverter.java b/config-model/src/main/java/com/yahoo/searchdefinition/expressiontransforms/OnnxFeatureConverter.java index 0dec12c4749..d31ffefde65 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/expressiontransforms/OnnxFeatureConverter.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/expressiontransforms/OnnxFeatureConverter.java @@ -31,7 +31,7 @@ public class OnnxFeatureConverter extends ExpressionTransformer convertedModels = new HashMap<>(); + private final Map importedModels = new HashMap<>(); @Override public ExpressionNode transform(ExpressionNode node, RankProfileTransformContext context) { @@ -47,9 +47,9 @@ public class OnnxFeatureConverter extends ExpressionTransformer new ConvertedModel(path, context, onnxImporter)); - return convertedModel.expression(asFeatureArguments(feature.getArguments())); + ConvertedModel convertedModel = new ConvertedModel(asFeatureArguments(feature.getArguments()), + context, onnxImporter, importedModels); + return convertedModel.expression(); } catch (IllegalArgumentException | UncheckedIOException e) { throw new IllegalArgumentException("Could not use Onnx model from " + feature, e); diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/expressiontransforms/TensorFlowFeatureConverter.java b/config-model/src/main/java/com/yahoo/searchdefinition/expressiontransforms/TensorFlowFeatureConverter.java index 585adc0c0d4..d28299b1d30 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/expressiontransforms/TensorFlowFeatureConverter.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/expressiontransforms/TensorFlowFeatureConverter.java @@ -28,7 +28,7 @@ public class TensorFlowFeatureConverter extends ExpressionTransformer convertedModels = new HashMap<>(); + private final Map importedModels = new HashMap<>(); @Override public ExpressionNode transform(ExpressionNode node, RankProfileTransformContext context) { @@ -44,9 +44,9 @@ public class TensorFlowFeatureConverter extends ExpressionTransformer new ConvertedModel(path, context, tensorFlowImporter)); - return convertedModel.expression(asFeatureArguments(feature.getArguments())); + ConvertedModel convertedModel = new ConvertedModel(asFeatureArguments(feature.getArguments()), + context, tensorFlowImporter, importedModels); + return convertedModel.expression(); } catch (IllegalArgumentException | UncheckedIOException e) { throw new IllegalArgumentException("Could not use tensorflow model from " + feature, e); diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/expressiontransforms/XgboostFeatureConverter.java b/config-model/src/main/java/com/yahoo/searchdefinition/expressiontransforms/XgboostFeatureConverter.java index 62f43e15849..4ae223ec3a5 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/expressiontransforms/XgboostFeatureConverter.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/expressiontransforms/XgboostFeatureConverter.java @@ -37,8 +37,7 @@ public class XgboostFeatureConverter extends ExpressionTransformer 1) throw new IllegalArgumentException("An xgboost feature can have at most 1 argument"); diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankProfileSearchFixture.java b/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankProfileSearchFixture.java index ab689b88993..0ce6129ef7f 100644 --- a/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankProfileSearchFixture.java +++ b/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankProfileSearchFixture.java @@ -10,9 +10,7 @@ import com.yahoo.searchdefinition.Search; import com.yahoo.searchdefinition.SearchBuilder; import com.yahoo.searchdefinition.parser.ParseException; -import java.util.HashMap; import java.util.List; -import java.util.Map; import static org.junit.Assert.assertEquals; @@ -27,7 +25,6 @@ class RankProfileSearchFixture { private RankProfileRegistry rankProfileRegistry = new RankProfileRegistry(); private final QueryProfileRegistry queryProfileRegistry; private Search search; - private Map compiledRankProfiles = new HashMap<>(); RankProfileSearchFixture(String rankProfiles) throws ParseException { this(MockApplicationPackage.createEmpty(), new QueryProfileRegistry(), rankProfiles); @@ -57,38 +54,25 @@ class RankProfileSearchFixture { } public void assertFirstPhaseExpression(String expExpression, String rankProfile) { - assertEquals(expExpression, compiledRankProfile(rankProfile).getFirstPhaseRanking().getRoot().toString()); + assertEquals(expExpression, rankProfile(rankProfile).getFirstPhaseRanking().getRoot().toString()); } public void assertSecondPhaseExpression(String expExpression, String rankProfile) { - assertEquals(expExpression, compiledRankProfile(rankProfile).getSecondPhaseRanking().getRoot().toString()); + assertEquals(expExpression, rankProfile(rankProfile).getSecondPhaseRanking().getRoot().toString()); } public void assertRankProperty(String expValue, String name, String rankProfile) { - List rankPropertyList = compiledRankProfile(rankProfile).getRankPropertyMap().get(name); + List rankPropertyList = rankProfile(rankProfile).getRankPropertyMap().get(name); assertEquals(1, rankPropertyList.size()); assertEquals(expValue, rankPropertyList.get(0).getValue()); } - public void assertMacro(String expexctedExpression, String macroName, String rankProfile) { - assertEquals(expexctedExpression, - compiledRankProfile(rankProfile).getMacros().get(macroName).getRankingExpression().getRoot().toString()); + public void assertMacro(String expExpression, String macroName, String rankProfile) { + assertEquals(expExpression, rankProfile(rankProfile).getMacros().get(macroName).getRankingExpression().getRoot().toString()); } - public RankProfile compileRankProfile(String rankProfile) { - RankProfile compiled = rankProfileRegistry.getRankProfile(search, rankProfile).compile(queryProfileRegistry); - compiledRankProfiles.put(rankProfile, compiled); - return compiled; - } - - /** Returns the given uncompiled profile */ public RankProfile rankProfile(String rankProfile) { - return rankProfileRegistry.getRankProfile(search, rankProfile); - } - - /** Returns the given compiled profile, or null if not compiled yet or not present at all */ - public RankProfile compiledRankProfile(String rankProfile) { - return compiledRankProfiles.get(rankProfile); + return rankProfileRegistry.getRankProfile(search, rankProfile).compile(queryProfileRegistry); } public Search search() { return search; } 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 80629172fe3..b2ef08dcc36 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 @@ -123,7 +123,6 @@ public class RankingExpressionWithOnnxTestCase { " expression: onnx('mnist_softmax.onnx')" + " }\n" + " }"); - search.compileRankProfile("my_profile"); search.assertFirstPhaseExpression(vespaExpression, "my_profile"); fail("Expecting exception"); } @@ -165,7 +164,7 @@ public class RankingExpressionWithOnnxTestCase { catch (IllegalArgumentException expected) { assertEquals("Rank profile 'my_profile' is invalid: Could not use Onnx model from " + "onnx('mnist_softmax.onnx','y'): " + - "No expressions named 'y' in model 'mnist_softmax.onnx'. Available expressions: mnist_softmax.onnx.default.add", + "Model does not have the specified signature 'y'", Exceptions.toMessageString(expected)); } } @@ -221,8 +220,7 @@ public class RankingExpressionWithOnnxTestCase { String vespaExpressionWithoutConstant = "join(reduce(join(rename(Placeholder, (d0, d1), (d0, d2)), mnist_softmax_onnx_Variable, f(a,b)(a * b)), sum, d2), constant(mnist_softmax_onnx_Variable_1), f(a,b)(a + b))"; - RankProfileSearchFixture search = uncompiledFixtureWith(rankProfile, new StoringApplicationPackage(applicationDir)); - search.compileRankProfile("my_profile"); + RankProfileSearchFixture search = fixtureWith(rankProfile, new StoringApplicationPackage(applicationDir)); search.assertFirstPhaseExpression(vespaExpressionWithoutConstant, "my_profile"); assertNull("Constant overridden by macro is not added", @@ -236,8 +234,7 @@ public class RankingExpressionWithOnnxTestCase { IOUtils.copyDirectory(applicationDir.append(ApplicationPackage.MODELS_GENERATED_DIR).toFile(), storedApplicationDirectory.append(ApplicationPackage.MODELS_GENERATED_DIR).toFile()); StoringApplicationPackage storedApplication = new StoringApplicationPackage(storedApplicationDirectory); - RankProfileSearchFixture searchFromStored = uncompiledFixtureWith(rankProfile, storedApplication); - searchFromStored.compileRankProfile("my_profile"); + RankProfileSearchFixture searchFromStored = fixtureWith(rankProfile, storedApplication); searchFromStored.assertFirstPhaseExpression(vespaExpressionWithoutConstant, "my_profile"); assertNull("Constant overridden by macro is not added", searchFromStored.search().getRankingConstants().get("mnist_softmax_onnx_Variable")); @@ -274,19 +271,19 @@ public class RankingExpressionWithOnnxTestCase { private RankProfileSearchFixture fixtureWith(String placeholderExpression, String firstPhaseExpression) { return fixtureWith(placeholderExpression, firstPhaseExpression, null, null, "Placeholder", - new StoringApplicationPackage(applicationDir)); + new StoringApplicationPackage(applicationDir)); } private RankProfileSearchFixture fixtureWith(String placeholderExpression, String firstPhaseExpression, String constant, String field) { return fixtureWith(placeholderExpression, firstPhaseExpression, constant, field, "Placeholder", - new StoringApplicationPackage(applicationDir)); + new StoringApplicationPackage(applicationDir)); } - private RankProfileSearchFixture uncompiledFixtureWith(String rankProfile, StoringApplicationPackage application) { + private RankProfileSearchFixture fixtureWith(String rankProfile, StoringApplicationPackage application) { try { return new RankProfileSearchFixture(application, application.getQueryProfiles(), - rankProfile, null, null); + rankProfile, null, null); } catch (ParseException e) { throw new IllegalArgumentException(e); @@ -300,7 +297,7 @@ public class RankingExpressionWithOnnxTestCase { String macroName, StoringApplicationPackage application) { try { - RankProfileSearchFixture fixture = new RankProfileSearchFixture( + return new RankProfileSearchFixture( application, application.getQueryProfiles(), " rank-profile my_profile {\n" + @@ -313,8 +310,6 @@ public class RankingExpressionWithOnnxTestCase { " }", constant, field); - fixture.compileRankProfile("my_profile"); - return fixture; } catch (ParseException e) { throw new IllegalArgumentException(e); diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionWithTensorFlowTestCase.java b/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionWithTensorFlowTestCase.java index 2d33cee5820..7228af2b0de 100644 --- a/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionWithTensorFlowTestCase.java +++ b/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionWithTensorFlowTestCase.java @@ -6,7 +6,6 @@ import com.yahoo.config.application.api.ApplicationPackage; import com.yahoo.config.model.test.MockApplicationPackage; import com.yahoo.io.GrowableByteBuffer; import com.yahoo.io.IOUtils; -import com.yahoo.io.reader.NamedReader; import com.yahoo.path.Path; import com.yahoo.search.query.profile.QueryProfileRegistry; import com.yahoo.searchdefinition.RankingConstant; @@ -23,12 +22,10 @@ import java.io.BufferedInputStream; import java.io.File; import java.io.FileInputStream; import java.io.FileNotFoundException; -import java.io.FileReader; import java.io.IOException; import java.io.InputStream; import java.io.Reader; import java.io.UncheckedIOException; -import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.Iterator; @@ -159,7 +156,6 @@ public class RankingExpressionWithTensorFlowTestCase { " expression: tensorflow('mnist_softmax/saved')" + " }\n" + " }"); - search.compileRankProfile("my_profile"); search.assertFirstPhaseExpression(vespaExpression, "my_profile"); fail("Expecting exception"); } @@ -200,8 +196,7 @@ public class RankingExpressionWithTensorFlowTestCase { catch (IllegalArgumentException expected) { assertEquals("Rank profile 'my_profile' is invalid: Could not use tensorflow model from " + "tensorflow('mnist_softmax/saved','serving_defaultz'): " + - "No expressions named 'serving_defaultz' in model 'mnist_softmax/saved'. "+ - "Available expressions: mnist_softmax_saved.serving_default.y", + "Model does not have the specified signature 'serving_defaultz'", Exceptions.toMessageString(expected)); } } @@ -217,8 +212,7 @@ public class RankingExpressionWithTensorFlowTestCase { catch (IllegalArgumentException expected) { assertEquals("Rank profile 'my_profile' is invalid: Could not use tensorflow model from " + "tensorflow('mnist_softmax/saved','serving_default','x'): " + - "No expression 'mnist_softmax_saved.serving_default.x' in model 'mnist_softmax/saved'. " + - "Available expressions: mnist_softmax_saved.serving_default.y", + "Model does not have the specified output 'x'", Exceptions.toMessageString(expected)); } } @@ -274,8 +268,7 @@ public class RankingExpressionWithTensorFlowTestCase { String vespaExpressionWithoutConstant = "join(reduce(join(rename(Placeholder, (d0, d1), (d0, d2)), mnist_softmax_saved_layer_Variable_read, f(a,b)(a * b)), sum, d2), constant(mnist_softmax_saved_layer_Variable_1_read), f(a,b)(a + b))"; - RankProfileSearchFixture search = fixtureWithUncompiled(rankProfile, new StoringApplicationPackage(applicationDir)); - search.compileRankProfile("my_profile"); + RankProfileSearchFixture search = fixtureWith(rankProfile, new StoringApplicationPackage(applicationDir)); search.assertFirstPhaseExpression(vespaExpressionWithoutConstant, "my_profile"); assertNull("Constant overridden by macro is not added", @@ -289,8 +282,7 @@ public class RankingExpressionWithTensorFlowTestCase { IOUtils.copyDirectory(applicationDir.append(ApplicationPackage.MODELS_GENERATED_DIR).toFile(), storedApplicationDirectory.append(ApplicationPackage.MODELS_GENERATED_DIR).toFile()); StoringApplicationPackage storedApplication = new StoringApplicationPackage(storedApplicationDirectory); - RankProfileSearchFixture searchFromStored = fixtureWithUncompiled(rankProfile, storedApplication); - searchFromStored.compileRankProfile("my_profile"); + RankProfileSearchFixture searchFromStored = fixtureWith(rankProfile, storedApplication); searchFromStored.assertFirstPhaseExpression(vespaExpressionWithoutConstant, "my_profile"); assertNull("Constant overridden by macro is not added", searchFromStored.search().getRankingConstants().get("mnist_softmax_saved_layer_Variable_read")); @@ -305,7 +297,7 @@ public class RankingExpressionWithTensorFlowTestCase { public void testTensorFlowReduceBatchDimension() { final String expression = "join(join(reduce(join(reduce(rename(Placeholder, (d0, d1), (d0, d2)), sum, d0), constant(mnist_softmax_saved_layer_Variable_read), f(a,b)(a * b)), sum, d2), constant(mnist_softmax_saved_layer_Variable_1_read), f(a,b)(a + b)), tensor(d0[1])(1.0), f(a,b)(a * b))"; RankProfileSearchFixture search = fixtureWith("tensor(d0[1],d1[784])(0.0)", - "tensorflow('mnist_softmax/saved')"); + "tensorflow('mnist_softmax/saved')"); search.assertFirstPhaseExpression(expression, "my_profile"); assertLargeConstant("mnist_softmax_saved_layer_Variable_1_read", search, Optional.of(10L)); assertLargeConstant("mnist_softmax_saved_layer_Variable_read", search, Optional.of(7840L)); @@ -370,7 +362,7 @@ public class RankingExpressionWithTensorFlowTestCase { } private void assertSmallConstant(String name, TensorType type, RankProfileSearchFixture search) { - Value value = search.compiledRankProfile("my_profile").getConstants().get(name); + Value value = search.rankProfile("my_profile").getConstants().get(name); assertNotNull(value); assertEquals(type, value.type()); } @@ -418,7 +410,7 @@ public class RankingExpressionWithTensorFlowTestCase { String macroName, StoringApplicationPackage application) { try { - RankProfileSearchFixture fixture = new RankProfileSearchFixture( + return new RankProfileSearchFixture( application, application.getQueryProfiles(), " rank-profile my_profile {\n" + @@ -431,15 +423,13 @@ public class RankingExpressionWithTensorFlowTestCase { " }", constant, field); - fixture.compileRankProfile("my_profile"); - return fixture; } catch (ParseException e) { throw new IllegalArgumentException(e); } } - private RankProfileSearchFixture fixtureWithUncompiled(String rankProfile, StoringApplicationPackage application) { + private RankProfileSearchFixture fixtureWith(String rankProfile, StoringApplicationPackage application) { try { return new RankProfileSearchFixture(application, application.getQueryProfiles(), rankProfile, null, null); @@ -473,21 +463,6 @@ public class RankingExpressionWithTensorFlowTestCase { return new StoringApplicationPackageFile(file, Path.fromString(root.toString())); } - @Override - public List getFiles(Path path, String suffix) { - List readers = new ArrayList<>(); - for (File file : getFileReference(path).listFiles()) { - if ( ! file.getName().endsWith(suffix)) continue; - try { - readers.add(new NamedReader(file.getName(), new FileReader(file))); - } - catch (IOException e) { - throw new UncheckedIOException(e); - } - } - return readers; - } - } static class StoringApplicationPackageFile extends ApplicationFile { diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionWithTensorTestCase.java b/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionWithTensorTestCase.java index 0866d3192cf..dba2bdbfbbf 100644 --- a/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionWithTensorTestCase.java +++ b/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionWithTensorTestCase.java @@ -25,7 +25,6 @@ public class RankingExpressionWithTensorTestCase { " }\n" + " }\n" + " }"); - f.compileRankProfile("my_profile"); f.assertFirstPhaseExpression("reduce(constant(my_tensor), sum)", "my_profile"); f.assertRankProperty("{{x:1,y:2}:1.0,{x:2,y:1}:2.0}", "constant(my_tensor).value", "my_profile"); f.assertRankProperty("tensor(x{},y{})", "constant(my_tensor).type", "my_profile"); @@ -48,7 +47,6 @@ public class RankingExpressionWithTensorTestCase { " }\n" + " }\n" + " }"); - f.compileRankProfile("my_profile"); f.assertFirstPhaseExpression("reduce(constant(my_tensor), sum)", "my_profile"); f.assertRankProperty("{{x:1,y:2}:1.0,{x:2,y:1}:2.0}", "constant(my_tensor).value", "my_profile"); f.assertRankProperty("tensor(x{},y{})", "constant(my_tensor).type", "my_profile"); @@ -67,7 +65,6 @@ public class RankingExpressionWithTensorTestCase { " }\n" + " }\n" + " }"); - f.compileRankProfile("my_profile"); f.assertSecondPhaseExpression("reduce(constant(my_tensor), sum)", "my_profile"); f.assertRankProperty("{{x:1}:1.0}", "constant(my_tensor).value", "my_profile"); f.assertRankProperty("tensor(x{})", "constant(my_tensor).type", "my_profile"); @@ -88,7 +85,6 @@ public class RankingExpressionWithTensorTestCase { " expression: sum(my_tensor)\n" + " }\n" + " }"); - f.compileRankProfile("my_profile"); f.assertFirstPhaseExpression("reduce(constant(my_tensor), sum)", "my_profile"); f.assertRankProperty("{{x:1}:1.0}", "constant(my_tensor).value", "my_profile"); f.assertRankProperty("tensor(x{})", "constant(my_tensor).type", "my_profile"); @@ -110,7 +106,6 @@ public class RankingExpressionWithTensorTestCase { " }\n" + " }\n" + " }"); - f.compileRankProfile("my_profile"); f.assertFirstPhaseExpression("5.0 + my_macro", "my_profile"); f.assertMacro("reduce(constant(my_tensor), sum)", "my_macro", "my_profile"); f.assertRankProperty("{{x:1}:1.0}", "constant(my_tensor).value", "my_profile"); @@ -132,7 +127,6 @@ public class RankingExpressionWithTensorTestCase { " my_number_2: 5.0\n" + " }\n" + " }"); - f.compileRankProfile("my_profile"); f.assertFirstPhaseExpression("3.0 + reduce(constant(my_tensor), sum) + 5.0", "my_profile"); f.assertRankProperty("{{x:1}:1.0}", "constant(my_tensor).value", "my_profile"); f.assertRankProperty("tensor(x{})", "constant(my_tensor).type", "my_profile"); @@ -145,7 +139,7 @@ public class RankingExpressionWithTensorTestCase { public void requireThatInvalidTensorTypeSpecThrowsException() throws ParseException { exception.expect(IllegalArgumentException.class); exception.expectMessage("For constant tensor 'my_tensor' in rank profile 'my_profile': Illegal tensor type spec: Failed parsing element 'x' in type spec 'tensor(x)'"); - RankProfileSearchFixture f = new RankProfileSearchFixture( + new RankProfileSearchFixture( " rank-profile my_profile {\n" + " constants {\n" + " my_tensor {\n" + @@ -154,7 +148,6 @@ public class RankingExpressionWithTensorTestCase { " }\n" + " }\n" + " }"); - f.compileRankProfile("my_profile"); } } diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionWithXgboostTestCase.java b/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionWithXgboostTestCase.java index f98783ad671..b65cb0b3d5f 100644 --- a/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionWithXgboostTestCase.java +++ b/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionWithXgboostTestCase.java @@ -36,7 +36,7 @@ public class RankingExpressionWithXgboostTestCase { String field, RankingExpressionWithTensorFlowTestCase.StoringApplicationPackage application) { try { - RankProfileSearchFixture fixture = new RankProfileSearchFixture( + return new RankProfileSearchFixture( application, application.getQueryProfiles(), " rank-profile my_profile {\n" + @@ -46,8 +46,6 @@ public class RankingExpressionWithXgboostTestCase { " }", constant, field); - fixture.compileRankProfile("my_profile"); - return fixture; } catch (ParseException e) { throw new IllegalArgumentException(e); } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/zookeeper/ZKApplicationPackage.java b/configserver/src/main/java/com/yahoo/vespa/config/server/zookeeper/ZKApplicationPackage.java index e9b4d6ac1aa..2c46f591037 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/zookeeper/ZKApplicationPackage.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/zookeeper/ZKApplicationPackage.java @@ -24,14 +24,12 @@ import java.io.File; import java.io.Reader; import java.io.StringReader; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collections; import java.util.HashMap; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Optional; -import java.util.stream.Collectors; /** * Represents an application residing in zookeeper. @@ -202,17 +200,16 @@ public class ZKApplicationPackage implements ApplicationPackage { return ret; } - /** - * Returns readers for all the children of a node. - * The node is looked up relative to the location of the active application package in zookeeper. - */ + //Returns readers for all the children of a node. + //The node is looked up relative to the location of the active application package + //in zookeeper. @Override - public List getFiles(Path relativePath, String suffix, boolean recurse) { + public List getFiles(Path relativePath,String suffix,boolean recurse) { return liveApp.getAllDataFromDirectory(ConfigCurator.USERAPP_ZK_SUBPATH + '/' + relativePath.getRelative(), suffix, recurse); } @Override - public ApplicationFile getFile(Path file) { + public ApplicationFile getFile(Path file) { // foo/bar/baz.json return new ZKApplicationFile(file, liveApp); } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/zookeeper/ZKLiveApp.java b/configserver/src/main/java/com/yahoo/vespa/config/server/zookeeper/ZKLiveApp.java index 956af02e36f..d7d43dea022 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/zookeeper/ZKLiveApp.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/zookeeper/ZKLiveApp.java @@ -69,8 +69,7 @@ public class ZKLiveApp { log.finer("ZKApplicationPackage: Skipped '" + child + "' (did not match suffix " + fileNameSuffix + ")"); } if (recursive) - result.addAll(getAllDataFromDirectory(path + "/" + child, - namePrefix + child + "/", fileNameSuffix, recursive)); + result.addAll(getAllDataFromDirectory(path + "/" + child, namePrefix + child + "/", fileNameSuffix, recursive)); } if (log.isLoggable(Level.FINE)) log.fine("ZKApplicationPackage: Found '" + result.size() + "' files in " + fullPath); @@ -81,15 +80,14 @@ public class ZKLiveApp { } /** - * Retrieves a node relative to the node of the live application, - * e.g. /vespa/config/apps/$lt;app_id>/<path>/<node> + * Retrieves a node relative to the node of the live application, e.g. /vespa/config/apps/$lt;app_id>/<path>/<node> * * @param path a path relative to the currently active application * @param node a path relative to the path above * @return a Reader that can be used to get the data */ public Reader getDataReader(String path, String node) { - String data = getData(path, node); + final String data = getData(path, node); if (data == null) { throw new IllegalArgumentException("No node for " + getFullPath(path) + "/" + node + " exists"); } @@ -100,8 +98,7 @@ public class ZKLiveApp { try { return zk.getData(getFullPath(path), node); } catch (Exception e) { - throw new IllegalArgumentException("Could not retrieve node '" + - getFullPath(path) + "/" + node + "' in zookeeper", e); + throw new IllegalArgumentException("Could not retrieve node '" + getFullPath(path) + "/" + node + "' in zookeeper", e); } } @@ -208,6 +205,5 @@ public class ZKLiveApp { } return reader(data); } - } diff --git a/vespajlib/src/main/java/com/yahoo/path/Path.java b/vespajlib/src/main/java/com/yahoo/path/Path.java index 2806631be18..c466fe50d6f 100644 --- a/vespajlib/src/main/java/com/yahoo/path/Path.java +++ b/vespajlib/src/main/java/com/yahoo/path/Path.java @@ -84,7 +84,6 @@ public final class Path { /** * Get the name of this path element, typically the last element in the path string. - * * @return the name */ public String getName() { -- cgit v1.2.3