Baunsgaard commented on a change in pull request #1455:
URL: https://github.com/apache/systemds/pull/1455#discussion_r753777279
##########
File path: src/main/java/org/apache/sysds/runtime/matrix/data/FrameBlock.java
##########
@@ -2520,25 +2527,35 @@ public FrameBlock removeEmptyOperations(boolean rows,
boolean emptyReturn, Matr
}
private FrameBlock removeEmptyRows(MatrixBlock select, boolean
emptyReturn) {
+ if(select != null && (select.getNumRows() != this.getNumRows()
&& select.getNumColumns() != this.getNumRows()))
+ throw new DMLRuntimeException("Frame rmempty: Incorrect
select vector dimensions.");
+
FrameBlock ret = new FrameBlock(_schema, _colnames);
- for(int i = 0; i < _numRows; i++) {
- boolean isEmpty = true;
- Object[] row = new Object[getNumColumns()];
+ if (select == null) {
+ for(int i = 0; i < _numRows; i++) {
+ boolean isEmpty = true;
+ Object[] row = new Object[getNumColumns()];
+ for(int j = 0; j < getNumColumns() && isEmpty;
j++) {
+ row[j] = _coldata[j].get(i);
+ isEmpty = isEmpty &&
(ArrayUtils.contains(new double[]{0.0, Double.NaN},
UtilFunctions.objectToDoubleSafe(_schema[j], _coldata[j].get(i))));
+ }
- for(int j = 0; j < getNumColumns(); j++) {
- Array colData = _coldata[j].clone();
- row[j] = colData.get(i);
- ValueType type = _schema[j];
- isEmpty = isEmpty && (ArrayUtils.contains(new
double[]{0.0, Double.NaN}, UtilFunctions.objectToDoubleSafe(type,
colData.get(i))));
+ if(!isEmpty)
+ ret.appendRow(row);
}
+ } else {
+ List<Integer> indices =
DataConverter.convertVectorToIndicesList(select);
Review comment:
better with int[]
##########
File path: src/main/java/org/apache/sysds/runtime/util/DataConverter.java
##########
@@ -307,6 +307,34 @@ public static MatrixBlock
readMatrixFromHDFS(ReadProperties prop)
return ret;
}
+ public static List<Integer> convertVectorToIndicesList(MatrixBlock mb)
+ {
+ int rows = mb.getNumRows();
+ int cols = mb.getNumColumns();
+ ArrayList<Integer> indices = new ArrayList<>();
+
+ if( mb.getNonZeros() > 0 )
+ {
+ if( mb.isInSparseFormat() )
+ {
+ Iterator<IJV> iter =
mb.getSparseBlockIterator();
+ while( iter.hasNext() ) {
+ IJV cell = iter.next();
+ if( cell.getV() != 0.0)
+
indices.add(cell.getI()*cols+cell.getJ());
+ }
+ }
+ else {
+ for( int i=0, cix=0; i<rows; i++ )
+ for( int j=0; j<cols; j++, cix++)
+ if( mb.getValueDenseUnsafe(i,
j) != 0.0 )
+ indices.add(cix);
Review comment:
don't add to the ArrayList (in general don't use an ArrayList here)
preallocate a single int[] based on nnz in the input.
then put the values into that.
##########
File path: src/main/java/org/apache/sysds/runtime/util/DataConverter.java
##########
@@ -307,6 +307,34 @@ public static MatrixBlock
readMatrixFromHDFS(ReadProperties prop)
return ret;
}
+ public static List<Integer> convertVectorToIndicesList(MatrixBlock mb)
+ {
+ int rows = mb.getNumRows();
+ int cols = mb.getNumColumns();
+ ArrayList<Integer> indices = new ArrayList<>();
+
+ if( mb.getNonZeros() > 0 )
+ {
+ if( mb.isInSparseFormat() )
+ {
+ Iterator<IJV> iter =
mb.getSparseBlockIterator();
+ while( iter.hasNext() ) {
+ IJV cell = iter.next();
+ if( cell.getV() != 0.0)
+
indices.add(cell.getI()*cols+cell.getJ());
+ }
Review comment:
on a sparse block call:
```java
SparseBlock sb = mb.getSparseBlock()
int[] aix = sb.indexes(0);
return aix;
```
This is assuming that you have only one row of values.
##########
File path:
src/test/java/org/apache/sysds/test/component/frame/FrameRemoveEmptyTest.java
##########
@@ -35,7 +35,7 @@
private final static String TEST_DIR = "functions/frame/";
private static final String TEST_CLASS_DIR = TEST_DIR +
RemoveEmptyTest.class.getSimpleName() + "/";
- private final static int _rows = 10;
+ private final static int _rows = 100;
Review comment:
add tests for selecting for selecting 0, 1, half, all-but-one, all.
in case of selecting all, simply return a deep copy.
100 does not really change performance measurements.
while debugging try to increase it, same goes for columns.
then create measurements for selecting 1, half, all-but-one elements from
the input.
and find out if the new version is faster or slower in each case.
In the PR message argue the performance difference say something like 2x
faster etc.
##########
File path: src/main/java/org/apache/sysds/runtime/util/DataConverter.java
##########
@@ -307,6 +307,34 @@ public static MatrixBlock
readMatrixFromHDFS(ReadProperties prop)
return ret;
}
+ public static List<Integer> convertVectorToIndicesList(MatrixBlock mb)
+ {
+ int rows = mb.getNumRows();
+ int cols = mb.getNumColumns();
+ ArrayList<Integer> indices = new ArrayList<>();
+
+ if( mb.getNonZeros() > 0 )
Review comment:
first if check for empty. (return null)
second check for sparse (return already allocated the sparse allocated
indexes )
third (default) iterate dense. (allocate based on nnz)
##########
File path: src/main/java/org/apache/sysds/runtime/util/DataConverter.java
##########
@@ -307,6 +307,34 @@ public static MatrixBlock
readMatrixFromHDFS(ReadProperties prop)
return ret;
}
+ public static List<Integer> convertVectorToIndicesList(MatrixBlock mb)
Review comment:
return int []
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
