You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@systemml.apache.org by du...@apache.org on 2016/04/14 20:21:11 UTC

incubator-systemml git commit: [SYSTEMML-495] Simplify SystemML Configuration Loading

Repository: incubator-systemml
Updated Branches:
  refs/heads/master 4664f2f7b -> 6aef70cfb


[SYSTEMML-495] Simplify SystemML Configuration Loading

Always looking for a default file, even when a file is explicitly supplied, causes confusion, and may create unintended side effects.  If a user explicitly supplies a configuration file, we should simply use that file, rather than attempting to also read an additional default file at the same time.  In order to provide backwards compatibility, if the user does not supply a file, then we should fall back to the default file location.  Note that we always use the internal defaults first, and only overwrite those settings with any settings in a specified file or a default file.

Additionally, in order to supply an additional configuration file, the `-config=path/to/file` flag has to be used, and it only works with the equals sign `=` present, which is unlike all other flags that use spaces.  Instead, we now support `-config path/to/file`.

Closes #58.


Project: http://git-wip-us.apache.org/repos/asf/incubator-systemml/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-systemml/commit/6aef70cf
Tree: http://git-wip-us.apache.org/repos/asf/incubator-systemml/tree/6aef70cf
Diff: http://git-wip-us.apache.org/repos/asf/incubator-systemml/diff/6aef70cf

Branch: refs/heads/master
Commit: 6aef70cfbdf2de15eeeeccff57128994a7270719
Parents: 4664f2f
Author: Mike Dusenberry <mw...@us.ibm.com>
Authored: Thu Apr 14 11:20:50 2016 -0700
Committer: Mike Dusenberry <mw...@us.ibm.com>
Committed: Thu Apr 14 11:20:50 2016 -0700

----------------------------------------------------------------------
 .../java/org/apache/sysml/api/DMLScript.java    | 16 +++--
 .../java/org/apache/sysml/conf/DMLConfig.java   | 74 ++++++++------------
 2 files changed, 37 insertions(+), 53 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-systemml/blob/6aef70cf/src/main/java/org/apache/sysml/api/DMLScript.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/sysml/api/DMLScript.java b/src/main/java/org/apache/sysml/api/DMLScript.java
index ed78889..ce58974 100644
--- a/src/main/java/org/apache/sysml/api/DMLScript.java
+++ b/src/main/java/org/apache/sysml/api/DMLScript.java
@@ -294,8 +294,10 @@ public class DMLScript
 					if( rtplatform==null ) 
 						return false;
 				}
-				else if (args[i].startsWith("-config="))
-					fnameOptConfig = args[i].substring(8).replaceAll("\"", ""); 
+				else if (args[i].startsWith("-config=")) // legacy
+					fnameOptConfig = args[i].substring(8).replaceAll("\"", "");
+				else if (args[i].equalsIgnoreCase("-config"))
+					fnameOptConfig = args[++i];
 				else if( args[i].equalsIgnoreCase("-debug") ) {					
 					ENABLE_DEBUG_MODE = true;
 				}
@@ -567,12 +569,12 @@ public class DMLScript
 		printStartExecInfo( dmlScriptStr );
 		
 		//Step 1: parse configuration files
-		DMLConfig dmlconf = DMLConfig.readAndMergeConfigurationFiles(fnameOptConfig);
+		DMLConfig dmlconf = DMLConfig.readConfigurationFile(fnameOptConfig);
 		ConfigurationManager.setGlobalConfig(dmlconf);		
 		CompilerConfig cconf = OptimizerUtils.constructCompilerConfig(dmlconf);
 		ConfigurationManager.setGlobalConfig(cconf);
 		LOG.debug("\nDML config: \n" + dmlconf.getConfigInfo());
-		
+
 		//Step 2: set local/remote memory if requested (for compile in AM context) 
 		if( dmlconf.getBooleanValue(DMLConfig.YARN_APPMASTER) ){
 			DMLAppMasterUtils.setupConfigRemoteMaxMemory(dmlconf); 
@@ -711,9 +713,9 @@ public class DMLScript
 		DMLDebuggerProgramInfo dbprog = new DMLDebuggerProgramInfo();
 		
 		//Step 1: parse configuration files
-		DMLConfig conf = DMLConfig.readAndMergeConfigurationFiles(fnameOptConfig);
+		DMLConfig conf = DMLConfig.readConfigurationFile(fnameOptConfig);
 		ConfigurationManager.setGlobalConfig(conf);
-	
+
 		//Step 2: parse dml script
 		AParserWrapper parser = AParserWrapper.createParser(parsePyDML);
 		DMLProgram prog = parser.parse(DML_FILE_PATH_ANTLR_PARSER, dmlScriptStr, argVals);
@@ -942,7 +944,7 @@ public class DMLScript
 		try
 		{
 			//read the default config
-			DMLConfig conf = DMLConfig.readAndMergeConfigurationFiles(null);
+			DMLConfig conf = DMLConfig.readConfigurationFile(null);
 			
 			//run cleanup job to clean remote local tmp dirs
 			CleanupMR.runJob(conf);

http://git-wip-us.apache.org/repos/asf/incubator-systemml/blob/6aef70cf/src/main/java/org/apache/sysml/conf/DMLConfig.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/sysml/conf/DMLConfig.java b/src/main/java/org/apache/sysml/conf/DMLConfig.java
index d65bba1..15ec73e 100644
--- a/src/main/java/org/apache/sysml/conf/DMLConfig.java
+++ b/src/main/java/org/apache/sysml/conf/DMLConfig.java
@@ -180,7 +180,7 @@ public class DMLConfig
 			} // end if (otherConfigNodeList != null && otherConfigNodeList.getLength() > 0){
 		} catch (Exception e){
 			LOG.error("Failed in merge default config file with optional config file",e);
-			throw new ParseException("ERROR: error merging config file" + otherConfig._fileName + " with " + _fileName);
+			throw new ParseException("ERROR: error merging config file " + otherConfig._fileName + " with " + _fileName);
 		}
 	}
 	
@@ -388,64 +388,46 @@ public class DMLConfig
 	}
 	
 	/**
-	 * 
-	 * @return
+	 * Start with the internal default settings, then merge in the
+	 * settings from any specified configuration file, if available.
+	 * If it is not explicitly given, then merge in settings from
+	 * the default configuration file location, if available.
+	 *
+	 * @param configPath User-defined path of the configuration file.
+	 * @return DMLConfig object containing configuration settings.
 	 * @throws ParseException 
 	 * @throws FileNotFoundException 
 	 */
-	public static DMLConfig readAndMergeConfigurationFiles( String optConfig ) 
+	public static DMLConfig readConfigurationFile(String configPath)
 		throws ParseException, FileNotFoundException
 	{
-		// optional config specified overwrites/merge into the default config
-		DMLConfig defaultConfig = null;
-		DMLConfig optionalConfig = null;
-		
-		if( optConfig != null ) { // the optional config is specified
-			try { // try to get the default config first 
-				defaultConfig = new DMLConfig(DEFAULT_SYSTEMML_CONFIG_FILEPATH, true);
-			} catch (FileNotFoundException fnfe) { // it is OK to not have the default, but give a warning
-				LOG.warn("No default SystemML config file (" + DEFAULT_SYSTEMML_CONFIG_FILEPATH + ") found");
-			} catch (ParseException e) {
-				defaultConfig = null;
-				throw e;
-			}
-			try { // try to get the optional config next
-				optionalConfig = new DMLConfig(optConfig, false);
+		// Always start with the internal defaults
+		DMLConfig config = new DMLConfig();
+
+		// Merge in any specified or default configs if available
+		if (configPath != null) {
+			// specified
+			try {
+				config = new DMLConfig(configPath, false);
 			} catch (FileNotFoundException fnfe) {
-				LOG.error("Config file " + optConfig + " not found");
+				LOG.error("Custom config file " + configPath + " not found.");
 				throw fnfe;
 			} catch (ParseException e) {
-				optionalConfig = null;
 				throw e;
 			}
-			if (defaultConfig != null) {
-				try {
-					defaultConfig.merge(optionalConfig);
-				}
-				catch(ParseException e){
-					defaultConfig = null;
-					throw e;
-				}
-			}
-			else {
-				defaultConfig = optionalConfig;
-			}
-		}
-		else { // the optional config is not specified
-			try { // try to get the default config 
-				defaultConfig = new DMLConfig(DEFAULT_SYSTEMML_CONFIG_FILEPATH, false);
-			} catch (FileNotFoundException fnfe) { // it is OK to not have the default, but give a warning
-				LOG.warn("No default SystemML config file (" + DEFAULT_SYSTEMML_CONFIG_FILEPATH + ") found");
-				LOG.warn("Using default settings in DMLConfig");
-				DMLConfig dmlConfig = new DMLConfig();
-				return dmlConfig;
-			} catch (ParseException e) { 
-				defaultConfig = null;
+		} else {
+			// default
+			try {
+				config = new DMLConfig(DEFAULT_SYSTEMML_CONFIG_FILEPATH, false);
+			} catch (FileNotFoundException fnfe) {
+				LOG.info("Using internal default configuration settings.  If you wish to " +
+						 "customize any settings, please supply a `SystemML-config.xml` file.");
+				config = new DMLConfig();
+			} catch (ParseException e) {
 				throw e;
 			}
 		}
-		
-		return defaultConfig;
+		return config;
 	}
 
 	/**