Skip to content

Commit b43fa11

Browse files
BaunsgaardBaunsgaard
committed
[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
1 parent 0d41508 commit b43fa11

File tree

3 files changed

+188
-148
lines changed

3 files changed

+188
-148
lines changed

‎src/main/java/org/apache/sysds/runtime/matrix/data/LibMatrixAppend.java‎

Lines changed: 184 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,184 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
package org.apache.sysds.runtime.matrix.data;
21+
22+
import org.apache.commons.lang3.ArrayUtils;
23+
import org.apache.sysds.runtime.DMLRuntimeException;
24+
import org.apache.sysds.runtime.compress.CompressedMatrixBlock;
25+
import org.apache.sysds.runtime.compress.lib.CLALibCBind;
26+
import org.apache.sysds.runtime.data.DenseBlock;
27+
import org.apache.sysds.runtime.data.SparseBlock;
28+
import org.apache.sysds.runtime.data.SparseBlockMCSR;
29+
30+
public class LibMatrixAppend {
31+
32+
public static MatrixBlock append(MatrixBlock a, MatrixBlock[] that, MatrixBlock result, boolean cbind) {
33+
int rlen = a.rlen;
34+
int clen = a.clen;
35+
long nonZeros = a.nonZeros;
36+
37+
checkDimensionsForAppend(that, cbind, rlen, clen);
38+
39+
for(int k = 0; k < that.length; k++)
40+
if(that[k] instanceof CompressedMatrixBlock) {
41+
if(that.length == 1 && cbind)
42+
return CLALibCBind.cbind(a, that[0], 1);
43+
that[k] = CompressedMatrixBlock.getUncompressed(that[k], "Append N");
44+
}
45+
46+
final int m = cbind ? rlen : combinedRows(that, rlen);
47+
final int n = cbind ? combinedCols(that, clen) : clen;
48+
final long nnz = calculateCombinedNNz(that, nonZeros);
49+
50+
boolean shallowCopy = (nonZeros == nnz);
51+
boolean sp = MatrixBlock.evalSparseFormatInMemory(m, n, nnz);
52+
53+
// init result matrix
54+
if(result == null)
55+
result = new MatrixBlock(m, n, sp, nnz);
56+
else
57+
result.reset(m, n, sp, nnz);
58+
59+
// core append operation
60+
// copy left and right input into output
61+
if(!result.sparse && nnz != 0)
62+
appendDense(a, that, result, cbind, rlen, m, n);
63+
else if(nnz != 0)
64+
appendSparse(a, that, result, cbind, rlen, clen, nnz, shallowCopy);
65+
66+
// update meta data
67+
result.nonZeros = nnz;
68+
return result;
69+
}
70+
71+
private static void appendSparse(MatrixBlock a, MatrixBlock[] that, MatrixBlock result, boolean cbind, int rlen,
72+
int clen, final long nnz, boolean shallowCopy) {
73+
// adjust sparse rows if required
74+
result.allocateSparseRowsBlock();
75+
// allocate sparse rows once for cbind
76+
if(cbind && nnz > rlen && !shallowCopy && result.getSparseBlock() instanceof SparseBlockMCSR) {
77+
final SparseBlock sblock = result.getSparseBlock();
78+
// for each row calculate how many non zeros are pressent.
79+
for(int i = 0; i < result.rlen; i++)
80+
sblock.allocate(i, computeNNzRow(that, i, a));
81+
82+
}
83+
84+
// core append operation
85+
// we can always append this directly to offset 0.0 in both cbind and rbind.
86+
result.appendToSparse(a, 0, 0, !shallowCopy);
87+
if(cbind) {
88+
for(int i = 0, off = clen; i < that.length; i++) {
89+
result.appendToSparse(that[i], 0, off);
90+
off += that[i].clen;
91+
}
92+
}
93+
else { // rbind
94+
for(int i = 0, off = rlen; i < that.length; i++) {
95+
result.appendToSparse(that[i], off, 0);
96+
off += that[i].rlen;
97+
}
98+
}
99+
}
100+
101+
private static void appendDense(MatrixBlock a, MatrixBlock[] that, MatrixBlock result, boolean cbind, int rlen,
102+
final int m, final int n) {
103+
if(cbind) {
104+
DenseBlock resd = result.allocateBlock().getDenseBlock();
105+
MatrixBlock[] in = ArrayUtils.addAll(new MatrixBlock[] {a}, that);
106+
107+
for(int i = 0; i < m; i++) {
108+
for(int k = 0, off = 0; k < in.length; off += in[k].clen, k++) {
109+
if(in[k].isEmptyBlock(false))
110+
continue;
111+
if(in[k].sparse) {
112+
SparseBlock src = in[k].sparseBlock;
113+
if(src.isEmpty(i))
114+
continue;
115+
int srcpos = src.pos(i);
116+
int srclen = src.size(i);
117+
int[] srcix = src.indexes(i);
118+
double[] srcval = src.values(i);
119+
double[] resval = resd.values(i);
120+
int resix = resd.pos(i, off);
121+
for(int j = srcpos; j < srcpos + srclen; j++)
122+
resval[resix + srcix[j]] = srcval[j];
123+
}
124+
else {
125+
DenseBlock src = in[k].getDenseBlock();
126+
double[] srcval = src.values(i);
127+
double[] resval = resd.values(i);
128+
System.arraycopy(srcval, src.pos(i), resval, resd.pos(i, off), in[k].clen);
129+
}
130+
}
131+
}
132+
}
133+
else { // rbind
134+
result.copy(0, rlen - 1, 0, n - 1, a, false);
135+
for(int i = 0, off = rlen; i < that.length; i++) {
136+
result.copy(off, off + that[i].rlen - 1, 0, n - 1, that[i], false);
137+
off += that[i].rlen;
138+
}
139+
}
140+
}
141+
142+
public static void checkDimensionsForAppend(MatrixBlock[] in, boolean cbind, int rlen, int clen) {
143+
if(cbind) {
144+
for(int i = 0; i < in.length; i++)
145+
if(in[i].rlen != rlen)
146+
throw new DMLRuntimeException(
147+
"Invalid nRow dimension for append cbind: was " + in[i].rlen + " should be: " + rlen);
148+
}
149+
else {
150+
for(int i = 0; i < in.length; i++)
151+
if(in[i].clen != clen)
152+
throw new DMLRuntimeException(
153+
"Invalid nCol dimension for append rbind: was " + in[i].clen + " should be: " + clen);
154+
}
155+
}
156+
157+
private static int combinedRows(MatrixBlock[] that, int rlen) {
158+
int r = rlen;
159+
for(MatrixBlock b : that)
160+
r += b.rlen;
161+
return r;
162+
}
163+
164+
private static int combinedCols(MatrixBlock[] that, int clen) {
165+
int c = clen;
166+
for(MatrixBlock b : that)
167+
c += b.clen;
168+
return c;
169+
}
170+
171+
private static long calculateCombinedNNz(MatrixBlock[] that, long nonZeros) {
172+
long nnz = nonZeros;
173+
for(MatrixBlock b : that)
174+
nnz += b.nonZeros;
175+
return nnz;
176+
}
177+
178+
private static int computeNNzRow(MatrixBlock[] that, int row, MatrixBlock a) {
179+
int lnnz = (int) a.recomputeNonZeros(row, row, 0, a.clen - 1);
180+
for(MatrixBlock b : that)
181+
lnnz += b.recomputeNonZeros(row, row, 0, b.clen - 1);
182+
return lnnz;
183+
}
184+
}

‎src/main/java/org/apache/sysds/runtime/matrix/data/MatrixBlock.java‎

Lines changed: 4 additions & 146 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,6 @@
3838
import java.util.stream.Collectors;
3939
import java.util.stream.IntStream;
4040

41-
import org.apache.commons.lang3.ArrayUtils;
4241
import org.apache.commons.lang3.NotImplementedException;
4342
import org.apache.commons.lang3.concurrent.ConcurrentUtils;
4443
import org.apache.commons.logging.Log;
@@ -56,7 +55,6 @@
5655
import org.apache.sysds.runtime.compress.CompressedMatrixBlock;
5756
import org.apache.sysds.runtime.compress.DMLCompressionException;
5857
import org.apache.sysds.runtime.compress.lib.CLALibAggTernaryOp;
59-
import org.apache.sysds.runtime.compress.lib.CLALibCBind;
6058
import org.apache.sysds.runtime.compress.lib.CLALibMerge;
6159
import org.apache.sysds.runtime.compress.lib.CLALibTernaryOp;
6260
import org.apache.sysds.runtime.controlprogram.caching.CacheBlock;
@@ -891,7 +889,7 @@ public void appendToSparse( MatrixBlock that, int rowoffset, int coloffset ) {
891889
appendToSparse(that, rowoffset, coloffset, true);
892890
}
893891

894-
private void appendToSparse( MatrixBlock that, int rowoffset, int coloffset, boolean deep )
892+
protected void appendToSparse( MatrixBlock that, int rowoffset, int coloffset, boolean deep )
895893
{
896894
if( that==null || that.isEmptyBlock(false) )
897895
return; //nothing to append
@@ -3656,7 +3654,7 @@ public final MatrixBlock append(MatrixBlock that, MatrixBlock ret ) {
36563654
}
36573655

36583656
/**
3659-
* Append that list of matrixblocks to this.
3657+
* Append that list of matrixblocks together.
36603658
*
36613659
* @param that That list.
36623660
* @param ret The output block
@@ -3681,38 +3679,10 @@ public static MatrixBlock append(List<MatrixBlock> that, MatrixBlock ret, boolea
36813679
* @param cbind if binding on columns or rows
36823680
* @return the ret MatrixBlock object with the appended result
36833681
*/
3684-
public final MatrixBlock append( MatrixBlock that, MatrixBlock ret, boolean cbind ) {
3682+
public final MatrixBlock append(MatrixBlock that, MatrixBlock ret, boolean cbind ) {
36853683
return append(new MatrixBlock[]{that}, ret, cbind);
36863684
}
36873685

3688-
private final long calculateCombinedNNz(MatrixBlock[] that){
3689-
long nnz = nonZeros;
3690-
for(MatrixBlock b : that)
3691-
nnz += b.nonZeros;
3692-
return nnz;
3693-
}
3694-
3695-
private final int combinedRows(MatrixBlock[] that){
3696-
int r = rlen;
3697-
for(MatrixBlock b : that)
3698-
r += b.rlen;
3699-
return r;
3700-
}
3701-
3702-
private final int combinedCols(MatrixBlock[] that){
3703-
int c = clen;
3704-
for(MatrixBlock b : that)
3705-
c += b.clen;
3706-
return c;
3707-
}
3708-
3709-
private final int computeNNzRow(MatrixBlock[] that, int row) {
3710-
int lnnz = (int) this.recomputeNonZeros(row, row, 0, this.clen - 1);
3711-
for(MatrixBlock b : that)
3712-
lnnz += b.recomputeNonZeros(row, row, 0, b.clen - 1);
3713-
return lnnz;
3714-
}
3715-
37163686
/**
37173687
* Append that list of matrixes to this matrix.
37183688
*
@@ -3724,119 +3694,7 @@ private final int computeNNzRow(MatrixBlock[] that, int row) {
37243694
* @return the ret MatrixBlock object with the appended result
37253695
*/
37263696
public MatrixBlock append(MatrixBlock[] that, MatrixBlock result, boolean cbind) {
3727-
checkDimensionsForAppend(that, cbind);
3728-
3729-
for(int k = 0; k < that.length; k++)
3730-
if( that[k] instanceof CompressedMatrixBlock){
3731-
if(that.length == 1 && cbind)
3732-
return CLALibCBind.cbind(this, that[0], 1);
3733-
that[k] = CompressedMatrixBlock.getUncompressed(that[k], "Append N");
3734-
}
3735-
3736-
final int m = cbind ? rlen : combinedRows(that);
3737-
final int n = cbind ? combinedCols(that) : clen;
3738-
final long nnz = calculateCombinedNNz(that);
3739-
3740-
boolean shallowCopy = (nonZeros == nnz);
3741-
boolean sp = evalSparseFormatInMemory(m, n, nnz);
3742-
3743-
//init result matrix
3744-
if( result == null )
3745-
result = new MatrixBlock(m, n, sp, nnz);
3746-
else
3747-
result.reset(m, n, sp, nnz);
3748-
3749-
//core append operation
3750-
//copy left and right input into output
3751-
if( !result.sparse && nnz!=0 ) //DENSE
3752-
{
3753-
if( cbind ) {
3754-
DenseBlock resd = result.allocateBlock().getDenseBlock();
3755-
MatrixBlock[] in = ArrayUtils.addAll(new MatrixBlock[]{this}, that);
3756-
3757-
for( int i=0; i<m; i++ ) {
3758-
for( int k=0, off=0; k<in.length; off+=in[k].clen, k++ ) {
3759-
if( in[k].isEmptyBlock(false) )
3760-
continue;
3761-
if( in[k].sparse ) {
3762-
SparseBlock src = in[k].sparseBlock;
3763-
if( src.isEmpty(i) )
3764-
continue;
3765-
int srcpos = src.pos(i);
3766-
int srclen = src.size(i);
3767-
int[] srcix = src.indexes(i);
3768-
double[] srcval = src.values(i);
3769-
double[] resval = resd.values(i);
3770-
int resix = resd.pos(i, off);
3771-
for (int j=srcpos; j<srcpos+srclen; j++)
3772-
resval[resix+srcix[j]] = srcval[j];
3773-
}
3774-
else {
3775-
DenseBlock src = in[k].getDenseBlock();
3776-
double[] srcval = src.values(i);
3777-
double[] resval = resd.values(i);
3778-
System.arraycopy(srcval, src.pos(i),
3779-
resval, resd.pos(i, off), in[k].clen);
3780-
}
3781-
}
3782-
}
3783-
}
3784-
else { //rbind
3785-
result.copy(0, rlen-1, 0, n-1, this, false);
3786-
for(int i=0, off=rlen; i<that.length; i++) {
3787-
result.copy(off, off+that[i].rlen-1, 0, n-1, that[i], false);
3788-
off += that[i].rlen;
3789-
}
3790-
}
3791-
}
3792-
//SPARSE
3793-
else if(nnz != 0) {
3794-
//adjust sparse rows if required
3795-
result.allocateSparseRowsBlock();
3796-
//allocate sparse rows once for cbind
3797-
if( cbind && nnz > rlen && !shallowCopy && result.getSparseBlock() instanceof SparseBlockMCSR ) {
3798-
final SparseBlock sblock = result.getSparseBlock();
3799-
// for each row calculate how many non zeros are pressent.
3800-
for( int i=0; i<result.rlen; i++ )
3801-
sblock.allocate(i, computeNNzRow(that, i));
3802-
3803-
}
3804-
3805-
//core append operation
3806-
// we can always append this directly to offset 0.0 in both cbind and rbind.
3807-
result.appendToSparse(this, 0, 0, !shallowCopy);
3808-
if( cbind ) {
3809-
for(int i=0, off=clen; i<that.length; i++) {
3810-
result.appendToSparse(that[i], 0, off);
3811-
off += that[i].clen;
3812-
}
3813-
}
3814-
else { //rbind
3815-
for(int i=0, off=rlen; i<that.length; i++) {
3816-
result.appendToSparse(that[i], off, 0);
3817-
off += that[i].rlen;
3818-
}
3819-
}
3820-
}
3821-
3822-
//update meta data
3823-
result.nonZeros = nnz;
3824-
return result;
3825-
}
3826-
3827-
public void checkDimensionsForAppend(MatrixBlock[] in, boolean cbind) {
3828-
if(cbind) {
3829-
for(int i = 0; i < in.length; i++)
3830-
if(in[i].rlen != rlen)
3831-
throw new DMLRuntimeException(
3832-
"Invalid nRow dimension for append cbind: was " + in[i].rlen + " should be: " + rlen);
3833-
}
3834-
else {
3835-
for(int i = 0; i < in.length; i++)
3836-
if(in[i].clen != clen)
3837-
throw new DMLRuntimeException(
3838-
"Invalid nCol dimension for append rbind: was " + in[i].clen + " should be: " + clen);
3839-
}
3697+
return LibMatrixAppend.append(this, that, result, cbind);
38403698
}
38413699

38423700
public static MatrixBlock naryOperations(Operator op, MatrixBlock[] matrices, ScalarObject[] scalars, MatrixBlock ret) {

‎src/test/java/org/apache/sysds/test/component/compress/ExtendedMatrixTests.java‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -200,8 +200,6 @@ public void testProd() {
200200
if(!(cmb instanceof CompressedMatrixBlock))
201201
return;
202202
double ret1 = cmb.prod();
203-
LOG.error(ret1);
204-
LOG.error(cmb);
205203
double ret2 = mb.prod();
206204
boolean res;
207205
if(_cs != null && (_cs.lossy || overlappingType == OverLapping.SQUASH))

0 commit comments

Comments
 (0)