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 2020/11/20 13:56:50 UTC

[GitHub] [systemds] OlgaOvcharenko opened a new pull request #1111: Lm pipeline test with 4 workers

OlgaOvcharenko opened a new pull request #1111:
URL: https://github.com/apache/systemds/pull/1111


   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [systemds] Baunsgaard commented on a change in pull request #1111: Lm pipeline test with 4 workers

Posted by GitBox <gi...@apache.org>.
Baunsgaard commented on a change in pull request #1111:
URL: https://github.com/apache/systemds/pull/1111#discussion_r527779516



##########
File path: src/test/java/org/apache/sysds/test/functions/federated/algorithms/FederatedLmPipeline.java
##########
@@ -19,6 +19,7 @@
 
 package org.apache.sysds.test.functions.federated.algorithms;
 
+import com.sun.tools.javac.util.List;

Review comment:
       and to create an array in oneline use:
   
   ```java
   int[] x = new int[] {1, 20, 3, 40, 0}
   ```




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [systemds] Baunsgaard commented on a change in pull request #1111: Lm pipeline test with 4 workers

Posted by GitBox <gi...@apache.org>.
Baunsgaard commented on a change in pull request #1111:
URL: https://github.com/apache/systemds/pull/1111#discussion_r527774713



##########
File path: src/test/java/org/apache/sysds/test/functions/federated/algorithms/FederatedLmPipeline.java
##########
@@ -76,37 +85,48 @@ public void federatedLmPipeline(ExecMode execMode, boolean contSplits) {
 			X = rc.append(X, new MatrixBlock(), true);
 			
 			// We have two matrices handled by a single federated worker
-			int halfRows = rows / 2;
-			writeInputMatrixWithMTD("X1", X.slice(0, halfRows-1), false);
-			writeInputMatrixWithMTD("X2", X.slice(halfRows, rows-1), false);
+			int quarterRows = TEST_NAME.equals(TEST_NAME2) ? rows / 4 : rows / 2;
+			List<Integer> k =  TEST_NAME.equals(TEST_NAME2) ? List.of(quarterRows-1, quarterRows, 2*quarterRows-1, 2*quarterRows, 3*quarterRows-1, 3*quarterRows, rows-1) :
+				List.of(quarterRows-1, quarterRows, rows-1, 0, 0, 0, 0);
+			writeInputMatrixWithMTD("X1", X.slice(0, k.get(0)), false);
+			writeInputMatrixWithMTD("X2", X.slice(k.get(1), k.get(2)), false);
+			writeInputMatrixWithMTD("X3", X.slice(k.get(3), k.get(4)), false);
+			writeInputMatrixWithMTD("X4", X.slice(k.get(5), k.get(6)), false);
 			writeInputMatrixWithMTD("Y", y, false);
 			
 			// empty script name because we don't execute any script, just start the worker
 			fullDMLScriptName = "";
 			int port1 = getRandomAvailablePort();
 			int port2 = getRandomAvailablePort();
+			int port3 = getRandomAvailablePort();
+			int port4 = getRandomAvailablePort();
 			Thread t1 = startLocalFedWorkerThread(port1, FED_WORKER_WAIT_S);
 			Thread t2 = startLocalFedWorkerThread(port2);
+			Thread t3 = startLocalFedWorkerThread(port3);

Review comment:
       I have added the second argument to reduce the Federated worker startup, so we don't have to wait for a second for each of the workers, but just the last.

##########
File path: src/test/java/org/apache/sysds/test/functions/federated/algorithms/FederatedLmPipeline.java
##########
@@ -19,6 +19,7 @@
 
 package org.apache.sysds.test.functions.federated.algorithms;
 
+import com.sun.tools.javac.util.List;

Review comment:
       I think you have some automated imports that have to change priority.
   Usually we don't import the com.sun libraries.

##########
File path: src/test/java/org/apache/sysds/test/functions/federated/algorithms/FederatedLmPipeline.java
##########
@@ -44,20 +46,27 @@
 	@Override
 	public void setUp() {
 		TestUtils.clearAssertionInformation();
-		addTestConfiguration(TEST_NAME, new TestConfiguration(TEST_CLASS_DIR, TEST_NAME, new String[] {"Z"}));
+		addTestConfiguration(TEST_NAME1, new TestConfiguration(TEST_CLASS_DIR, TEST_NAME1, new String[] {"Z"}));
+		addTestConfiguration(TEST_NAME2, new TestConfiguration(TEST_CLASS_DIR, TEST_NAME2, new String[] {"Z"}));
 	}
 
 	@Test
 	public void federatedLmPipelineContinguous() {
-		federatedLmPipeline(Types.ExecMode.SINGLE_NODE, true);
+		federatedLmPipeline(Types.ExecMode.SINGLE_NODE, true, TEST_NAME1);
 	}
+
+	@Test
+	public void federatedLmPipelineContinguous4Workers() { federatedLmPipeline(Types.ExecMode.SINGLE_NODE, true, TEST_NAME2); }
 	
 	@Test
 	public void federatedLmPipelineSampled() {
-		federatedLmPipeline(Types.ExecMode.SINGLE_NODE, false);
+		federatedLmPipeline(Types.ExecMode.SINGLE_NODE, false, TEST_NAME1);
 	}
 
-	public void federatedLmPipeline(ExecMode execMode, boolean contSplits) {
+	@Test
+	public void federatedLmPipelineSampled4Workers() { federatedLmPipeline(Types.ExecMode.SINGLE_NODE, false, TEST_NAME2); }

Review comment:
       i personally like it in one line as well, but our other tests, does not follow this syntax so i would suggest multi line.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [systemds] Baunsgaard commented on a change in pull request #1111: Lm pipeline test with 4 workers

Posted by GitBox <gi...@apache.org>.
Baunsgaard commented on a change in pull request #1111:
URL: https://github.com/apache/systemds/pull/1111#discussion_r527777280



##########
File path: src/test/java/org/apache/sysds/test/functions/federated/algorithms/FederatedLmPipeline.java
##########
@@ -19,6 +19,7 @@
 
 package org.apache.sysds.test.functions.federated.algorithms;
 
+import com.sun.tools.javac.util.List;

Review comment:
       import java.util.List;




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [systemds] Baunsgaard closed pull request #1111: Lm pipeline test with 4 workers

Posted by GitBox <gi...@apache.org>.
Baunsgaard closed pull request #1111:
URL: https://github.com/apache/systemds/pull/1111


   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org