janniklinde commented on code in PR #2361:
URL: https://github.com/apache/systemds/pull/2361#discussion_r2559990365
##########
src/main/java/org/apache/sysds/runtime/compress/colgroup/ColGroupDeltaDDC.java:
##########
@@ -19,62 +19,107 @@
package org.apache.sysds.runtime.compress.colgroup;
+import org.apache.commons.lang3.NotImplementedException;
+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.colgroup.dictionary.DeltaDictionary;
+import org.apache.sysds.runtime.compress.colgroup.dictionary.IDictionary;
+import org.apache.sysds.runtime.compress.colgroup.indexes.IColIndex;
+import org.apache.sysds.runtime.compress.colgroup.mapping.AMapToData;
+import org.apache.sysds.runtime.data.DenseBlock;
+import org.apache.sysds.runtime.data.SparseBlock;
+import org.apache.sysds.runtime.matrix.operators.ScalarOperator;
+
/**
* Class to encapsulate information about a column group that is first delta
encoded then encoded with dense dictionary
* encoding (DeltaDDC).
*/
-public class ColGroupDeltaDDC { // extends ColGroupDDC
+public class ColGroupDeltaDDC extends ColGroupDDC {
+ private static final long serialVersionUID = -1045556313148564147L;
-// private static final long serialVersionUID = -1045556313148564147L;
+ /** Constructor for serialization */
+ protected ColGroupDeltaDDC() {
+ super();
+ }
-// /** Constructor for serialization */
-// protected ColGroupDeltaDDC() {
-// }
+ private ColGroupDeltaDDC(IColIndex colIndexes, IDictionary dict,
AMapToData data, int[] cachedCounts) {
+ super(colIndexes, dict, data, cachedCounts);
+ if(CompressedMatrixBlock.debug) {
+ if(!(dict instanceof DeltaDictionary))
+ throw new DMLCompressionException("DeltaDDC
must use DeltaDictionary");
+ }
+ }
-// private ColGroupDeltaDDC(int[] colIndexes, ADictionary dict, AMapToData
data, int[] cachedCounts) {
-// super();
-// LOG.info("Carefully use of DeltaDDC since implementation is not
finished.");
-// _colIndexes = colIndexes;
-// _dict = dict;
-// _data = data;
-// }
+ public static AColGroup create(IColIndex colIndexes, IDictionary dict,
AMapToData data, int[] cachedCounts) {
+ if(data.getUnique() == 1)
+ return ColGroupConst.create(colIndexes, dict);
+ else if(dict == null)
+ return new ColGroupEmpty(colIndexes);
+ else
+ return new ColGroupDeltaDDC(colIndexes, dict, data,
cachedCounts);
+ }
-// public static AColGroup create(int[] colIndices, ADictionary dict,
AMapToData data, int[] cachedCounts) {
-// if(dict == null)
-// throw new NotImplementedException("Not implemented
constant delta group");
-// else
-// return new ColGroupDeltaDDC(colIndices, dict, data,
cachedCounts);
-// }
+ @Override
+ public CompressionType getCompType() {
+ return CompressionType.DeltaDDC;
+ }
-// public CompressionType getCompType() {
-// return CompressionType.DeltaDDC;
-// }
+ @Override
+ protected void decompressToDenseBlockDenseDictionary(DenseBlock db, int
rl, int ru, int offR, int offC,
+ double[] values) {
+ final int nCol = _colIndexes.size();
+ final double[] prevRow = new double[nCol];
+
+ if(rl > 0) {
+ final double[] prevRowData = db.values(rl - 1 + offR);
+ final int prevOff = db.pos(rl - 1 + offR) + offC;
+ for(int j = 0; j < nCol; j++) {
+ prevRow[j] = prevRowData[prevOff +
_colIndexes.get(j)];
+ }
+ }
Review Comment:
The handling of the case `rl > 0` is incorrect. The intent of this method is
to copy the data from the compressed colgroup into the `DenseBlock` with some
offset. The `DenseBlock` does not hold the previous row data as is assumed by
your code. Instead, you would have to aggregate from row = 0 to row = rl to now
the contents of the previous row.
You should also add some tests to cover that case.
--
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]
