j143 commented on a change in pull request #893:
URL: https://github.com/apache/systemml/pull/893#discussion_r415398205
##########
File path: src/main/java/org/apache/sysds/runtime/util/UtilFunctions.java
##########
@@ -41,6 +36,18 @@
import org.apache.sysds.runtime.matrix.data.Pair;
import org.apache.sysds.runtime.meta.TensorCharacteristics;
+import java.util.ArrayList;
Review comment:
Changes such as
1. Formatting (whitespace addition/removal)
2. Rearragement of imports
These kind of changes, we cannot be decisive. Now, the reviewing person
needs to be able give OK for the functionality and formatting changes.
**TL;DR**: If there is a need for change of formatting opening a separate PR
would be more helpful.
##########
File path:
src/test/java/org/apache/sysds/test/functions/federated/FederatedConstructionTest.java
##########
@@ -62,28 +70,59 @@ public void setUp() {
}
@Test
- public void federatedConstructionCP() {
- federatedConstruction(Types.ExecMode.SINGLE_NODE);
+ public void federatedMatrixConstructionCP() {
+ federatedMatrixConstruction(Types.ExecMode.SINGLE_NODE);
+ }
+
+ @Test
+ public void federatedMatrixConstructionSP() {
+ federatedMatrixConstruction(Types.ExecMode.SPARK);
}
+ public void federatedMatrixConstruction(Types.ExecMode execMode) {
+ getAndLoadTestConfiguration(TEST_NAME);
+ // write input matrix
+ double[][] A = getRandomMatrix(rows, cols, -1, 1, 1, 1234);
+ writeInputMatrixWithMTD("A", A, false, new
MatrixCharacteristics(rows, cols, blocksize, rows * cols));
+ federatedConstruction(execMode, MATRIX_TEST_FILE_NAME, "A",
null);
+ }
+
@Test
- public void federatedConstructionSP() {
- federatedConstruction(Types.ExecMode.SPARK);
+ public void federatedFrameConstructionCP() throws IOException {
+ federatedFrameConstruction(Types.ExecMode.SINGLE_NODE);
+ }
+
+ /* like other federated functionality, SPARK execution mode is not yet
working (waiting for better integration
Review comment:
Should we mark this a TODO.
##########
File path: src/test/java/org/apache/sysds/test/TestUtils.java
##########
@@ -268,78 +268,171 @@ else if (Integer.parseInt(expRcn[2]) !=
Integer.parseInt(rcn[2])) {
fail("unable to read file: " + e.getMessage());
}
}
-
+
/**
* <p>
- * Compares the expected values calculated in Java by testcase and
which are
- * in the normal filesystem, with those calculated by SystemDS located
in
- * HDFS
+ * Read the cell values of the expected file and actual files. Schema
is used for correct parsing if the file is a
+ * frame and if it is null FP64 will be used for all values (useful for
Matrices).
* </p>
- *
- * @param expectedFile
- * file with expected values, which is located in OS
filesystem
- * @param actualDir
- * file with actual values, which is located in HDFS
- * @param epsilon
- * tolerance for value comparison
+ *
+ * @param schema the schema of the frame, can be null (for FP64)
+ * @param expectedFile the file with expected values
+ * @param actualDir the directory where the actual values were
written
+ * @param expectedValues the HashMap where the expected values will be
written to
+ * @param actualValues the HashMap where the actual values will be
written to
*/
- @SuppressWarnings("resource")
- public static void compareDMLMatrixWithJavaMatrix(String expectedFile,
String actualDir, double epsilon) {
+ private static void readActualAndExpectedFile(ValueType[] schema,
String expectedFile, String actualDir,
+ HashMap<CellIndex, Object> expectedValues, HashMap<CellIndex,
Object> actualValues) {
try {
Path outDirectory = new Path(actualDir);
Path compareFile = new Path(expectedFile);
FileSystem fs =
IOUtilFunctions.getFileSystem(outDirectory, conf);
FSDataInputStream fsin = fs.open(compareFile);
-
- HashMap<CellIndex, Double> expectedValues = new
HashMap<>();
-
- try( BufferedReader compareIn = new BufferedReader(new
InputStreamReader(fsin)) ) {
+
+ try(BufferedReader compareIn = new BufferedReader(new
InputStreamReader(fsin))) {
Review comment:
The test configuration has been modified, would you mind rebasing the
latest.
##########
File path:
src/main/java/org/apache/sysds/runtime/controlprogram/caching/FrameObject.java
##########
@@ -155,15 +182,46 @@ public long getNumColumns() {
return dc.getCols();
}
+ private FrameBlock readFederated() {
+ FrameBlock result = new FrameBlock(_schema);
+ // provide long support?
Review comment:
`int` may not be sufficient. Either this be marked a TODO or rationally
assume to that cases would fall within range.
----------------------------------------------------------------
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]
