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:
[email protected]
