This is an automated email from the ASF dual-hosted git repository.

alsay pushed a commit to branch fix_bit_packing
in repository https://gitbox.apache.org/repos/asf/datasketches-cpp.git

commit 5c1933342d7a6b1ad957a5e670c9693946ab8810
Author: AlexanderSaydakov <[email protected]>
AuthorDate: Thu Jan 9 10:39:47 2025 -0800

    fixed bit packing
---
 theta/include/bit_packing.hpp   |  6 +--
 theta/test/bit_packing_test.cpp | 82 +++++++++++++++++++++--------------------
 2 files changed, 46 insertions(+), 42 deletions(-)

diff --git a/theta/include/bit_packing.hpp b/theta/include/bit_packing.hpp
index 8515788..284239f 100644
--- a/theta/include/bit_packing.hpp
+++ b/theta/include/bit_packing.hpp
@@ -329,7 +329,7 @@ static inline void pack_bits_13(const uint64_t* values, 
uint8_t* ptr) {
 
   *ptr++ = static_cast<uint8_t>(values[3] >> 4);
 
-  *ptr = static_cast<uint8_t>(values[3] >> 4);
+  *ptr = static_cast<uint8_t>(values[3] << 4);
   *ptr++ |= static_cast<uint8_t>(values[4] >> 9);
 
   *ptr++ = static_cast<uint8_t>(values[4] >> 1);
@@ -4227,7 +4227,7 @@ static inline void unpack_bits_33(uint64_t* values, const 
uint8_t* ptr) {
   values[6] |= *ptr >> 1;
 
   values[7] = static_cast<uint64_t>(*ptr++ & 1) << 32;
-  values[7] |= *ptr++ << 24;
+  values[7] |= static_cast<uint64_t>(*ptr++) << 24;
   values[7] |= *ptr++ << 16;
   values[7] |= *ptr++ << 8;
   values[7] |= *ptr;
@@ -4296,7 +4296,7 @@ static inline void unpack_bits_35(uint64_t* values, const 
uint8_t* ptr) {
   values[1] |= *ptr++ << 6;
   values[1] |= *ptr >> 2;
 
-  values[2] = static_cast<uint64_t>(*ptr++ & 2) << 33;
+  values[2] = static_cast<uint64_t>(*ptr++ & 3) << 33;
   values[2] |= static_cast<uint64_t>(*ptr++) << 25;
   values[2] |= *ptr++ << 17;
   values[2] |= *ptr++ << 9;
diff --git a/theta/test/bit_packing_test.cpp b/theta/test/bit_packing_test.cpp
index 2e25a4a..b7ecf63 100644
--- a/theta/test/bit_packing_test.cpp
+++ b/theta/test/bit_packing_test.cpp
@@ -17,6 +17,8 @@
  * under the License.
  */
 
+#include <iostream>
+
 #include <catch2/catch.hpp>
 #include <bit_packing.hpp>
 
@@ -29,51 +31,53 @@ namespace datasketches {
 static const uint64_t IGOLDEN64 = 0x9e3779b97f4a7c13ULL;
 
 TEST_CASE("pack unpack bits") {
-  for (uint8_t bits = 1; bits <= 63; ++bits) {
-    int n = 8;
-    const uint64_t mask = (1ULL << bits) - 1;
-    std::vector<uint64_t> input(n, 0);
-    const uint64_t igolden64 = IGOLDEN64;
-    uint64_t value = 0xaa55aa55aa55aa55ULL; // arbitrary starting value
-    for (int i = 0; i < n; ++i) {
-      input[i] = value & mask;
-      value += igolden64;
-    }
-    std::vector<uint8_t> bytes(n * sizeof(uint64_t), 0);
-    uint8_t offset = 0;
-    uint8_t* ptr = bytes.data();
-    for (int i = 0; i < n; ++i) {
-      offset = pack_bits(input[i], bits, ptr, offset);
-    }
+  uint64_t value = 0xaa55aa55aa55aa55ULL; // arbitrary starting value
+  for (int n = 0; n < 100; ++n) {
+    for (uint8_t bits = 1; bits <= 63; ++bits) {
+      int n = 8;
+      const uint64_t mask = (1ULL << bits) - 1;
+      std::vector<uint64_t> input(n, 0);
+      for (int i = 0; i < n; ++i) {
+        input[i] = value & mask;
+        value += IGOLDEN64;
+      }
+      std::vector<uint8_t> bytes(n * sizeof(uint64_t), 0);
+      uint8_t offset = 0;
+      uint8_t* ptr = bytes.data();
+      for (int i = 0; i < n; ++i) {
+        offset = pack_bits(input[i], bits, ptr, offset);
+      }
 
-    std::vector<uint64_t> output(n, 0);
-    offset = 0;
-    const uint8_t* cptr = bytes.data();
-    for (int i = 0; i < n; ++i) {
-      offset = unpack_bits(output[i], bits, cptr, offset);
-    }
-    for (int i = 0; i < n; ++i) {
-      REQUIRE(input[i] == output[i]);
+      std::vector<uint64_t> output(n, 0);
+      offset = 0;
+      const uint8_t* cptr = bytes.data();
+      for (int i = 0; i < n; ++i) {
+        offset = unpack_bits(output[i], bits, cptr, offset);
+      }
+      for (int i = 0; i < n; ++i) {
+        REQUIRE(input[i] == output[i]);
+      }
     }
   }
 }
 
 TEST_CASE("pack unpack blocks") {
-  for (uint8_t bits = 1; bits <= 63; ++bits) {
-    const uint64_t mask = (1ULL << bits) - 1;
-    std::vector<uint64_t> input(8, 0);
-    const uint64_t igolden64 = IGOLDEN64;
-    uint64_t value = 0xaa55aa55aa55aa55ULL; // arbitrary starting value
-    for (int i = 0; i < 8; ++i) {
-      input[i] = value & mask;
-      value += igolden64;
-    }
-    std::vector<uint8_t> bytes(8 * sizeof(uint64_t), 0);
-    pack_bits_block8(input.data(), bytes.data(), bits);
-    std::vector<uint64_t> output(8, 0);
-    unpack_bits_block8(output.data(), bytes.data(), bits);
-    for (int i = 0; i < 8; ++i) {
-      REQUIRE((input[i] & mask) == output[i]);
+  uint64_t value = 0xaa55aa55aa55aa55ULL; // arbitrary starting value
+  for (int n = 0; n < 100; ++n) {
+    for (uint8_t bits = 1; bits <= 63; ++bits) {
+      const uint64_t mask = (1ULL << bits) - 1;
+      std::vector<uint64_t> input(8, 0);
+      for (int i = 0; i < 8; ++i) {
+        input[i] = value & mask;
+        value += IGOLDEN64;
+      }
+      std::vector<uint8_t> bytes(bits, 0);
+      pack_bits_block8(input.data(), bytes.data(), bits);
+      std::vector<uint64_t> output(8, 0);
+      unpack_bits_block8(output.data(), bytes.data(), bits);
+      for (int i = 0; i < 8; ++i) {
+        REQUIRE(input[i] == output[i]);
+      }
     }
   }
 }


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to