From b3d3dd71e6f576de36a3e5c18f1fcd6ce6933f17 Mon Sep 17 00:00:00 2001 From: jonmv Date: Sun, 2 Oct 2022 21:23:24 +0200 Subject: Revert "Merge pull request #24257 from vespa-engine/bratseth/boolean-optimize-primitives-only" This reverts commit 4a1ca594e4cf3810974696ce970f5a161ec099eb, reversing changes made to 62928f4d8b7571c4b10fedffc56b762f57b6b2ca. --- .../BooleanExpressionTransformerTestCase.java | 68 +++++++++------------- .../RankingExpressionWithOnnxTestCase.java | 51 ++++++---------- 2 files changed, 43 insertions(+), 76 deletions(-) (limited to 'config-model/src/test/java/com/yahoo') diff --git a/config-model/src/test/java/com/yahoo/schema/expressiontransforms/BooleanExpressionTransformerTestCase.java b/config-model/src/test/java/com/yahoo/schema/expressiontransforms/BooleanExpressionTransformerTestCase.java index d06573f7bae..d692b69d3c8 100644 --- a/config-model/src/test/java/com/yahoo/schema/expressiontransforms/BooleanExpressionTransformerTestCase.java +++ b/config-model/src/test/java/com/yahoo/schema/expressiontransforms/BooleanExpressionTransformerTestCase.java @@ -2,13 +2,10 @@ package com.yahoo.schema.expressiontransforms; import com.yahoo.searchlib.rankingexpression.RankingExpression; -import com.yahoo.searchlib.rankingexpression.Reference; import com.yahoo.searchlib.rankingexpression.evaluation.MapContext; import com.yahoo.searchlib.rankingexpression.evaluation.MapTypeContext; import com.yahoo.searchlib.rankingexpression.rule.OperationNode; import com.yahoo.searchlib.rankingexpression.transform.TransformContext; -import com.yahoo.tensor.TensorType; -import com.yahoo.tensor.evaluation.TypeContext; import org.junit.jupiter.api.Test; import java.util.Map; @@ -23,7 +20,7 @@ import static org.junit.jupiter.api.Assertions.assertTrue; public class BooleanExpressionTransformerTestCase { @Test - public void booleanTransformation() throws Exception { + public void testTransformer() throws Exception { assertTransformed("if (a, b, false)", "a && b"); assertTransformed("if (a, true, b)", "a || b"); assertTransformed("if (a, true, b + c)", "a || b + c"); @@ -36,17 +33,16 @@ public class BooleanExpressionTransformerTestCase { } @Test - public void noTransformationOnTensorTypes() throws Exception { - var typeContext = new MapTypeContext(); - typeContext.setType(Reference.fromIdentifier("tensorA"), TensorType.fromSpec("tensor(x{})")); - typeContext.setType(Reference.fromIdentifier("tensorB"), TensorType.fromSpec("tensor(x{})")); - assertUntransformed("tensorA && tensorB", typeContext); - assertTransformed("a && (tensorA * tensorB)","a && ( tensorA * tensorB)", typeContext); + public void testIt() throws Exception { + assertTransformed("if(1 - 1, true, 1 - 1)", "1 - 1 || 1 - 1"); } @Test public void testNotSkewingNonBoolean() throws Exception { - var expr = assertTransformed("a + b + c * d + e + f", "a + b + c * d + e + f"); + assertTransformed("a + b + c * d + e + f", "a + b + c * d + e + f"); + var expr = new BooleanExpressionTransformer() + .transform(new RankingExpression("a + b + c * d + e + f"), + new TransformContext(Map.of(), new MapTypeContext())); assertTrue(expr.getRoot() instanceof OperationNode); OperationNode root = (OperationNode) expr.getRoot(); assertEquals(5, root.operators().size()); @@ -55,53 +51,41 @@ public class BooleanExpressionTransformerTestCase { @Test public void testTransformPreservesPrecedence() throws Exception { - assertUntransformed("a"); - assertUntransformed("a + b"); - assertUntransformed("a + b + c"); - assertUntransformed("a * b"); - assertUntransformed("a + b * c + d"); - assertUntransformed("a + b + c * d + e + f"); - assertUntransformed("a * b + c + d + e * f"); - assertUntransformed("(a * b) + c + d + e * f"); - assertUntransformed("(a * b + c) + d + e * f"); - assertUntransformed("a * (b + c) + d + e * f"); - assertUntransformed("(a * b) + (c + (d + e)) * f"); - } - - private void assertUntransformed(String input) throws Exception { - assertUntransformed(input, new MapTypeContext()); - } - - private void assertUntransformed(String input, MapTypeContext typeContext) throws Exception { - assertTransformed(input, input, typeContext); + assertUnTransformed("a"); + assertUnTransformed("a + b"); + assertUnTransformed("a + b + c"); + assertUnTransformed("a * b"); + assertUnTransformed("a + b * c + d"); + assertUnTransformed("a + b + c * d + e + f"); + assertUnTransformed("a * b + c + d + e * f"); + assertUnTransformed("(a * b) + c + d + e * f"); + assertUnTransformed("(a * b + c) + d + e * f"); + assertUnTransformed("a * (b + c) + d + e * f"); + assertUnTransformed("(a * b) + (c + (d + e)) * f"); } - private RankingExpression assertTransformed(String expected, String input) throws Exception { - return assertTransformed(expected, input, new MapTypeContext()); + private void assertUnTransformed(String input) throws Exception { + assertTransformed(input, input); } - private RankingExpression assertTransformed(String expected, String input, MapTypeContext typeContext) throws Exception { - MapContext context = contextWithSingleLetterVariables(typeContext); + private void assertTransformed(String expected, String input) throws Exception { var transformedExpression = new BooleanExpressionTransformer() .transform(new RankingExpression(input), - new TransformContext(Map.of(), typeContext)); + new TransformContext(Map.of(), new MapTypeContext())); assertEquals(new RankingExpression(expected), transformedExpression, "Transformed as expected"); + MapContext context = contextWithSingleLetterVariables(); var inputExpression = new RankingExpression(input); assertEquals(inputExpression.evaluate(context).asBoolean(), transformedExpression.evaluate(context).asBoolean(), "Transform and original input are equivalent"); - return transformedExpression; } - private MapContext contextWithSingleLetterVariables(MapTypeContext typeContext) { + private MapContext contextWithSingleLetterVariables() { var context = new MapContext(); - for (int i = 0; i < 26; i++) { - String name = Character.toString(i + 97); - typeContext.setType(Reference.fromIdentifier(name), TensorType.empty); - context.put(name, Math.floorMod(i, 2)); - } + for (int i = 0; i < 26; i++) + context.put(Character.toString(i + 97), Math.floorMod(i, 2)); return context; } diff --git a/config-model/src/test/java/com/yahoo/schema/processing/RankingExpressionWithOnnxTestCase.java b/config-model/src/test/java/com/yahoo/schema/processing/RankingExpressionWithOnnxTestCase.java index 2f53dba7bb4..83d19b010bb 100644 --- a/config-model/src/test/java/com/yahoo/schema/processing/RankingExpressionWithOnnxTestCase.java +++ b/config-model/src/test/java/com/yahoo/schema/processing/RankingExpressionWithOnnxTestCase.java @@ -31,8 +31,6 @@ public class RankingExpressionWithOnnxTestCase { private final static String name = "mnist_softmax"; private final static String vespaExpression = "join(reduce(join(rename(Placeholder, (d0, d1), (d0, d2)), constant(mnist_softmax_layer_Variable), f(a,b)(a * b)), sum, d2) * 1.0, constant(mnist_softmax_layer_Variable_1) * 1.0, f(a,b)(a + b))"; - private final static String vespaExpressionConstants = "constant mnist_softmax_layer_Variable { file: ignored\ntype: tensor(d0[1],d1[784]) }\n" + - "constant mnist_softmax_layer_Variable_1 { file: ignored\ntype: tensor(d0[1],d1[784]) }\n"; @AfterEach public void removeGeneratedModelFiles() { @@ -43,7 +41,7 @@ public class RankingExpressionWithOnnxTestCase { void testOnnxReferenceWithConstantFeature() { RankProfileSearchFixture search = fixtureWith("constant(mytensor)", "onnx_vespa('mnist_softmax.onnx')", - vespaExpressionConstants + "constant mytensor { file: ignored\ntype: tensor(d0[1],d1[784]) }", + "constant mytensor { file: ignored\ntype: tensor(d0[1],d1[784]) }", null); search.assertFirstPhaseExpression(vespaExpression, "my_profile"); } @@ -60,7 +58,7 @@ public class RankingExpressionWithOnnxTestCase { queryProfileType); RankProfileSearchFixture search = fixtureWith("query(mytensor)", "onnx_vespa('mnist_softmax.onnx')", - vespaExpressionConstants, + null, null, "Placeholder", application); @@ -72,7 +70,7 @@ public class RankingExpressionWithOnnxTestCase { StoringApplicationPackage application = new StoringApplicationPackage(applicationDir); RankProfileSearchFixture search = fixtureWith("attribute(mytensor)", "onnx_vespa('mnist_softmax.onnx')", - vespaExpressionConstants, + null, "field mytensor type tensor(d0[1],d1[784]) { indexing: attribute }", "Placeholder", application); @@ -90,7 +88,7 @@ public class RankingExpressionWithOnnxTestCase { StoringApplicationPackage application = new StoringApplicationPackage(applicationDir, queryProfile, queryProfileType); RankProfileSearchFixture search = fixtureWith("sum(query(mytensor) * attribute(mytensor) * constant(mytensor),d2)", "onnx_vespa('mnist_softmax.onnx')", - vespaExpressionConstants + "constant mytensor { file: ignored\ntype: tensor(d0[1],d1[784]) }", + "constant mytensor { file: ignored\ntype: tensor(d0[1],d1[784]) }", "field mytensor type tensor(d0[1],d1[784]) { indexing: attribute }", "Placeholder", application); @@ -101,24 +99,21 @@ public class RankingExpressionWithOnnxTestCase { @Test void testNestedOnnxReference() { RankProfileSearchFixture search = fixtureWith("tensor(d0[1],d1[784])(0.0)", - "5 + sum(onnx_vespa('mnist_softmax.onnx'))", - vespaExpressionConstants); + "5 + sum(onnx_vespa('mnist_softmax.onnx'))"); search.assertFirstPhaseExpression("5 + reduce(" + vespaExpression + ", sum)", "my_profile"); } @Test void testOnnxReferenceWithSpecifiedOutput() { RankProfileSearchFixture search = fixtureWith("tensor(d0[1],d1[784])(0.0)", - "onnx_vespa('mnist_softmax.onnx', 'layer_add')", - vespaExpressionConstants); + "onnx_vespa('mnist_softmax.onnx', 'layer_add')"); search.assertFirstPhaseExpression(vespaExpression, "my_profile"); } @Test void testOnnxReferenceWithSpecifiedOutputAndSignature() { RankProfileSearchFixture search = fixtureWith("tensor(d0[1],d1[784])(0.0)", - "onnx_vespa('mnist_softmax.onnx', 'default.layer_add')", - vespaExpressionConstants); + "onnx_vespa('mnist_softmax.onnx', 'default.layer_add')"); search.assertFirstPhaseExpression(vespaExpression, "my_profile"); } @@ -182,8 +177,7 @@ public class RankingExpressionWithOnnxTestCase { @Test void testImportingFromStoredExpressions() throws IOException { RankProfileSearchFixture search = fixtureWith("tensor(d0[1],d1[784])(0.0)", - "onnx_vespa(\"mnist_softmax.onnx\")", - vespaExpressionConstants); + "onnx_vespa(\"mnist_softmax.onnx\")"); search.assertFirstPhaseExpression(vespaExpression, "my_profile"); // At this point the expression is stored - copy application to another location which do not have a models dir @@ -193,14 +187,12 @@ public class RankingExpressionWithOnnxTestCase { IOUtils.copyDirectory(applicationDir.append(ApplicationPackage.MODELS_GENERATED_DIR).toFile(), storedApplicationDirectory.append(ApplicationPackage.MODELS_GENERATED_DIR).toFile()); StoringApplicationPackage storedApplication = new StoringApplicationPackage(storedApplicationDirectory); - String constants = "constant mnist_softmax_layer_Variable { file: ignored\ntype: tensor(d0[2],d1[784]) }\n" + - "constant mnist_softmax_layer_Variable_1 { file: ignored\ntype: tensor(d0[2],d1[784]) }\n"; RankProfileSearchFixture searchFromStored = fixtureWith("tensor(d0[2],d1[784])(0.0)", - "onnx_vespa('mnist_softmax.onnx')", - constants, - null, - "Placeholder", - storedApplication); + "onnx_vespa('mnist_softmax.onnx')", + null, + null, + "Placeholder", + storedApplication); searchFromStored.assertFirstPhaseExpression(vespaExpression, "my_profile"); // Verify that the constants exists, but don't verify the content as we are not // simulating file distribution in this test @@ -229,8 +221,7 @@ public class RankingExpressionWithOnnxTestCase { String vespaExpressionWithoutConstant = "join(reduce(join(rename(Placeholder, (d0, d1), (d0, d2)), " + name + "_layer_Variable, f(a,b)(a * b)), sum, d2) * 1.0, constant(" + name + "_layer_Variable_1) * 1.0, f(a,b)(a + b))"; - String constant = "constant mnist_softmax_layer_Variable_1 { file: ignored\ntype: tensor(d0[1],d1[10]) }\n"; - RankProfileSearchFixture search = uncompiledFixtureWith(rankProfile, new StoringApplicationPackage(applicationDir), constant); + RankProfileSearchFixture search = uncompiledFixtureWith(rankProfile, new StoringApplicationPackage(applicationDir)); search.compileRankProfile("my_profile", applicationDir.append("models")); search.compileRankProfile("my_profile_child", applicationDir.append("models")); search.assertFirstPhaseExpression(vespaExpressionWithoutConstant, "my_profile"); @@ -246,7 +237,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, constant); + RankProfileSearchFixture searchFromStored = uncompiledFixtureWith(rankProfile, storedApplication); searchFromStored.compileRankProfile("my_profile", applicationDir.append("models")); searchFromStored.compileRankProfile("my_profile_child", applicationDir.append("models")); searchFromStored.assertFirstPhaseExpression(vespaExpressionWithoutConstant, "my_profile"); @@ -335,11 +326,7 @@ public class RankingExpressionWithOnnxTestCase { } private RankProfileSearchFixture fixtureWith(String placeholderExpression, String firstPhaseExpression) { - return fixtureWith(placeholderExpression, firstPhaseExpression, null); - } - - private RankProfileSearchFixture fixtureWith(String placeholderExpression, String firstPhaseExpression, String constant) { - return fixtureWith(placeholderExpression, firstPhaseExpression, constant, null, "Placeholder", + return fixtureWith(placeholderExpression, firstPhaseExpression, null, null, "Placeholder", new StoringApplicationPackage(applicationDir)); } @@ -350,13 +337,9 @@ public class RankingExpressionWithOnnxTestCase { } private RankProfileSearchFixture uncompiledFixtureWith(String rankProfile, StoringApplicationPackage application) { - return uncompiledFixtureWith(rankProfile, application, null); - } - - private RankProfileSearchFixture uncompiledFixtureWith(String rankProfile, StoringApplicationPackage application, String constant) { try { return new RankProfileSearchFixture(application, application.getQueryProfiles(), - rankProfile, constant, null); + rankProfile, null, null); } catch (ParseException e) { throw new IllegalArgumentException(e); -- cgit v1.2.3