feat: add count distinct primes from binary string algorithm #2970
Conversation
Add count_distinct_primes_from_binary_string.cpp
rudrakshtank
requested review from
Panquesito7 and
realstealthninja
as code owners
| #include <unordered_set> | ||
| #include <algorithm> | ||
|
|
||
| const int MAX = 1e6; |
There was a problem hiding this comment.
max cannot be a negative number, hence replace int with uint32_t
There was a problem hiding this comment.
you still havent done this
| const int MAX = 1e6; | |
| const uint32_t MAX = 1e6; | |
| #include <algorithm> | ||
|
|
||
| const int MAX = 1e6; | ||
| std::vector<bool> is_prime; |
There was a problem hiding this comment.
| std::vector<bool> is_prime; | |
| static std::vector<bool> is_prime; | |
There was a problem hiding this comment.
Since you want to pre compute primes why not make this a constexpr? and assign in it directly? instead of a call from main. Also afaik vectors cant be used as constexpr in c++17 hence it might be preferred if you use std::array instead.
if any of this sounds confusing let me know I can explain or direct you to resources to learn about this feature of c++
| std::unordered_set<int> seen; | ||
|
|
||
| for (int i = 2; i < limit; i++) { | ||
| if (__builtin_popcount(i) <= k && is_prime[i]) { |
There was a problem hiding this comment.
__builtin_popcount is not part of the STL library it only exists in GCC.
thus this should be replaced with std::popcount()
| if (__builtin_popcount(i) <= k && is_prime[i]) { | |
| if (std::popcount(i) <= k && is_prime[i]) { | |
| #include <iostream> | ||
| #include <vector> | ||
| #include <unordered_set> | ||
| #include <algorithm> |
There was a problem hiding this comment.
| #include <iostream> | |
| #include <vector> | |
| #include <unordered_set> | |
| #include <algorithm> | |
| #include <bit> /// for std::popcount | |
| #include <iostream> | |
| #include <vector> | |
| #include <unordered_set> | |
| #include <algorithm> |
There was a problem hiding this comment.
please add documentation for all the above headers as shown in the example
realstealthninja
changed the title
|
Thank you for the review, I'll get back to you with suggested changes |
|
I have done with the suggested changes, please review the changes |
| void tests(){ | ||
| precomputePrimes(); | ||
| std::string s; | ||
| std::cin >> s; | ||
| std::cout << countPrimeBinaryStrings(s) << std::endl; | ||
| } | ||
|
|
There was a problem hiding this comment.
tests should be outside the namespace and static, A test should be comparing a computed value to a known value using an assert statement
| #include <unordered_set> | ||
| #include <algorithm> | ||
|
|
||
| const int MAX = 1e6; |
There was a problem hiding this comment.
you still havent done this
| const int MAX = 1e6; | |
| const uint32_t MAX = 1e6; | |
| #include <iostream> | ||
| #include <vector> | ||
| #include <unordered_set> | ||
| #include <algorithm> |
There was a problem hiding this comment.
please add documentation for all the above headers as shown in the example
| static std::vector<bool> is_prime; | ||
|
|
||
| /** | ||
| * @namespace bit_manipulation | ||
| * @brief Bit manipulation algorithms | ||
| * @brief Bit manipulation algorithms and prime preprocessing |
There was a problem hiding this comment.
| * @brief Bit manipulation algorithms and prime preprocessing | |
| * @brief Bit manipulation algorithms |
| /** | ||
| * @brief Main function to test the algorithm. | ||
| * @brief Main function to run tests. |
There was a problem hiding this comment.
| * @brief Main function to run tests. | |
| * @brief Main function | |
| * @returns 0 on successful exit |
|
|
||
| // Example test cases | ||
| assert(countPrimeBinaryStrings("1") == 0); // Only "1" -> not prime | ||
| assert(countPrimeBinaryStrings("11") > 0); // Should form primes like 3 |
There was a problem hiding this comment.
you need to compare against the actual value that should be forming, comparing this way against 0 is too general. here from my estimation
01 not prime
10 prime
11 prime
so the answer should be three
you should be comparing against three
| // Example test cases | ||
| assert(countPrimeBinaryStrings("1") == 0); // Only "1" -> not prime | ||
| assert(countPrimeBinaryStrings("11") > 0); // Should form primes like 3 | ||
| assert(countPrimeBinaryStrings("101") > 0); // Can form primes like 5 |
There was a problem hiding this comment.
Same thing here, saying any binary string which is non zero and non 1 can form primes above 0 is not a great test.
here i estimate
001 not prime
100 not prime
101 prime
so the answer should be one, please compare against the actual number
@realstealthninja Could you please review this PR?
Description of Change
Added
count_distinct_primes_from_binary_string.cppto thebit_manipulation/directory.This program counts the number of distinct prime numbers that can be formed from a binary string using:
It uses an efficient implementation of the Sieve of Eratosthenes for prime checking and bitmasking to explore all possible combinations.
Includes:
main()with an example test caseContributors guide
Checklist
main()Notes:
Efficient bitmask + sieve-based solution to find all valid prime numbers from binary strings using allowed transformations.