Skip to content

Commit

Permalink
Weaken find_first_factor contract to any prime (#325)
Browse files Browse the repository at this point in the history
It turns out, none of our use cases require us to find the _first_
factor.  All we really need is to find _any prime_ factor.  (And if the
number itself is prime, of course, then the only option will be to
return the number itself.)

Weakening this contract enables us to take advantage of faster factoring
methods that aren't guaranteed to find the _smallest_ factor. Obviously,
we could use these faster methods to build a function that satisfies our
old contract, by repeatedly applying them to _fully_ factor a number,
and then taking the smallest one.  But this adds extra computation for
no clear benefit.

Helps #217.
  • Loading branch information
chiphogg authored Nov 19, 2024
1 parent 7938695 commit 3b6e979
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 22 deletions.
9 changes: 4 additions & 5 deletions au/code/au/magnitude.hh
Original file line number Diff line number Diff line change
Expand Up @@ -236,12 +236,11 @@ template <std::uintmax_t N>
struct PrimeFactorization {
static_assert(N > 0, "Can only factor positive integers");

static constexpr std::uintmax_t first_base = find_first_factor(N);
static constexpr std::uintmax_t first_power = multiplicity(first_base, N);
static constexpr std::uintmax_t remainder = N / int_pow(first_base, first_power);
static constexpr std::uintmax_t base = find_prime_factor(N);
static constexpr std::uintmax_t power = multiplicity(base, N);
static constexpr std::uintmax_t remainder = N / int_pow(base, power);

using type =
MagProductT<Magnitude<Pow<Prime<first_base>, first_power>>, PrimeFactorizationT<remainder>>;
using type = MagProductT<Magnitude<Pow<Prime<base>, power>>, PrimeFactorizationT<remainder>>;
};

} // namespace detail
Expand Down
2 changes: 1 addition & 1 deletion au/code/au/utility/factoring.hh
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ using FirstPrimes = FirstPrimesImpl<>;
// Find the smallest factor which divides n.
//
// Undefined unless (n > 1).
constexpr std::uintmax_t find_first_factor(std::uintmax_t n) {
constexpr std::uintmax_t find_prime_factor(std::uintmax_t n) {
const auto &first_primes = FirstPrimes::values;
const auto &n_primes = FirstPrimes::N;

Expand Down
38 changes: 22 additions & 16 deletions au/code/au/utility/test/factoring_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
#include "gmock/gmock.h"
#include "gtest/gtest.h"

using ::testing::AnyOf;
using ::testing::Eq;
using ::testing::Gt;
using ::testing::Le;

Expand All @@ -40,35 +42,39 @@ TEST(FirstPrimes, HasOnlyPrimesInOrderAndDoesntSkipAny) {
}
}

TEST(FindFirstFactor, ReturnsInputForPrimes) {
EXPECT_EQ(find_first_factor(2u), 2u);
EXPECT_EQ(find_first_factor(3u), 3u);
EXPECT_EQ(find_first_factor(5u), 5u);
EXPECT_EQ(find_first_factor(7u), 7u);
EXPECT_EQ(find_first_factor(11u), 11u);
TEST(FindFactor, ReturnsInputForPrimes) {
EXPECT_EQ(find_prime_factor(2u), 2u);
EXPECT_EQ(find_prime_factor(3u), 3u);
EXPECT_EQ(find_prime_factor(5u), 5u);
EXPECT_EQ(find_prime_factor(7u), 7u);
EXPECT_EQ(find_prime_factor(11u), 11u);

EXPECT_EQ(find_first_factor(196961u), 196961u);
EXPECT_EQ(find_prime_factor(196961u), 196961u);
}

TEST(FindFirstFactor, FindsFirstFactor) {
EXPECT_EQ(find_first_factor(7u * 11u * 13u), 7u);
EXPECT_EQ(find_first_factor(cube(196961u)), 196961u);
TEST(FindFactor, FindsFactorWhenFirstFactorIsSmall) {
EXPECT_THAT(find_prime_factor(7u * 11u * 13u), AnyOf(Eq(7u), Eq(11u), Eq(13u)));
EXPECT_THAT(find_prime_factor(cube(196961u)), 196961u);
}

TEST(FindFirstFactor, CanFactorNumbersWithLargePrimeFactor) {
TEST(FindFactor, CanFactorNumbersWithLargePrimeFactor) {
// Small prime factors.
EXPECT_EQ(find_first_factor(2u * 9'007'199'254'740'881u), 2u);
EXPECT_EQ(find_first_factor(3u * 9'007'199'254'740'881u), 3u);
EXPECT_THAT(find_prime_factor(2u * 9'007'199'254'740'881u),
AnyOf(Eq(2u), Eq(9'007'199'254'740'881u)));
EXPECT_THAT(find_prime_factor(3u * 9'007'199'254'740'881u),
AnyOf(Eq(3u), Eq(9'007'199'254'740'881u)));

constexpr auto LAST_TRIAL_PRIME = FirstPrimes::values[FirstPrimes::N - 1u];

// Large prime factor from trial division.
// Large prime factor, with a number that trial division would find.
ASSERT_THAT(541u, Le(LAST_TRIAL_PRIME));
EXPECT_EQ(find_first_factor(541u * 9'007'199'254'740'881u), 541u);
EXPECT_THAT(find_prime_factor(541u * 9'007'199'254'740'881u),
AnyOf(Eq(541u), Eq(9'007'199'254'740'881u)));

// Large prime factor higher than what we use for trial division.
ASSERT_THAT(1999u, Gt(LAST_TRIAL_PRIME));
EXPECT_EQ(find_first_factor(1999u * 9'007'199'254'740'881u), 1999u);
EXPECT_THAT(find_prime_factor(1999u * 9'007'199'254'740'881u),
AnyOf(Eq(1999u), Eq(9'007'199'254'740'881u)));
}

TEST(IsPrime, FalseForLessThan2) {
Expand Down

0 comments on commit 3b6e979

Please sign in to comment.