Add approximate counting C++ sample code #834
Conversation
Amaras
added
the
Implementation
|
Thanks for the PR. |
| auto approximate_count(int n_items, int a) { | ||
| auto v = 0; | ||
| for (auto i = 0; i < n_items; ++i) | ||
| v = increment(v, a); |
There was a problem hiding this comment.
I have a warning for double to int conversion, because v is of type int and increment returns a double, which would round down the values.
I'm guessing it should be auto v = 0.0 instead?
There was a problem hiding this comment.
I want to reproduce the issue, what compiler are you using, and compilation flags?
| // - n_items: number of items to count and loop over | ||
| // - a: a scaling value for the logarithm based on Morris's paper | ||
| // It returns n(v,a), the approximate count | ||
| auto approximate_count(int n_items, int a) { |
There was a problem hiding this comment.
I have a warning on ligne 56 about double to int conversion for the value a. A double is given in, and then it's passed to functions that require it to be double. Maybe it should be double a?
There was a problem hiding this comment.
I want to reproduce the issue, what compiler are you using, and compilation flags?
There was a problem hiding this comment.
Sure, I was using GCC with "g++ -std=c++17 -Wall -Wextra -Wshadow -Wnon-virtual-dtor -Wold-style-cast -Wcast-align -Wunused -Woverloaded-virtual -Wpedantic -Wconversion -Wsign-conversion -Wnull-dereference -Wdouble-promotion -Wformat=2"
There was a problem hiding this comment.
Yes, it should be double, I am extremely confused about how it passed the last test case with a=0.5. Let me spend some time with the test cases.
There was a problem hiding this comment.
Mitia, I fix warnings also found the bug in the test.
There was a problem hiding this comment.
I filed the similar issue on Julia code:
#843
There was a problem hiding this comment.
Looks good! Thanks for the submission and thanks @ShadowMitia for the review!
No description provided.