[SYSTEMML-1175] Fix robustness toString for scalars, tests The runtime instruction for toString does not support scalars, but because the ParameterizedCPInstruction requires instruction patching, the error message is unclear. This patch makes the parser more robust to compile toString only for matrices and frames but directly convert scalars to strings.
Project: http://git-wip-us.apache.org/repos/asf/systemml/repo Commit: http://git-wip-us.apache.org/repos/asf/systemml/commit/d03396f2 Tree: http://git-wip-us.apache.org/repos/asf/systemml/tree/d03396f2 Diff: http://git-wip-us.apache.org/repos/asf/systemml/diff/d03396f2 Branch: refs/heads/master Commit: d03396f20ecf3ed46e11fb8b3b960a54d94d329a Parents: 3737b9a Author: Matthias Boehm <[email protected]> Authored: Tue Jan 16 16:07:53 2018 -0800 Committer: Matthias Boehm <[email protected]> Committed: Tue Jan 16 16:08:10 2018 -0800 ---------------------------------------------------------------------- .../org/apache/sysml/parser/DMLTranslator.java | 142 +++++++++---------- .../functions/jmlc/InputToStringTest.java | 68 +++++++++ .../functions/jmlc/ZPackageSuite.java | 1 + 3 files changed, 138 insertions(+), 73 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/systemml/blob/d03396f2/src/main/java/org/apache/sysml/parser/DMLTranslator.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/sysml/parser/DMLTranslator.java b/src/main/java/org/apache/sysml/parser/DMLTranslator.java index 9d88ff7..76ea23d 100644 --- a/src/main/java/org/apache/sysml/parser/DMLTranslator.java +++ b/src/main/java/org/apache/sysml/parser/DMLTranslator.java @@ -2100,87 +2100,83 @@ public class DMLTranslator // construct hop based on opcode switch(source.getOpCode()) { - case CDF: - case INVCDF: - case QNORM: - case QT: - case QF: - case QCHISQ: - case QEXP: - case PNORM: - case PT: - case PF: - case PCHISQ: - case PEXP: - currBuiltinOp = constructDfHop(target.getName(), target.getDataType(), target.getValueType(), source.getOpCode(), paramHops); - break; + case CDF: + case INVCDF: + case QNORM: + case QT: + case QF: + case QCHISQ: + case QEXP: + case PNORM: + case PT: + case PF: + case PCHISQ: + case PEXP: + currBuiltinOp = constructDfHop(target.getName(), target.getDataType(), target.getValueType(), source.getOpCode(), paramHops); + break; - case GROUPEDAGG: - currBuiltinOp = new ParameterizedBuiltinOp( - target.getName(), target.getDataType(), target.getValueType(), ParamBuiltinOp.GROUPEDAGG, paramHops); - break; - - case RMEMPTY: - currBuiltinOp = new ParameterizedBuiltinOp( - target.getName(), target.getDataType(), target.getValueType(), ParamBuiltinOp.RMEMPTY, paramHops); - break; + case GROUPEDAGG: + currBuiltinOp = new ParameterizedBuiltinOp( + target.getName(), target.getDataType(), target.getValueType(), ParamBuiltinOp.GROUPEDAGG, paramHops); + break; - case REPLACE: - currBuiltinOp = new ParameterizedBuiltinOp( - target.getName(), target.getDataType(), target.getValueType(), ParamBuiltinOp.REPLACE, paramHops); - break; + case RMEMPTY: + currBuiltinOp = new ParameterizedBuiltinOp( + target.getName(), target.getDataType(), target.getValueType(), ParamBuiltinOp.RMEMPTY, paramHops); + break; - case ORDER: - ArrayList<Hop> inputs = new ArrayList<>(); - inputs.add(paramHops.get("target")); - inputs.add(paramHops.get("by")); - inputs.add(paramHops.get("decreasing")); - inputs.add(paramHops.get("index.return")); + case REPLACE: + currBuiltinOp = new ParameterizedBuiltinOp( + target.getName(), target.getDataType(), target.getValueType(), ParamBuiltinOp.REPLACE, paramHops); + break; - currBuiltinOp = new ReorgOp(target.getName(), target.getDataType(), target.getValueType(), ReOrgOp.SORT, inputs); + case ORDER: + ArrayList<Hop> inputs = new ArrayList<>(); + inputs.add(paramHops.get("target")); + inputs.add(paramHops.get("by")); + inputs.add(paramHops.get("decreasing")); + inputs.add(paramHops.get("index.return")); + + currBuiltinOp = new ReorgOp(target.getName(), target.getDataType(), target.getValueType(), ReOrgOp.SORT, inputs); + + break; - break; - - case TRANSFORMAPPLY: - currBuiltinOp = new ParameterizedBuiltinOp( - target.getName(), target.getDataType(), target.getValueType(), - ParamBuiltinOp.TRANSFORMAPPLY, paramHops); - break; - - case TRANSFORMDECODE: - currBuiltinOp = new ParameterizedBuiltinOp( - target.getName(), target.getDataType(), target.getValueType(), - ParamBuiltinOp.TRANSFORMDECODE, paramHops); - break; - - case TRANSFORMCOLMAP: - currBuiltinOp = new ParameterizedBuiltinOp( - target.getName(), target.getDataType(), target.getValueType(), - ParamBuiltinOp.TRANSFORMCOLMAP, paramHops); - break; - - case TRANSFORMMETA: - currBuiltinOp = new ParameterizedBuiltinOp( - target.getName(), target.getDataType(), target.getValueType(), - ParamBuiltinOp.TRANSFORMMETA, paramHops); - break; - - case TOSTRING: - currBuiltinOp = new ParameterizedBuiltinOp( - target.getName(), target.getDataType(), - target.getValueType(), ParamBuiltinOp.TOSTRING, - paramHops); - break; + case TRANSFORMAPPLY: + currBuiltinOp = new ParameterizedBuiltinOp( + target.getName(), target.getDataType(), target.getValueType(), + ParamBuiltinOp.TRANSFORMAPPLY, paramHops); + break; - default: + case TRANSFORMDECODE: + currBuiltinOp = new ParameterizedBuiltinOp( + target.getName(), target.getDataType(), target.getValueType(), + ParamBuiltinOp.TRANSFORMDECODE, paramHops); + break; - LOG.error(source.printErrorLocation() + - "processParameterizedBuiltinFunctionExpression() -- Unknown operation: " - + source.getOpCode()); + case TRANSFORMCOLMAP: + currBuiltinOp = new ParameterizedBuiltinOp( + target.getName(), target.getDataType(), target.getValueType(), + ParamBuiltinOp.TRANSFORMCOLMAP, paramHops); + break; - throw new ParseException(source.printErrorLocation() + - "processParameterizedBuiltinFunctionExpression() -- Unknown operation: " - + source.getOpCode()); + case TRANSFORMMETA: + currBuiltinOp = new ParameterizedBuiltinOp( + target.getName(), target.getDataType(), target.getValueType(), + ParamBuiltinOp.TRANSFORMMETA, paramHops); + break; + + case TOSTRING: + //check for input data type and only compile toString Hop for matrices/frames, + //for scalars, we compile (s + "") to ensure consistent string output value types + currBuiltinOp = !paramHops.get("target").getDataType().isScalar() ? + new ParameterizedBuiltinOp(target.getName(), target.getDataType(), + target.getValueType(), ParamBuiltinOp.TOSTRING, paramHops) : + HopRewriteUtils.createBinary(paramHops.get("target"), new LiteralOp(""), OpOp2.PLUS); + break; + + default: + throw new ParseException(source.printErrorLocation() + + "processParameterizedBuiltinFunctionExpression() -- Unknown operation: " + source.getOpCode()); } setIdentifierParams(currBuiltinOp, source.getOutput()); http://git-wip-us.apache.org/repos/asf/systemml/blob/d03396f2/src/test/java/org/apache/sysml/test/integration/functions/jmlc/InputToStringTest.java ---------------------------------------------------------------------- diff --git a/src/test/java/org/apache/sysml/test/integration/functions/jmlc/InputToStringTest.java b/src/test/java/org/apache/sysml/test/integration/functions/jmlc/InputToStringTest.java new file mode 100644 index 0000000..21d0973 --- /dev/null +++ b/src/test/java/org/apache/sysml/test/integration/functions/jmlc/InputToStringTest.java @@ -0,0 +1,68 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.sysml.test.integration.functions.jmlc; + +import org.apache.sysml.api.DMLException; +import org.apache.sysml.api.jmlc.Connection; +import org.apache.sysml.api.jmlc.PreparedScript; +import org.apache.sysml.runtime.matrix.data.MatrixBlock; +import org.apache.sysml.runtime.util.DataConverter; +import org.apache.sysml.test.integration.AutomatedTestBase; +import org.junit.Test; + +public class InputToStringTest extends AutomatedTestBase +{ + @Override + public void setUp() { } + + @Test + public void testScalartoString() throws DMLException { + try( Connection conn = new Connection() ) { + PreparedScript pscript = conn.prepareScript( + "s = read(\"tmp\", data_type=\"scalar\"); print(toString(s));", + new String[]{"s"}, new String[]{}); + pscript.setScalar("s", 7); + pscript.executeScript(); + } + } + + @Test + public void testMatrixtoString() throws DMLException { + try( Connection conn = new Connection() ) { + PreparedScript pscript = conn.prepareScript( + "m = read(\"tmp\", data_type=\"matrix\"); print(toString(m));", + new String[]{"m"}, new String[]{}); + pscript.setMatrix("m", MatrixBlock.randOperations(7, 3, 1.0, 0, 1, "uniform", 7), false); + pscript.executeScript(); + } + } + + @Test + public void testFrametoString() throws DMLException { + try( Connection conn = new Connection() ) { + PreparedScript pscript = conn.prepareScript( + "f = read(\"tmp\", data_type=\"frame\"); print(toString(f));", + new String[]{"f"}, new String[]{}); + pscript.setFrame("f", DataConverter.convertToFrameBlock( + MatrixBlock.randOperations(7, 3, 1.0, 0, 1, "uniform", 7)), false); + pscript.executeScript(); + } + } +} http://git-wip-us.apache.org/repos/asf/systemml/blob/d03396f2/src/test_suites/java/org/apache/sysml/test/integration/functions/jmlc/ZPackageSuite.java ---------------------------------------------------------------------- diff --git a/src/test_suites/java/org/apache/sysml/test/integration/functions/jmlc/ZPackageSuite.java b/src/test_suites/java/org/apache/sysml/test/integration/functions/jmlc/ZPackageSuite.java index 05fa8bc..6c0135c 100644 --- a/src/test_suites/java/org/apache/sysml/test/integration/functions/jmlc/ZPackageSuite.java +++ b/src/test_suites/java/org/apache/sysml/test/integration/functions/jmlc/ZPackageSuite.java @@ -34,6 +34,7 @@ import org.junit.runners.Suite; FrameLeftIndexingTest.class, FrameReadMetaTest.class, FrameTransformTest.class, + InputToStringTest.class, JMLCClonedPreparedScriptTest.class, JMLCInputOutputTest.class, JMLCInputStreamReadTest.class,
