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";

Reply via email to