Repository: incubator-systemml Updated Branches: refs/heads/master 5744e752d -> 9820f4c52
[SYSTEMML-1473] Fix correctness codegen cellwise min/max aggregation The cellwise codegen template allows for sum, sum_sq, min and max as aggregation functions. In case of multi-threaded execution there is a need for a final aggregation of partial results. This patch fixes resulting correctness issues for min and max in such scenarios. Project: http://git-wip-us.apache.org/repos/asf/incubator-systemml/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-systemml/commit/7c153390 Tree: http://git-wip-us.apache.org/repos/asf/incubator-systemml/tree/7c153390 Diff: http://git-wip-us.apache.org/repos/asf/incubator-systemml/diff/7c153390 Branch: refs/heads/master Commit: 7c15339019360f3add4aac23b06cde45a4b758f8 Parents: f2ea633 Author: Matthias Boehm <[email protected]> Authored: Thu Apr 6 18:51:45 2017 -0700 Committer: Matthias Boehm <[email protected]> Committed: Thu Apr 6 21:16:30 2017 -0700 ---------------------------------------------------------------------- .../sysml/runtime/codegen/SpoofCellwise.java | 25 +++++++++------ .../functions/codegen/CellwiseTmplTest.java | 16 ++++++++-- .../scripts/functions/codegen/cellwisetmpl13.R | 33 ++++++++++++++++++++ .../functions/codegen/cellwisetmpl13.dml | 29 +++++++++++++++++ 4 files changed, 92 insertions(+), 11 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-systemml/blob/7c153390/src/main/java/org/apache/sysml/runtime/codegen/SpoofCellwise.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/sysml/runtime/codegen/SpoofCellwise.java b/src/main/java/org/apache/sysml/runtime/codegen/SpoofCellwise.java index 67dd346..12d14ad 100644 --- a/src/main/java/org/apache/sysml/runtime/codegen/SpoofCellwise.java +++ b/src/main/java/org/apache/sysml/runtime/codegen/SpoofCellwise.java @@ -122,10 +122,10 @@ public abstract class SpoofCellwise extends SpoofOperator implements Serializabl boolean sparseSafe = isSparseSafe() || (b.length == 0 && genexec( 0, b, scalars, m, n, 0, 0 ) == 0); - double sum = 0; + double ret = 0; if( k <= 1 ) //SINGLE-THREADED { - sum = ( !inputs.get(0).isInSparseFormat() ) ? + ret = ( !inputs.get(0).isInSparseFormat() ) ? executeDenseAndAgg(inputs.get(0).getDenseBlock(), b, scalars, m, n, sparseSafe, 0, m) : executeSparseAndAgg(inputs.get(0).getSparseBlock(), b, scalars, m, n, sparseSafe, 0, m); } @@ -143,11 +143,18 @@ public abstract class SpoofCellwise extends SpoofOperator implements Serializabl pool.shutdown(); //aggregate partial results - KahanObject kbuff = new KahanObject(0, 0); - KahanPlus kplus = KahanPlus.getKahanPlusFnObject(); - for( Future<Double> task : taskret ) - kplus.execute2(kbuff, task.get()); - sum = kbuff._sum; + ValueFunction vfun = getAggFunction(); + if( vfun instanceof KahanFunction ) { + KahanObject kbuff = new KahanObject(0, 0); + KahanPlus kplus = KahanPlus.getKahanPlusFnObject(); + for( Future<Double> task : taskret ) + kplus.execute2(kbuff, task.get()); + ret = kbuff._sum; + } + else { + for( Future<Double> task : taskret ) + ret = vfun.execute(ret, task.get()); + } } catch(Exception ex) { throw new DMLRuntimeException(ex); @@ -157,9 +164,9 @@ public abstract class SpoofCellwise extends SpoofOperator implements Serializabl //correction for min/max if( (_aggOp == AggOp.MIN || _aggOp == AggOp.MAX) && sparseSafe && inputs.get(0).getNonZeros()<inputs.get(0).getNumRows()*inputs.get(0).getNumColumns() ) - sum = getAggFunction().execute(sum, 0); //unseen 0 might be max or min value + ret = getAggFunction().execute(ret, 0); //unseen 0 might be max or min value - return new DoubleObject(sum); + return new DoubleObject(ret); } @Override http://git-wip-us.apache.org/repos/asf/incubator-systemml/blob/7c153390/src/test/java/org/apache/sysml/test/integration/functions/codegen/CellwiseTmplTest.java ---------------------------------------------------------------------- diff --git a/src/test/java/org/apache/sysml/test/integration/functions/codegen/CellwiseTmplTest.java b/src/test/java/org/apache/sysml/test/integration/functions/codegen/CellwiseTmplTest.java index 6468003..fc41837 100644 --- a/src/test/java/org/apache/sysml/test/integration/functions/codegen/CellwiseTmplTest.java +++ b/src/test/java/org/apache/sysml/test/integration/functions/codegen/CellwiseTmplTest.java @@ -48,7 +48,9 @@ public class CellwiseTmplTest extends AutomatedTestBase private static final String TEST_NAME10 = TEST_NAME+10; //min/max(X + 7 * Y) private static final String TEST_NAME11 = TEST_NAME+11; //replace((0 / (X - 500))+1, 0/0, 7) private static final String TEST_NAME12 = TEST_NAME+12; //((X/3) %% 0.6) + ((X/3) %/% 0.6) - + private static final String TEST_NAME13 = TEST_NAME+13; //min(X + 7 * Y) large + + private static final String TEST_DIR = "functions/codegen/"; private static final String TEST_CLASS_DIR = TEST_DIR + CellwiseTmplTest.class.getSimpleName() + "/"; private final static String TEST_CONF6 = "SystemML-config-codegen6.xml"; @@ -60,7 +62,7 @@ public class CellwiseTmplTest extends AutomatedTestBase @Override public void setUp() { TestUtils.clearAssertionInformation(); - for( int i=1; i<=12; i++ ) { + for( int i=1; i<=13; i++ ) { addTestConfiguration( TEST_NAME+i, new TestConfiguration( TEST_CLASS_DIR, TEST_NAME+i, new String[] {String.valueOf(i)}) ); } @@ -126,6 +128,11 @@ public class CellwiseTmplTest extends AutomatedTestBase public void testCodegenCellwiseRewrite12() { testCodegenIntegration( TEST_NAME12, true, ExecType.CP ); } + + @Test + public void testCodegenCellwiseRewrite13() { + testCodegenIntegration( TEST_NAME13, true, ExecType.CP ); + } @Test public void testCodegenCellwise1() { @@ -187,6 +194,11 @@ public class CellwiseTmplTest extends AutomatedTestBase public void testCodegenCellwise12() { testCodegenIntegration( TEST_NAME12, false, ExecType.CP ); } + + @Test + public void testCodegenCellwise13() { + testCodegenIntegration( TEST_NAME13, false, ExecType.CP ); + } @Test public void testCodegenCellwiseRewrite1_sp() { http://git-wip-us.apache.org/repos/asf/incubator-systemml/blob/7c153390/src/test/scripts/functions/codegen/cellwisetmpl13.R ---------------------------------------------------------------------- diff --git a/src/test/scripts/functions/codegen/cellwisetmpl13.R b/src/test/scripts/functions/codegen/cellwisetmpl13.R new file mode 100644 index 0000000..527bb13 --- /dev/null +++ b/src/test/scripts/functions/codegen/cellwisetmpl13.R @@ -0,0 +1,33 @@ +#------------------------------------------------------------- +# +# 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. +# +#------------------------------------------------------------- + +args<-commandArgs(TRUE) +options(digits=22) +library("Matrix") + +X = matrix(seq(7, 1100006), 1100, 1000, byrow=TRUE); +Y = matrix(seq(6, 1100005), 1100, 1000, byrow=TRUE); + +Z = X + -7 * Y; +R1 = min(Z); +R = as.matrix(R1); + +writeMM(as(R,"CsparseMatrix"), paste(args[2], "S", sep="")); http://git-wip-us.apache.org/repos/asf/incubator-systemml/blob/7c153390/src/test/scripts/functions/codegen/cellwisetmpl13.dml ---------------------------------------------------------------------- diff --git a/src/test/scripts/functions/codegen/cellwisetmpl13.dml b/src/test/scripts/functions/codegen/cellwisetmpl13.dml new file mode 100644 index 0000000..58037dd --- /dev/null +++ b/src/test/scripts/functions/codegen/cellwisetmpl13.dml @@ -0,0 +1,29 @@ +#------------------------------------------------------------- +# +# 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. +# +#------------------------------------------------------------- + +X = matrix(seq(7, 1100006), 1100, 1000); +Y = matrix(seq(6, 1100005), 1100, 1000); + +Z = X + -7 * Y; +R1 = min(Z); +R = as.matrix(R1); + +write(R, $1)
