This is an automated email from the ASF dual-hosted git repository. baunsgaard pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/systemds.git
The following commit(s) were added to refs/heads/master by this push: new 966908b [MINOR] Fix error in exportData 966908b is described below commit 966908b60c40861a2bec84607510a4163f75396f Author: baunsgaard <baunsga...@tugraz.at> AuthorDate: Thu Oct 29 15:59:08 2020 +0100 [MINOR] Fix error in exportData Fix error in logic of export data, introduces while making federated write instruction. This error resulted in null matrices, if the matrices were not fully generated, which some of our tests exploit. --- .../controlprogram/caching/CacheableData.java | 47 +++++++++++----------- .../instructions/fed/VariableFEDInstruction.java | 2 +- .../federated/io/FederatedWriterTest.java | 15 ++++--- 3 files changed, 31 insertions(+), 33 deletions(-) diff --git a/src/main/java/org/apache/sysds/runtime/controlprogram/caching/CacheableData.java b/src/main/java/org/apache/sysds/runtime/controlprogram/caching/CacheableData.java index e598493..a646d64 100644 --- a/src/main/java/org/apache/sysds/runtime/controlprogram/caching/CacheableData.java +++ b/src/main/java/org/apache/sysds/runtime/controlprogram/caching/CacheableData.java @@ -482,7 +482,6 @@ public abstract class CacheableData<T extends CacheBlock> extends Data if( _data==null && isEmpty(true) ) { try { if( isFederated() ) { - LOG.error("Federated pull all data"); _data = readBlobFromFederated( _fedMapping ); //mark for initial local write despite read operation @@ -783,35 +782,35 @@ public abstract class CacheableData<T extends CacheBlock> extends Data { // CASE 1: dirty in-mem matrix or pWrite w/ different format (write matrix to fname; load into memory if evicted) // a) get the matrix - boolean federatedWrite = outputFormat.contains("federated"); - if( ! federatedWrite){ + boolean federatedWrite = (outputFormat != null ) && outputFormat.contains("federated"); - if( isEmpty(true)) + if( isEmpty(true)) + { + //read data from HDFS if required (never read before), this applies only to pWrite w/ different output formats + //note: for large rdd outputs, we compile dedicated writespinstructions (no need to handle this here) + try { - //read data from HDFS if required (never read before), this applies only to pWrite w/ different output formats - //note: for large rdd outputs, we compile dedicated writespinstructions (no need to handle this here) - try - { - if( getRDDHandle()==null || getRDDHandle().allowsShortCircuitRead() ) - _data = readBlobFromHDFS( _hdfsFileName ); - else if( getRDDHandle() != null ) - _data = readBlobFromRDD( getRDDHandle(), new MutableBoolean() ); - else { - _data = readBlobFromFederated( getFedMapping() ); - } - - setDirty(false); - } - catch (IOException e) { - throw new DMLRuntimeException("Reading of " + _hdfsFileName + " ("+hashCode()+") failed.", e); - } + if( getRDDHandle()==null || getRDDHandle().allowsShortCircuitRead() ) + _data = readBlobFromHDFS( _hdfsFileName ); + else if( getRDDHandle() != null ) + _data = readBlobFromRDD( getRDDHandle(), new MutableBoolean() ); + else if(!federatedWrite) + _data = readBlobFromFederated( getFedMapping() ); + + setDirty(false); + } + catch (IOException e) { + throw new DMLRuntimeException("Reading of " + _hdfsFileName + " ("+hashCode()+") failed.", e); } - //get object from cache - if( _data == null ) + } + //get object from cache + if(!federatedWrite){ + + if( _data == null ) getCache(); acquire( false, _data==null ); //incl. read matrix if evicted } - + // b) write the matrix try { writeMetaData( fName, outputFormat, formatProperties ); diff --git a/src/main/java/org/apache/sysds/runtime/instructions/fed/VariableFEDInstruction.java b/src/main/java/org/apache/sysds/runtime/instructions/fed/VariableFEDInstruction.java index b425dee..793ba59 100644 --- a/src/main/java/org/apache/sysds/runtime/instructions/fed/VariableFEDInstruction.java +++ b/src/main/java/org/apache/sysds/runtime/instructions/fed/VariableFEDInstruction.java @@ -58,7 +58,7 @@ public class VariableFEDInstruction extends FEDInstruction implements LineageTra } private void processWriteInstruction(ExecutionContext ec) { - LOG.error("processing write command federated"); + LOG.warn("Processing write command federated"); // TODO Add write command to the federated site if the matrix has been modified // this has to be done while appending some string to the federated output file. // furthermore the outputted file on the federated sites path should be returned diff --git a/src/test/java/org/apache/sysds/test/functions/federated/io/FederatedWriterTest.java b/src/test/java/org/apache/sysds/test/functions/federated/io/FederatedWriterTest.java index 9a913b9..ef92a67 100644 --- a/src/test/java/org/apache/sysds/test/functions/federated/io/FederatedWriterTest.java +++ b/src/test/java/org/apache/sysds/test/functions/federated/io/FederatedWriterTest.java @@ -21,8 +21,6 @@ package org.apache.sysds.test.functions.federated.io; import java.util.Arrays; import java.util.Collection; -import org.apache.commons.logging.Log; -import org.apache.commons.logging.LogFactory; import org.apache.sysds.api.DMLScript; import org.apache.sysds.common.Types; import org.apache.sysds.runtime.meta.MatrixCharacteristics; @@ -38,7 +36,7 @@ import org.junit.runners.Parameterized; @net.jcip.annotations.NotThreadSafe public class FederatedWriterTest extends AutomatedTestBase { - private static final Log LOG = LogFactory.getLog(FederatedWriterTest.class.getName()); + // private static final Log LOG = LogFactory.getLog(FederatedWriterTest.class.getName()); private final static String TEST_DIR = "functions/federated/"; private final static String TEST_NAME = "FederatedWriterTest"; private final static String TEST_CLASS_DIR = TEST_DIR + FederatedWriterTest.class.getSimpleName() + "/"; @@ -97,11 +95,12 @@ public class FederatedWriterTest extends AutomatedTestBase { // Run reader and write a federated json to enable the rest of the test fullDMLScriptName = SCRIPT_DIR + "functions/federated/io/FederatedReaderTestCreate.dml"; - programArgs = new String[] {"-stats", "-explain","-args", input("X1"), input("X2"), port1 + "", port2 + "", input("X.json")}; - String writer = runTest(null).toString(); - // runTest(null); - LOG.error(writer); - LOG.error("Writing Done"); + programArgs = new String[] {"-stats", "-explain", "-args", input("X1"), input("X2"), port1 + "", port2 + "", + input("X.json")}; + // String writer = runTest(null).toString(); + runTest(null); + // LOG.error(writer); + // LOG.error("Writing Done"); // Run reference dml script with normal matrix fullDMLScriptName = SCRIPT_DIR + "functions/federated/io/FederatedReaderTest.dml";