You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@datasketches.apache.org by GitBox <gi...@apache.org> on 2021/03/18 08:27:58 UTC
[GitHub] [datasketches-cpp] marco6 opened a new issue #205: Empty kll sketches behavior
marco6 opened a new issue #205:
URL: https://github.com/apache/datasketches-cpp/issues/205
Hi! I've been experimenting with the kll sketch lately, and I don't understand the behavior of the ["empty"](https://github.com/apache/datasketches-cpp/blob/master/kll/test/kll_sketch_test.cpp#L57) test.
On [line 73](https://github.com/apache/datasketches-cpp/blob/master/kll/test/kll_sketch_test.cpp#L73) I see a counting loop and a `REQUIRE(count == 0);` that in my mind means "it should never enter the loop", but I was surprised that on my machine (Windows 10, Visual Studio 19 Community latest version, both Release and Debug configuration, x64 platform) that is not the case: it just loops _until count overflows_ and than exits the loop. This makes the test "pass", but (apart from overflow being UB) I don't think it's the right behavior.
To make sure I'm not missing something, I changed the test with
```diff
diff --git a/kll/test/kll_sketch_test.cpp b/kll/test/kll_sketch_test.cpp
index def96f6..c29c135 100644
--- a/kll/test/kll_sketch_test.cpp
+++ b/kll/test/kll_sketch_test.cpp
@@ -74,6 +74,7 @@ TEST_CASE("kll sketch", "[kll_sketch]") {
for (auto& it: sketch) {
(void) it; // to suppress "unused" warning
++count;
+ FAIL("the sketch should be empty");
}
REQUIRE(count == 0);
}
```
And the test immediately fails.
I also tested this behavior using GCC (on a WSL2 machine, Ubuntu 20.04, g++ 9.3.0) and the test fails, too.
Am I missing something?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For additional commands, e-mail: commits-help@datasketches.apache.org
[GitHub] [datasketches-cpp] AlexanderSaydakov commented on issue #205: Empty kll sketches behavior
Posted by GitBox <gi...@apache.org>.
AlexanderSaydakov commented on issue #205:
URL: https://github.com/apache/datasketches-cpp/issues/205#issuecomment-802239235
You are right. I see that iteration over an empty sketch is not working correctly. We will work on a fix. In the meantime as a workaround I suggest checking is_empty() before iterating. Thank you for reporting this.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For additional commands, e-mail: commits-help@datasketches.apache.org
[GitHub] [datasketches-cpp] AlexanderSaydakov closed issue #205: Empty kll sketches behavior
Posted by GitBox <gi...@apache.org>.
AlexanderSaydakov closed issue #205:
URL: https://github.com/apache/datasketches-cpp/issues/205
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For additional commands, e-mail: commits-help@datasketches.apache.org