Merge pull request #629 from apache/fix_bit_packing

fixed bit packing
apache · Jan 9, 2025 · 6df0c11 · 6df0c11
2 parents 8604dc3 + 2d2b69d
commit 6df0c11
Show file tree

Hide file tree

Showing 2 changed files with 49 additions and 47 deletions.
diff --git a/src/main/java/org/apache/datasketches/theta/BitPacking.java b/src/main/java/org/apache/datasketches/theta/BitPacking.java
@@ -482,7 +482,7 @@ static void packBits13(final long[] values, final int i, final byte[] buf, int o
 
     buf[off++] = (byte) (values[i + 3] >>> 4);
 
-    buf[off] = (byte) (values[i + 3] >>> 4);
+    buf[off] = (byte) (values[i + 3] << 4);
     buf[off++] |= values[i + 4] >>> 9;
 
     buf[off++] = (byte) (values[i + 4] >>> 1);
@@ -4449,7 +4449,7 @@ static void unpackBits35(final long[] values, final int i, final byte[] buf, int
     values[i + 1] |= Byte.toUnsignedLong(buf[off++]) << 6;
     values[i + 1] |= Byte.toUnsignedLong(buf[off]) >>> 2;
 
-    values[i + 2] = (Byte.toUnsignedLong(buf[off++]) & 2) << 33;
+    values[i + 2] = (Byte.toUnsignedLong(buf[off++]) & 3) << 33;
     values[i + 2] |= (Byte.toUnsignedLong(buf[off++])) << 25;
     values[i + 2] |= Byte.toUnsignedLong(buf[off++]) << 17;
     values[i + 2] |= Byte.toUnsignedLong(buf[off++]) << 9;

diff --git a/src/test/java/org/apache/datasketches/theta/BitPackingTest.java b/src/test/java/org/apache/datasketches/theta/BitPackingTest.java
@@ -31,60 +31,62 @@ public class BitPackingTest {
 
   @Test
   public void packUnpackBits() {
-    for (int bits = 1; bits <= 63; bits++) {
-      final long mask = (1 << bits) - 1;
-      long[] input = new long[8];
-      final long golden64 = Util.INVERSE_GOLDEN_U64;
-      long value = 0xaa55aa55aa55aa55L; // arbitrary starting value
-      for (int i = 0; i < 8; ++i) {
-        input[i] = value & mask;
-        value += golden64;
-      }
-      byte[] bytes = new byte[8 * Long.BYTES];
-      int bitOffset = 0;
-      int bufOffset = 0;
-      for (int i = 0; i < 8; ++i) {
-        BitPacking.packBits(input[i], bits, bytes, bufOffset, bitOffset);
-        bufOffset += (bitOffset + bits) >>> 3;
-        bitOffset = (bitOffset + bits) & 7;
-      }
+    long value = 0xaa55aa55aa55aa55L; // arbitrary starting value
+    for (int n = 0; n < 100; n++) {
+      for (int bits = 1; bits <= 63; bits++) {
+        final long mask = (1 << bits) - 1;
+        long[] input = new long[8];
+        for (int i = 0; i < 8; ++i) {
+          input[i] = value & mask;
+          value += Util.INVERSE_GOLDEN_U64;
+        }
+        byte[] bytes = new byte[8 * Long.BYTES];
+        int bitOffset = 0;
+        int bufOffset = 0;
+        for (int i = 0; i < 8; ++i) {
+          BitPacking.packBits(input[i], bits, bytes, bufOffset, bitOffset);
+          bufOffset += (bitOffset + bits) >>> 3;
+          bitOffset = (bitOffset + bits) & 7;
+        }
 
-      long[] output = new long[8];
-      bitOffset = 0;
-      bufOffset = 0;
-      for (int i = 0; i < 8; ++i) {
-        BitPacking.unpackBits(output, i, bits, bytes, bufOffset, bitOffset);
-        bufOffset += (bitOffset + bits) >>> 3;
-        bitOffset = (bitOffset + bits) & 7;
-      }
-      for (int i = 0; i < 8; ++i) {
-        assertEquals(output[i], input[i]);
+        long[] output = new long[8];
+        bitOffset = 0;
+        bufOffset = 0;
+        for (int i = 0; i < 8; ++i) {
+          BitPacking.unpackBits(output, i, bits, bytes, bufOffset, bitOffset);
+          bufOffset += (bitOffset + bits) >>> 3;
+          bitOffset = (bitOffset + bits) & 7;
+        }
+        for (int i = 0; i < 8; ++i) {
+          assertEquals(output[i], input[i]);
+        }
       }
     }
   }
 
   @Test
   public void packUnpackBlocks() {
-    for (int bits = 1; bits <= 63; bits++) {
-      if (enablePrinting) { System.out.println("bits " + bits); }
-      final long mask = (1L << bits) - 1;
-      long[] input = new long[8];
-      final long golden64 = Util.INVERSE_GOLDEN_U64;
-      long value = 0xaa55aa55aa55aa55L; // arbitrary starting value
-      for (int i = 0; i < 8; ++i) {
-        input[i] = value & mask;
-        value += golden64;
-      }
-      byte[] bytes = new byte[8 * Long.BYTES];
-      BitPacking.packBitsBlock8(input, 0, bytes, 0, bits);
-      if (enablePrinting) { hexDump(bytes); }
+    long value = 0xaa55aa55aa55aa55L; // arbitrary starting value
+    for (int n = 0; n < 100; n++) {
+      for (int bits = 1; bits <= 63; bits++) {
+        if (enablePrinting) { System.out.println("bits " + bits); }
+        final long mask = (1L << bits) - 1;
+        long[] input = new long[8];
+        for (int i = 0; i < 8; ++i) {
+          input[i] = value & mask;
+          value += Util.INVERSE_GOLDEN_U64;
+        }
+        byte[] bytes = new byte[8 * Long.BYTES];
+        BitPacking.packBitsBlock8(input, 0, bytes, 0, bits);
+        if (enablePrinting) { hexDump(bytes); }
 
-      long[] output = new long[8];
-      BitPacking.unpackBitsBlock8(output, 0, bytes, 0, bits);
+        long[] output = new long[8];
+        BitPacking.unpackBitsBlock8(output, 0, bytes, 0, bits);
 
-      for (int i = 0; i < 8; ++i) {
-        if (enablePrinting) { System.out.println("checking value " + i); }
-        assertEquals(output[i], input[i]);
+        for (int i = 0; i < 8; ++i) {
+          if (enablePrinting) { System.out.println("checking value " + i); }
+          assertEquals(output[i], input[i]);
+        }
       }
     }
   }