You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@systemds.apache.org by GitBox <gi...@apache.org> on 2022/07/20 13:14:25 UTC
[GitHub] [systemds] kev-inn opened a new pull request, #1667: [MINOR] Fix federation for Python API
kev-inn opened a new pull request, #1667:
URL: https://github.com/apache/systemds/pull/1667
Fixes a bug where instructions were not replaced by FED equivalent instructions, because the correct `CompilerConfig` option was not set.
In the future we want to check the statistics report in our federated testcases to check if execution was performed on federated workers.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [systemds] Baunsgaard commented on a diff in pull request #1667: [MINOR] Fix federation for Python API
Posted by GitBox <gi...@apache.org>.
Baunsgaard commented on code in PR #1667:
URL: https://github.com/apache/systemds/pull/1667#discussion_r925600464
##########
src/main/java/org/apache/sysds/api/jmlc/Connection.java:
##########
@@ -145,23 +146,15 @@ public Connection(DMLConfig dmlconfig) {
//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_INDIVIDUAL_SB_SPECIFIC_OPS, false);
+ _cconf.set(ConfigType.ALLOW_CSE_PERSISTENT_READS, false);
+ _cconf.set(ConfigType.CODEGEN_ENABLED, false);
Review Comment:
but Codegen for instance?
I get the Reject_read_write unknowns should be enabled.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [systemds] kev-inn commented on a diff in pull request #1667: [MINOR] Fix federation for Python API
Posted by GitBox <gi...@apache.org>.
kev-inn commented on code in PR #1667:
URL: https://github.com/apache/systemds/pull/1667#discussion_r925774815
##########
src/main/java/org/apache/sysds/api/jmlc/Connection.java:
##########
@@ -145,23 +146,15 @@ public Connection(DMLConfig dmlconfig) {
//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_INDIVIDUAL_SB_SPECIFIC_OPS, false);
+ _cconf.set(ConfigType.ALLOW_CSE_PERSISTENT_READS, false);
+ _cconf.set(ConfigType.CODEGEN_ENABLED, false);
Review Comment:
Removed two configs. Not sure what `ALLOW_INDIVIDUAL_SB_SPECIFIC_OPS` does, but removing didn't create any problems with the testcases.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [systemds] Baunsgaard closed pull request #1667: [MINOR] Fix federation for Python API
Posted by GitBox <gi...@apache.org>.
Baunsgaard closed pull request #1667: [MINOR] Fix federation for Python API
URL: https://github.com/apache/systemds/pull/1667
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [systemds] kev-inn commented on a diff in pull request #1667: [MINOR] Fix federation for Python API
Posted by GitBox <gi...@apache.org>.
kev-inn commented on code in PR #1667:
URL: https://github.com/apache/systemds/pull/1667#discussion_r925598420
##########
src/main/java/org/apache/sysds/api/jmlc/Connection.java:
##########
@@ -145,23 +146,15 @@ public Connection(DMLConfig dmlconfig) {
//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_INDIVIDUAL_SB_SPECIFIC_OPS, false);
+ _cconf.set(ConfigType.ALLOW_CSE_PERSISTENT_READS, false);
+ _cconf.set(ConfigType.CODEGEN_ENABLED, false);
Review Comment:
We need those, otherwise we can't execute the scripts as they read from files that do not exist, but are provided to the runtime by python.
Edit: or are some of them not necessary? I wanted to go the save route of not removing flags I am not sure for.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [systemds] Baunsgaard commented on a diff in pull request #1667: [MINOR] Fix federation for Python API
Posted by GitBox <gi...@apache.org>.
Baunsgaard commented on code in PR #1667:
URL: https://github.com/apache/systemds/pull/1667#discussion_r925596907
##########
src/main/java/org/apache/sysds/api/jmlc/Connection.java:
##########
@@ -145,23 +146,15 @@ public Connection(DMLConfig dmlconfig) {
//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_INDIVIDUAL_SB_SPECIFIC_OPS, false);
+ _cconf.set(ConfigType.ALLOW_CSE_PERSISTENT_READS, false);
+ _cconf.set(ConfigType.CODEGEN_ENABLED, false);
Review Comment:
It looks good to me, but i think many of these settings should not be set custom here.
I we can i suggest removing all of them.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [systemds] kev-inn commented on a diff in pull request #1667: [MINOR] Fix federation for Python API
Posted by GitBox <gi...@apache.org>.
kev-inn commented on code in PR #1667:
URL: https://github.com/apache/systemds/pull/1667#discussion_r925598420
##########
src/main/java/org/apache/sysds/api/jmlc/Connection.java:
##########
@@ -145,23 +146,15 @@ public Connection(DMLConfig dmlconfig) {
//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_INDIVIDUAL_SB_SPECIFIC_OPS, false);
+ _cconf.set(ConfigType.ALLOW_CSE_PERSISTENT_READS, false);
+ _cconf.set(ConfigType.CODEGEN_ENABLED, false);
Review Comment:
We need those, otherwise we can't execute the scripts as they read from files that do not exist, but are provided to the runtime by python.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@systemds.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org