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 2022/07/25 10:54:34 UTC

[systemds] branch main updated: [SYSTEMDS-3411] Python configuration not loading defaults

This is an automated email from the ASF dual-hosted git repository.

baunsgaard pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/systemds.git


The following commit(s) were added to refs/heads/main by this push:
     new 1db2a0f07c [SYSTEMDS-3411] Python configuration not loading defaults
1db2a0f07c is described below

commit 1db2a0f07c85586fabfe68d7aaae9d15f7b8b65c
Author: Kevin Innerebner <ke...@gmail.com>
AuthorDate: Wed Jul 20 15:12:34 2022 +0200

    [SYSTEMDS-3411] Python configuration not loading defaults
    
    Fixes a bug where instructions were not replaced by FED equivalent
    instructions, because the correct `CompilerConfig` option was not set.
    And remove unnecessary CompilerConfigs
    
    Closes #1667
---
 .../java/org/apache/sysds/api/PythonDMLScript.java |  9 +----
 .../java/org/apache/sysds/api/jmlc/Connection.java | 41 +++++++++++-----------
 .../jmlc/JMLCClonedPreparedScriptTest.java         |  2 ++
 .../functions/jmlc/JMLCParfor2ForCompileTest.java  |  4 +--
 4 files changed, 25 insertions(+), 31 deletions(-)

diff --git a/src/main/java/org/apache/sysds/api/PythonDMLScript.java b/src/main/java/org/apache/sysds/api/PythonDMLScript.java
index d93409d8b2..03ff025892 100644
--- a/src/main/java/org/apache/sysds/api/PythonDMLScript.java
+++ b/src/main/java/org/apache/sysds/api/PythonDMLScript.java
@@ -55,14 +55,7 @@ public class PythonDMLScript {
 		// we enable multi-threaded I/O and operations for a single JMLC
 		// connection because the calling Python process is unlikely to run
 		// multi-threaded streams of operations on the same shared context
-		_connection = new Connection(
-			CompilerConfig.ConfigType.PARALLEL_CP_READ_TEXTFORMATS,
-			CompilerConfig.ConfigType.PARALLEL_CP_WRITE_TEXTFORMATS,
-			CompilerConfig.ConfigType.PARALLEL_CP_READ_BINARYFORMATS,
-			CompilerConfig.ConfigType.PARALLEL_CP_WRITE_BINARYFORMATS,
-			CompilerConfig.ConfigType.PARALLEL_CP_MATRIX_OPERATIONS,
-			CompilerConfig.ConfigType.PARALLEL_LOCAL_OR_REMOTE_PARFOR,
-			CompilerConfig.ConfigType.ALLOW_DYN_RECOMPILATION);
+		_connection = new Connection();
 	}
 
 	public Connection getConnection() {
diff --git a/src/main/java/org/apache/sysds/api/jmlc/Connection.java b/src/main/java/org/apache/sysds/api/jmlc/Connection.java
index 1c4bada33e..7037e2172b 100644
--- a/src/main/java/org/apache/sysds/api/jmlc/Connection.java
+++ b/src/main/java/org/apache/sysds/api/jmlc/Connection.java
@@ -31,6 +31,7 @@ import java.util.Map;
 
 import org.apache.hadoop.fs.FileSystem;
 import org.apache.hadoop.fs.Path;
+import org.apache.sysds.hops.OptimizerUtils;
 import org.apache.sysds.runtime.meta.MetaDataAll;
 import org.apache.sysds.api.DMLException;
 import org.apache.sysds.api.DMLScript;
@@ -111,8 +112,7 @@ public class Connection implements Closeable
 		this(new DMLConfig()); //with default dml configuration
 		
 		//set optional compiler configurations in current config
-		for( ConfigType configType : cconfigs )
-			_cconf.set(configType, true);
+		setConfigTypes(true, cconfigs);
 		setLocalConfigs();
 	}
 	
@@ -129,8 +129,7 @@ public class Connection implements Closeable
 		this(dmlconfig); 
 		
 		//set optional compiler configurations in current config
-		for( ConfigType configType : cconfigs )
-			_cconf.set(configType, true);
+		setConfigTypes(true, cconfigs);
 		setLocalConfigs();
 	}
 	
@@ -145,23 +144,13 @@ public class Connection implements Closeable
 		
 		//setup basic parameters for embedded execution
 		//(parser, compiler, and runtime parameters)
-		CompilerConfig cconf = new CompilerConfig();
-		cconf.set(ConfigType.IGNORE_UNSPECIFIED_ARGS, true);
-		cconf.set(ConfigType.IGNORE_READ_WRITE_METADATA, true);
-		cconf.set(ConfigType.IGNORE_TEMPORARY_FILENAMES, true);
-		cconf.set(ConfigType.REJECT_READ_WRITE_UNKNOWNS, false);
-		cconf.set(ConfigType.PARALLEL_CP_READ_TEXTFORMATS, false);
-		cconf.set(ConfigType.PARALLEL_CP_WRITE_TEXTFORMATS, false);
-		cconf.set(ConfigType.PARALLEL_CP_READ_BINARYFORMATS, false);
-		cconf.set(ConfigType.PARALLEL_CP_WRITE_BINARYFORMATS, false);
-		cconf.set(ConfigType.PARALLEL_CP_MATRIX_OPERATIONS, false);
-		cconf.set(ConfigType.PARALLEL_LOCAL_OR_REMOTE_PARFOR, false);
-		cconf.set(ConfigType.ALLOW_DYN_RECOMPILATION, false);
-		cconf.set(ConfigType.ALLOW_INDIVIDUAL_SB_SPECIFIC_OPS, false);
-		cconf.set(ConfigType.ALLOW_CSE_PERSISTENT_READS, false);
-		cconf.set(ConfigType.CODEGEN_ENABLED, false);
-		_cconf = cconf;
-		
+		_cconf = OptimizerUtils.constructCompilerConfig(dmlconfig);
+		_cconf.set(ConfigType.IGNORE_UNSPECIFIED_ARGS, true);
+		_cconf.set(ConfigType.IGNORE_READ_WRITE_METADATA, true);
+		_cconf.set(ConfigType.IGNORE_TEMPORARY_FILENAMES, true);
+		_cconf.set(ConfigType.REJECT_READ_WRITE_UNKNOWNS, false);
+		_cconf.set(ConfigType.ALLOW_CSE_PERSISTENT_READS, false);
+
 		//disable caching globally 
 		CacheableData.disableCaching();
 		
@@ -171,6 +160,16 @@ public class Connection implements Closeable
 		setLocalConfigs();
 	}
 
+	/**
+	 * Sets compiler configs.
+	 * @param activate activate or disable
+	 * @param cconfigs the configs to set
+	 */
+	public void setConfigTypes(boolean activate, CompilerConfig.ConfigType... cconfigs) {
+		for( ConfigType configType : cconfigs )
+			_cconf.set(configType, activate);
+	}
+
 	/**
 	 * Sets a boolean flag indicating if runtime statistics should be gathered
 	 * Same behavior as in "MLContext.setStatistics()"
diff --git a/src/test/java/org/apache/sysds/test/functions/jmlc/JMLCClonedPreparedScriptTest.java b/src/test/java/org/apache/sysds/test/functions/jmlc/JMLCClonedPreparedScriptTest.java
index 8d98436e25..9409584131 100644
--- a/src/test/java/org/apache/sysds/test/functions/jmlc/JMLCClonedPreparedScriptTest.java
+++ b/src/test/java/org/apache/sysds/test/functions/jmlc/JMLCClonedPreparedScriptTest.java
@@ -26,6 +26,7 @@ import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
 import java.util.concurrent.Future;
 
+import org.apache.sysds.conf.CompilerConfig;
 import org.junit.Assert;
 import org.junit.Test;
 import org.apache.sysds.api.DMLException;
@@ -94,6 +95,7 @@ public class JMLCClonedPreparedScriptTest extends AutomatedTestBase
 		
 		boolean failed = false;
 		try( Connection conn = new Connection() ) {
+			conn.setConfigTypes(false, CompilerConfig.ConfigType.PARALLEL_LOCAL_OR_REMOTE_PARFOR);
 			DMLScript.STATISTICS = true;
 			Statistics.reset();
 			PreparedScript pscript = conn.prepareScript(
diff --git a/src/test/java/org/apache/sysds/test/functions/jmlc/JMLCParfor2ForCompileTest.java b/src/test/java/org/apache/sysds/test/functions/jmlc/JMLCParfor2ForCompileTest.java
index 1c163ac0bf..18e1fc5ad5 100644
--- a/src/test/java/org/apache/sysds/test/functions/jmlc/JMLCParfor2ForCompileTest.java
+++ b/src/test/java/org/apache/sysds/test/functions/jmlc/JMLCParfor2ForCompileTest.java
@@ -48,8 +48,8 @@ public class JMLCParfor2ForCompileTest extends AutomatedTestBase
 
 	private static void runJMLCParFor2ForTest(boolean par) {
 		try {
-			Connection conn = !par ? new Connection() :
-				new Connection(ConfigType.PARALLEL_LOCAL_OR_REMOTE_PARFOR);
+			Connection conn = new Connection();
+			conn.setConfigTypes(par, ConfigType.PARALLEL_LOCAL_OR_REMOTE_PARFOR);
 			String script =
 				"  X = rand(rows=10, cols=10);"
 				+ "R = matrix(0, rows=10, cols=1)"