You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@systemds.apache.org by ba...@apache.org on 2020/10/29 15:03:26 UTC

[systemds] branch master updated: [MINOR] Fix error in exportData

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 <ba...@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";