-
Notifications
You must be signed in to change notification settings - Fork 132
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Is probable prime #32
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@KingAkeem Sorry for the late review. If you're still interested in working on this then please make the following changes. Otherwise, just let me know and I'll either complete this myself or assign it to someone else.
num = 3; | ||
REQUIRE(num.is_probable_prime(25) == 1); | ||
num = 5; | ||
REQUIRE(num.is_probable_prime(25) == 1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about larger cases? Can you check that it returns true
for the following prime numbers:
- 4361161843811
- 91584398720031
- 54362229927468991056799869539182953179604007
- 1141606828476848812192797260322842016771684147
- 237082482904158189833801188468727382999221896206963750677
- 4978732140987321986509824957843275042983659820346238764217
and false
for the following composite numbers:
- 576308207413
- 648273642634986
- 328964398726983264982
- 4079327665427094820942557
- 879654387682647825646328764
- 98732243986019286982046325298743
- 589658224987973265974369876397863298796328749
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is because a
is not declared. I think you forgot to replace all instances of a
to rand_num
.
test/functions/math.cpp
Outdated
BigInt num = 11; | ||
REQUIRE(pow(num, 9) == 2357947691); | ||
BigInt num = 11; | ||
REQUIRE(pow(num, 9) == 2357947691); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please indent your code with 4 spaces per level.
include/functions/utility.hpp
Outdated
s++; | ||
long long spower = pow(2, s); | ||
if (spower > sentinel_value) { | ||
return std::make_tuple(0,0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please indent your code with 4 spaces per level.
include/functions/math.hpp
Outdated
bool BigInt::is_probable_prime(size_t certainty) { | ||
// The BigInt object that this method refers to will be referred to as n in comments | ||
#define COMPOSITE false | ||
#define PRIME true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These #defines
are not required.
include/functions/math.hpp
Outdated
BigInt a; | ||
while (certainty-- > 0) { | ||
// 1 <= a < n | ||
a = random_number(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rename a
to rand_num
, and random_number
to get_random_number
.
Some cases fail:
|
Going to take a lot at this again soon |
#5 I've added the is_probable_prime method using Rabin Miller primality testing. I started with Little Fermat's Theorem and built from that. I also had to add a utility function in order to calculate much needed values. I've added a test to cover the base cases but I've ran a number of test using extremely large primes and a variety random primes using random generators. The test is fast and very accurate based on the certainty passed to it.