From 5c1933342d7a6b1ad957a5e670c9693946ab8810 Mon Sep 17 00:00:00 2001 From: AlexanderSaydakov Date: Thu, 9 Jan 2025 10:39:47 -0800 Subject: [PATCH 1/3] 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 85157889..284239f5 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(values[3] >> 4); - *ptr = static_cast(values[3] >> 4); + *ptr = static_cast(values[3] << 4); *ptr++ |= static_cast(values[4] >> 9); *ptr++ = static_cast(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(*ptr++ & 1) << 32; - values[7] |= *ptr++ << 24; + values[7] |= static_cast(*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(*ptr++ & 2) << 33; + values[2] = static_cast(*ptr++ & 3) << 33; values[2] |= static_cast(*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 2e25a4af..b7ecf63e 100644 --- a/theta/test/bit_packing_test.cpp +++ b/theta/test/bit_packing_test.cpp @@ -17,6 +17,8 @@ * under the License. */ +#include + #include #include @@ -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 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 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 input(n, 0); + for (int i = 0; i < n; ++i) { + input[i] = value & mask; + value += IGOLDEN64; + } + std::vector 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 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 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 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 bytes(8 * sizeof(uint64_t), 0); - pack_bits_block8(input.data(), bytes.data(), bits); - std::vector 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 input(8, 0); + for (int i = 0; i < 8; ++i) { + input[i] = value & mask; + value += IGOLDEN64; + } + std::vector bytes(bits, 0); + pack_bits_block8(input.data(), bytes.data(), bits); + std::vector output(8, 0); + unpack_bits_block8(output.data(), bytes.data(), bits); + for (int i = 0; i < 8; ++i) { + REQUIRE(input[i] == output[i]); + } } } } From 666df8b5e6f787a90ad123a72322acbae566ac83 Mon Sep 17 00:00:00 2001 From: AlexanderSaydakov Date: Thu, 9 Jan 2025 10:42:42 -0800 Subject: [PATCH 2/3] unused import --- theta/test/bit_packing_test.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/theta/test/bit_packing_test.cpp b/theta/test/bit_packing_test.cpp index b7ecf63e..ef9eb880 100644 --- a/theta/test/bit_packing_test.cpp +++ b/theta/test/bit_packing_test.cpp @@ -17,8 +17,6 @@ * under the License. */ -#include - #include #include From 6e944cfded9f28ce8435b198213306eb69e61053 Mon Sep 17 00:00:00 2001 From: AlexanderSaydakov Date: Thu, 9 Jan 2025 12:33:56 -0800 Subject: [PATCH 3/3] fixed name clash --- theta/test/bit_packing_test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/theta/test/bit_packing_test.cpp b/theta/test/bit_packing_test.cpp index ef9eb880..bc9e08f5 100644 --- a/theta/test/bit_packing_test.cpp +++ b/theta/test/bit_packing_test.cpp @@ -30,7 +30,7 @@ static const uint64_t IGOLDEN64 = 0x9e3779b97f4a7c13ULL; TEST_CASE("pack unpack bits") { uint64_t value = 0xaa55aa55aa55aa55ULL; // arbitrary starting value - for (int n = 0; n < 100; ++n) { + for (int m = 0; m < 100; ++m) { for (uint8_t bits = 1; bits <= 63; ++bits) { int n = 8; const uint64_t mask = (1ULL << bits) - 1;