Skip to content

Commit aaa0192

Browse files
committed
[MINOR] Fix edgecase of Compressed OffsetSlice
This commit fixes an edgecase of offset slices where a single element is sliced. Closes #2163 Signed-off-by: Sebastian Baunsgaard <[email protected]>
1 parent e34286d commit aaa0192

File tree

3 files changed

+81
-15
lines changed

3 files changed

+81
-15
lines changed

src/main/java/org/apache/sysds/runtime/compress/colgroup/offset/AOffset.java

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -569,6 +569,8 @@ public OffsetSliceInfo slice(int l, int u) {
569569
else
570570
return new OffsetSliceInfo(0, s, moveIndex(l));
571571
}
572+
else if (u < first)
573+
return EMPTY_SLICE;
572574

573575
final AIterator it = getIteratorSkipCache(l);
574576

@@ -587,6 +589,7 @@ private OffsetSliceInfo genericSlice(int l, int u, AIterator it) {
587589
final int low = it.getDataIndex();
588590
final int lowOff = it.getOffsetsIndex();
589591
final int lowValue = it.value();
592+
590593
// set c
591594
int high = low;
592595
int highOff = lowOff;
@@ -607,7 +610,7 @@ private OffsetSliceInfo genericSlice(int l, int u, AIterator it) {
607610

608611
private final OffsetSliceInfo constructSliceReturn(int l, int u, int low, int high, int lowOff, int highOff,
609612
int lowValue, int highValue) {
610-
if(low == high)
613+
if(low == high) // Implicit lowValue == highValue
611614
return new OffsetSliceInfo(low, high + 1, new OffsetSingle(lowValue - l));
612615
else if(low + 1 == high)
613616
return new OffsetSliceInfo(low, high + 1, new OffsetTwo(lowValue - l, highValue - l));

src/main/java/org/apache/sysds/runtime/compress/colgroup/offset/OffsetTwo.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ public static OffsetTwo readFields(DataInput in) throws IOException {
9797
@Override
9898
public OffsetSliceInfo slice(int l, int u) {
9999
if(l <= first) {
100-
if(u < first)
100+
if(u <= first)
101101
return EMPTY_SLICE;
102102
else if(u > last)
103103
return new OffsetSliceInfo(0, 2, moveIndex(l));

src/test/java/org/apache/sysds/test/component/compress/offset/OffsetTests.java

Lines changed: 76 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@
5858

5959
@RunWith(value = Parameterized.class)
6060
public class OffsetTests {
61-
static{
61+
static {
6262
CompressedMatrixBlock.debug = true;
6363
}
6464

@@ -77,6 +77,10 @@ public static Collection<Object[]> data() {
7777
for(OFF_TYPE t : OFF_TYPE.values()) {
7878
tests.add(new Object[] {new int[] {}, t});
7979
tests.add(new Object[] {new int[] {1, 2}, t});
80+
tests.add(new Object[] {new int[] {1, 3, 5, 7}, t});
81+
tests.add(new Object[] {new int[] {2, 3, 6, 7}, t});
82+
tests.add(new Object[] {new int[] {2, 5, 10, 12}, t});
83+
tests.add(new Object[] {new int[] {2, 6, 8, 14}, t});
8084
tests.add(new Object[] {new int[] {2, 142}, t});
8185
tests.add(new Object[] {new int[] {142, 421}, t});
8286
tests.add(new Object[] {new int[] {1, 1023}, t});
@@ -562,6 +566,46 @@ public void sliceFirst100() {
562566
slice(0, 100);
563567
}
564568

569+
@Test
570+
public void slice1_2() {
571+
slice(1, 2);
572+
}
573+
574+
@Test
575+
public void slice3_4() {
576+
slice(3, 4);
577+
}
578+
579+
@Test
580+
public void sliceRange() {
581+
for(int i = 0; i < 100; i++) {
582+
// not allowed to slice smaller or equal range.
583+
// slice(i, i - 1);
584+
// slice(i, i);
585+
slice(i, i + 1);
586+
slice(i, i + 2);
587+
slice(i, i + 3);
588+
}
589+
}
590+
591+
@Test
592+
public void sliceRange256() {
593+
slice(250, 260);
594+
slice(254, 260);
595+
slice(254, 257);
596+
slice(255, 257);
597+
slice(255, 256);
598+
}
599+
600+
@Test
601+
public void sliceRange2x256() {
602+
slice(250 * 2, 260 * 2);
603+
slice(254 * 2, 260 * 2);
604+
slice(254 * 2, 257 * 2);
605+
slice(255 * 2, 257 * 2);
606+
slice(255 * 2, 256 * 2);
607+
}
608+
565609
@Test
566610
public void sliceToLast() {
567611
if(data.length > 1)
@@ -579,11 +623,6 @@ public void slice100to10000() {
579623
slice(100, 10000);
580624
}
581625

582-
@Test
583-
public void verify() {
584-
o.verify(o.getSize());
585-
}
586-
587626
@Test
588627
public void slice1to4() {
589628
slice(1, 4);
@@ -594,7 +633,7 @@ public void slice() {
594633
if(data.length > 1) {
595634
int n = data[data.length - 1];
596635
for(int i = 0; i < n && i < 100; i++) {
597-
for(int j = i; j < n + 1 && j < 100; j++) {
636+
for(int j = i+1; j < n + 1 && j < 100; j++) {
598637
slice(i, j, false);
599638
}
600639
}
@@ -624,12 +663,28 @@ private void slice(int l, int u, boolean str) {
624663
int i = 0;
625664
while(i < data.length && data[i] < l)
626665
i++;
627-
628-
while(data[i] < u) {
629-
assertEquals(data[i] - l, it.value());
630-
if(a.offsetSlice.getOffsetToLast() > it.value())
631-
it.next();
632-
i++;
666+
667+
if(! (a.offsetSlice instanceof OffsetEmpty)){
668+
int t = 0;
669+
final int lasstSliceOffset = a.offsetSlice.getOffsetToLast();
670+
while(data[i] < u) {
671+
assertEquals(data[i] - l, it.value());
672+
if(lasstSliceOffset > it.value())
673+
it.next();
674+
i++;
675+
t++;
676+
}
677+
678+
int sliceSize = a.offsetSlice.getSize();
679+
if(sliceSize != t) {
680+
fail("Slice size is not equal to elements that should have been sliced:\n" + sliceSize + " vs " + t
681+
+ "\n" + a + "\nrange: " + l + " " + u + "\ninput " + o);
682+
}
683+
}
684+
else {
685+
if( i < data.length){
686+
assertTrue(data[i] >= u);
687+
}
633688
}
634689
}
635690
}
@@ -640,6 +695,14 @@ private void slice(int l, int u, boolean str) {
640695

641696
}
642697

698+
699+
@Test
700+
public void verify() {
701+
o.verify(o.getSize());
702+
}
703+
704+
705+
643706
@Test
644707
public void append() {
645708
if(data.length > 0) {

0 commit comments

Comments
 (0)