[SYSTEMDS-206] HDF5 File Format#1299
[SYSTEMDS-206] HDF5 File Format#1299fathollahzadeh wants to merge 20 commits intoapache:masterfrom fathollahzadeh:sf-hdf5
Conversation
Baunsgaard
left a comment
Baunsgaard
left a comment
There was a problem hiding this comment.
A few comments, I have issues with the syntax for @,,, , double array allocations, and the tests. but otherwise it looks fine.
I am unsure how much of the code is actually executed in the tests, maybe you want to see what parts are covered using jacoco?
most importantly the tests are missing verification of the actual values parsed and written to disk since the tests currently only verify that there are no crashes.
src/main/java/org/apache/sysds/runtime/instructions/cp/VariableCPInstruction.java
Outdated
Show resolved
Hide resolved
src/main/java/org/apache/sysds/runtime/instructions/cp/VariableCPInstruction.java
Show resolved
Hide resolved
src/main/java/org/apache/sysds/runtime/io/hdf5/message/H5FillValueMessage.java
Outdated
Show resolved
Hide resolved
| return bb; | ||
| } | ||
|
|
||
| public void toBuffer(H5BufferBuilder bb) { |
There was a problem hiding this comment.
should this method do nothing?
src/main/java/org/apache/sysds/runtime/io/hdf5/message/H5ObjectModificationTimeMessage.java
Outdated
Show resolved
Hide resolved
| double[][] A = getRandomMatrix(rows, cols, 0, 10, 1, 714); | ||
| writeInputMatrixWithMTD("A", A, false); | ||
|
|
||
| runTest(true, false, null, -1); |
There was a problem hiding this comment.
Missing verify of the written matrix.
There was a problem hiding this comment.
R scripts for read/write verification added
add r test to verify r/w fix code style
…orts, ... Added read/write verification test with R Removed some MutableInt variables and replace them with primitive variables Removed unused codes
Thanks for reviewing PR @Baunsgaard . I really appreciate your review and your comments. I fixed most of them and pushed them in separate commits. Also, some short responses to resolve the comments. |
|
Thanks for the PR, i have now merged it. |
3 participants
This commit is an implementation for HDF5 data format files. The HDF5 The file format specification is available from the HDF Group here
Currently, this patch is targeting the HDF5 format with supporting some features of the HDF5. The supported features: