This is an automated email from the ASF dual-hosted git repository.
baunsgaard pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/systemds.git
The following commit(s) were added to refs/heads/main by this push:
new b43fa112c7 [MINOR] Refactor MatrixBlock append
b43fa112c7 is described below
commit b43fa112c7b4ba53921e8471fdcbc3d0a15da7ae
Author: Sebastian Baunsgaard <[email protected]>
AuthorDate: Wed Feb 5 12:52:56 2025 +0100
[MINOR] Refactor MatrixBlock append
This PR contains a refactor of MatrixBlock.append, the refactor serves two
purposes.
1. The append logic inside the MatrixBlock, was > 200 lines with many
specific functions only for append logic.
2. To allow JIT compilation of the Sparse and Dense appending respectively.
The current implementation contained one big method that did not -- in my
experiments -- JIT compile ever.
Closes #2219
---
.../sysds/runtime/matrix/data/LibMatrixAppend.java | 184 +++++++++++++++++++++
.../sysds/runtime/matrix/data/MatrixBlock.java | 150 +----------------
.../component/compress/ExtendedMatrixTests.java | 2 -
3 files changed, 188 insertions(+), 148 deletions(-)
diff --git
a/src/main/java/org/apache/sysds/runtime/matrix/data/LibMatrixAppend.java
b/src/main/java/org/apache/sysds/runtime/matrix/data/LibMatrixAppend.java
new file mode 100644
index 0000000000..9858a1e84b
--- /dev/null
+++ b/src/main/java/org/apache/sysds/runtime/matrix/data/LibMatrixAppend.java
@@ -0,0 +1,184 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.sysds.runtime.matrix.data;
+
+import org.apache.commons.lang3.ArrayUtils;
+import org.apache.sysds.runtime.DMLRuntimeException;
+import org.apache.sysds.runtime.compress.CompressedMatrixBlock;
+import org.apache.sysds.runtime.compress.lib.CLALibCBind;
+import org.apache.sysds.runtime.data.DenseBlock;
+import org.apache.sysds.runtime.data.SparseBlock;
+import org.apache.sysds.runtime.data.SparseBlockMCSR;
+
+public class LibMatrixAppend {
+
+ public static MatrixBlock append(MatrixBlock a, MatrixBlock[] that,
MatrixBlock result, boolean cbind) {
+ int rlen = a.rlen;
+ int clen = a.clen;
+ long nonZeros = a.nonZeros;
+
+ checkDimensionsForAppend(that, cbind, rlen, clen);
+
+ for(int k = 0; k < that.length; k++)
+ if(that[k] instanceof CompressedMatrixBlock) {
+ if(that.length == 1 && cbind)
+ return CLALibCBind.cbind(a, that[0], 1);
+ that[k] =
CompressedMatrixBlock.getUncompressed(that[k], "Append N");
+ }
+
+ final int m = cbind ? rlen : combinedRows(that, rlen);
+ final int n = cbind ? combinedCols(that, clen) : clen;
+ final long nnz = calculateCombinedNNz(that, nonZeros);
+
+ boolean shallowCopy = (nonZeros == nnz);
+ boolean sp = MatrixBlock.evalSparseFormatInMemory(m, n, nnz);
+
+ // init result matrix
+ if(result == null)
+ result = new MatrixBlock(m, n, sp, nnz);
+ else
+ result.reset(m, n, sp, nnz);
+
+ // core append operation
+ // copy left and right input into output
+ if(!result.sparse && nnz != 0)
+ appendDense(a, that, result, cbind, rlen, m, n);
+ else if(nnz != 0)
+ appendSparse(a, that, result, cbind, rlen, clen, nnz,
shallowCopy);
+
+ // update meta data
+ result.nonZeros = nnz;
+ return result;
+ }
+
+ private static void appendSparse(MatrixBlock a, MatrixBlock[] that,
MatrixBlock result, boolean cbind, int rlen,
+ int clen, final long nnz, boolean shallowCopy) {
+ // adjust sparse rows if required
+ result.allocateSparseRowsBlock();
+ // allocate sparse rows once for cbind
+ if(cbind && nnz > rlen && !shallowCopy &&
result.getSparseBlock() instanceof SparseBlockMCSR) {
+ final SparseBlock sblock = result.getSparseBlock();
+ // for each row calculate how many non zeros are
pressent.
+ for(int i = 0; i < result.rlen; i++)
+ sblock.allocate(i, computeNNzRow(that, i, a));
+
+ }
+
+ // core append operation
+ // we can always append this directly to offset 0.0 in both
cbind and rbind.
+ result.appendToSparse(a, 0, 0, !shallowCopy);
+ if(cbind) {
+ for(int i = 0, off = clen; i < that.length; i++) {
+ result.appendToSparse(that[i], 0, off);
+ off += that[i].clen;
+ }
+ }
+ else { // rbind
+ for(int i = 0, off = rlen; i < that.length; i++) {
+ result.appendToSparse(that[i], off, 0);
+ off += that[i].rlen;
+ }
+ }
+ }
+
+ private static void appendDense(MatrixBlock a, MatrixBlock[] that,
MatrixBlock result, boolean cbind, int rlen,
+ final int m, final int n) {
+ if(cbind) {
+ DenseBlock resd =
result.allocateBlock().getDenseBlock();
+ MatrixBlock[] in = ArrayUtils.addAll(new MatrixBlock[]
{a}, that);
+
+ for(int i = 0; i < m; i++) {
+ for(int k = 0, off = 0; k < in.length; off +=
in[k].clen, k++) {
+ if(in[k].isEmptyBlock(false))
+ continue;
+ if(in[k].sparse) {
+ SparseBlock src =
in[k].sparseBlock;
+ if(src.isEmpty(i))
+ continue;
+ int srcpos = src.pos(i);
+ int srclen = src.size(i);
+ int[] srcix = src.indexes(i);
+ double[] srcval = src.values(i);
+ double[] resval =
resd.values(i);
+ int resix = resd.pos(i, off);
+ for(int j = srcpos; j < srcpos
+ srclen; j++)
+ resval[resix +
srcix[j]] = srcval[j];
+ }
+ else {
+ DenseBlock src =
in[k].getDenseBlock();
+ double[] srcval = src.values(i);
+ double[] resval =
resd.values(i);
+ System.arraycopy(srcval,
src.pos(i), resval, resd.pos(i, off), in[k].clen);
+ }
+ }
+ }
+ }
+ else { // rbind
+ result.copy(0, rlen - 1, 0, n - 1, a, false);
+ for(int i = 0, off = rlen; i < that.length; i++) {
+ result.copy(off, off + that[i].rlen - 1, 0, n -
1, that[i], false);
+ off += that[i].rlen;
+ }
+ }
+ }
+
+ public static void checkDimensionsForAppend(MatrixBlock[] in, boolean
cbind, int rlen, int clen) {
+ if(cbind) {
+ for(int i = 0; i < in.length; i++)
+ if(in[i].rlen != rlen)
+ throw new DMLRuntimeException(
+ "Invalid nRow dimension for
append cbind: was " + in[i].rlen + " should be: " + rlen);
+ }
+ else {
+ for(int i = 0; i < in.length; i++)
+ if(in[i].clen != clen)
+ throw new DMLRuntimeException(
+ "Invalid nCol dimension for
append rbind: was " + in[i].clen + " should be: " + clen);
+ }
+ }
+
+ private static int combinedRows(MatrixBlock[] that, int rlen) {
+ int r = rlen;
+ for(MatrixBlock b : that)
+ r += b.rlen;
+ return r;
+ }
+
+ private static int combinedCols(MatrixBlock[] that, int clen) {
+ int c = clen;
+ for(MatrixBlock b : that)
+ c += b.clen;
+ return c;
+ }
+
+ private static long calculateCombinedNNz(MatrixBlock[] that, long
nonZeros) {
+ long nnz = nonZeros;
+ for(MatrixBlock b : that)
+ nnz += b.nonZeros;
+ return nnz;
+ }
+
+ private static int computeNNzRow(MatrixBlock[] that, int row,
MatrixBlock a) {
+ int lnnz = (int) a.recomputeNonZeros(row, row, 0, a.clen - 1);
+ for(MatrixBlock b : that)
+ lnnz += b.recomputeNonZeros(row, row, 0, b.clen - 1);
+ return lnnz;
+ }
+}
diff --git
a/src/main/java/org/apache/sysds/runtime/matrix/data/MatrixBlock.java
b/src/main/java/org/apache/sysds/runtime/matrix/data/MatrixBlock.java
index af068d2523..33903954a2 100644
--- a/src/main/java/org/apache/sysds/runtime/matrix/data/MatrixBlock.java
+++ b/src/main/java/org/apache/sysds/runtime/matrix/data/MatrixBlock.java
@@ -38,7 +38,6 @@ import java.util.concurrent.Future;
import java.util.stream.Collectors;
import java.util.stream.IntStream;
-import org.apache.commons.lang3.ArrayUtils;
import org.apache.commons.lang3.NotImplementedException;
import org.apache.commons.lang3.concurrent.ConcurrentUtils;
import org.apache.commons.logging.Log;
@@ -56,7 +55,6 @@ import org.apache.sysds.runtime.DMLRuntimeException;
import org.apache.sysds.runtime.compress.CompressedMatrixBlock;
import org.apache.sysds.runtime.compress.DMLCompressionException;
import org.apache.sysds.runtime.compress.lib.CLALibAggTernaryOp;
-import org.apache.sysds.runtime.compress.lib.CLALibCBind;
import org.apache.sysds.runtime.compress.lib.CLALibMerge;
import org.apache.sysds.runtime.compress.lib.CLALibTernaryOp;
import org.apache.sysds.runtime.controlprogram.caching.CacheBlock;
@@ -891,7 +889,7 @@ public class MatrixBlock extends MatrixValue implements
CacheBlock<MatrixBlock>,
appendToSparse(that, rowoffset, coloffset, true);
}
- private void appendToSparse( MatrixBlock that, int rowoffset, int
coloffset, boolean deep )
+ protected void appendToSparse( MatrixBlock that, int rowoffset, int
coloffset, boolean deep )
{
if( that==null || that.isEmptyBlock(false) )
return; //nothing to append
@@ -3656,7 +3654,7 @@ public class MatrixBlock extends MatrixValue implements
CacheBlock<MatrixBlock>,
}
/**
- * Append that list of matrixblocks to this.
+ * Append that list of matrixblocks together.
*
* @param that That list.
* @param ret The output block
@@ -3681,38 +3679,10 @@ public class MatrixBlock extends MatrixValue implements
CacheBlock<MatrixBlock>,
* @param cbind if binding on columns or rows
* @return the ret MatrixBlock object with the appended result
*/
- public final MatrixBlock append( MatrixBlock that, MatrixBlock ret,
boolean cbind ) {
+ public final MatrixBlock append(MatrixBlock that, MatrixBlock ret,
boolean cbind ) {
return append(new MatrixBlock[]{that}, ret, cbind);
}
- private final long calculateCombinedNNz(MatrixBlock[] that){
- long nnz = nonZeros;
- for(MatrixBlock b : that)
- nnz += b.nonZeros;
- return nnz;
- }
-
- private final int combinedRows(MatrixBlock[] that){
- int r = rlen;
- for(MatrixBlock b : that)
- r += b.rlen;
- return r;
- }
-
- private final int combinedCols(MatrixBlock[] that){
- int c = clen;
- for(MatrixBlock b : that)
- c += b.clen;
- return c;
- }
-
- private final int computeNNzRow(MatrixBlock[] that, int row) {
- int lnnz = (int) this.recomputeNonZeros(row, row, 0, this.clen
- 1);
- for(MatrixBlock b : that)
- lnnz += b.recomputeNonZeros(row, row, 0, b.clen - 1);
- return lnnz;
- }
-
/**
* Append that list of matrixes to this matrix.
*
@@ -3724,119 +3694,7 @@ public class MatrixBlock extends MatrixValue implements
CacheBlock<MatrixBlock>,
* @return the ret MatrixBlock object with the appended result
*/
public MatrixBlock append(MatrixBlock[] that, MatrixBlock result,
boolean cbind) {
- checkDimensionsForAppend(that, cbind);
-
- for(int k = 0; k < that.length; k++)
- if( that[k] instanceof CompressedMatrixBlock){
- if(that.length == 1 && cbind)
- return CLALibCBind.cbind(this, that[0],
1);
- that[k] =
CompressedMatrixBlock.getUncompressed(that[k], "Append N");
- }
-
- final int m = cbind ? rlen : combinedRows(that);
- final int n = cbind ? combinedCols(that) : clen;
- final long nnz = calculateCombinedNNz(that);
-
- boolean shallowCopy = (nonZeros == nnz);
- boolean sp = evalSparseFormatInMemory(m, n, nnz);
-
- //init result matrix
- if( result == null )
- result = new MatrixBlock(m, n, sp, nnz);
- else
- result.reset(m, n, sp, nnz);
-
- //core append operation
- //copy left and right input into output
- if( !result.sparse && nnz!=0 ) //DENSE
- {
- if( cbind ) {
- DenseBlock resd =
result.allocateBlock().getDenseBlock();
- MatrixBlock[] in = ArrayUtils.addAll(new
MatrixBlock[]{this}, that);
-
- for( int i=0; i<m; i++ ) {
- for( int k=0, off=0; k<in.length;
off+=in[k].clen, k++ ) {
- if( in[k].isEmptyBlock(false) )
- continue;
- if( in[k].sparse ) {
- SparseBlock src =
in[k].sparseBlock;
- if( src.isEmpty(i) )
- continue;
- int srcpos = src.pos(i);
- int srclen =
src.size(i);
- int[] srcix =
src.indexes(i);
- double[] srcval =
src.values(i);
- double[] resval =
resd.values(i);
- int resix = resd.pos(i,
off);
- for (int j=srcpos;
j<srcpos+srclen; j++)
-
resval[resix+srcix[j]] = srcval[j];
- }
- else {
- DenseBlock src =
in[k].getDenseBlock();
- double[] srcval =
src.values(i);
- double[] resval =
resd.values(i);
-
System.arraycopy(srcval, src.pos(i),
- resval,
resd.pos(i, off), in[k].clen);
- }
- }
- }
- }
- else { //rbind
- result.copy(0, rlen-1, 0, n-1, this, false);
- for(int i=0, off=rlen; i<that.length; i++) {
- result.copy(off, off+that[i].rlen-1, 0,
n-1, that[i], false);
- off += that[i].rlen;
- }
- }
- }
- //SPARSE
- else if(nnz != 0) {
- //adjust sparse rows if required
- result.allocateSparseRowsBlock();
- //allocate sparse rows once for cbind
- if( cbind && nnz > rlen && !shallowCopy &&
result.getSparseBlock() instanceof SparseBlockMCSR ) {
- final SparseBlock sblock =
result.getSparseBlock();
- // for each row calculate how many non zeros
are pressent.
- for( int i=0; i<result.rlen; i++ )
- sblock.allocate(i, computeNNzRow(that,
i));
-
- }
-
- //core append operation
- // we can always append this directly to offset 0.0 in
both cbind and rbind.
- result.appendToSparse(this, 0, 0, !shallowCopy);
- if( cbind ) {
- for(int i=0, off=clen; i<that.length; i++) {
- result.appendToSparse(that[i], 0, off);
- off += that[i].clen;
- }
- }
- else { //rbind
- for(int i=0, off=rlen; i<that.length; i++) {
- result.appendToSparse(that[i], off, 0);
- off += that[i].rlen;
- }
- }
- }
-
- //update meta data
- result.nonZeros = nnz;
- return result;
- }
-
- public void checkDimensionsForAppend(MatrixBlock[] in, boolean cbind) {
- if(cbind) {
- for(int i = 0; i < in.length; i++)
- if(in[i].rlen != rlen)
- throw new DMLRuntimeException(
- "Invalid nRow dimension for
append cbind: was " + in[i].rlen + " should be: " + rlen);
- }
- else {
- for(int i = 0; i < in.length; i++)
- if(in[i].clen != clen)
- throw new DMLRuntimeException(
- "Invalid nCol dimension for
append rbind: was " + in[i].clen + " should be: " + clen);
- }
+ return LibMatrixAppend.append(this, that, result, cbind);
}
public static MatrixBlock naryOperations(Operator op, MatrixBlock[]
matrices, ScalarObject[] scalars, MatrixBlock ret) {
diff --git
a/src/test/java/org/apache/sysds/test/component/compress/ExtendedMatrixTests.java
b/src/test/java/org/apache/sysds/test/component/compress/ExtendedMatrixTests.java
index ed27dfb68e..3176586065 100644
---
a/src/test/java/org/apache/sysds/test/component/compress/ExtendedMatrixTests.java
+++
b/src/test/java/org/apache/sysds/test/component/compress/ExtendedMatrixTests.java
@@ -200,8 +200,6 @@ public class ExtendedMatrixTests extends CompressedTestBase
{
if(!(cmb instanceof CompressedMatrixBlock))
return;
double ret1 = cmb.prod();
- LOG.error(ret1);
- LOG.error(cmb);
double ret2 = mb.prod();
boolean res;
if(_cs != null && (_cs.lossy || overlappingType ==
OverLapping.SQUASH))
