From 1123023c6484be61ce6833936aa50719aa3b56a4 Mon Sep 17 00:00:00 2001 From: "James C. Owens" Date: Mon, 5 Feb 2024 16:58:00 -0500 Subject: [PATCH 1/2] Enhance fraction addition by using gcd of denominators This makes fraction addition more resistant to overflows. --- src/test/util_tests.cpp | 18 +++++++++++++ src/util.h | 58 ++++++++++++++++++++++++++++++++++++++--- 2 files changed, 72 insertions(+), 4 deletions(-) diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp index 7c29dcf892..fe5410094e 100755 --- a/src/test/util_tests.cpp +++ b/src/test/util_tests.cpp @@ -1385,6 +1385,24 @@ BOOST_AUTO_TEST_CASE(util_Fraction_addition_with_internal_simplification) BOOST_CHECK_EQUAL(sum.IsSimplified(), true); } +BOOST_AUTO_TEST_CASE(util_Fraction_addition_with_internal_gcd_simplification) +{ + Fraction lhs(1, 6); + Fraction rhs(2, 15); + + // gcd(6, 15) = 3, so this really is + // + // 1 * (15/3) + 2 * (6/3) 1 * 5 + 2 * 2 3 + // ---------------------- = ------------- = -- + // 3 * (6/3) * (15/3) 3 * 2 * 5 10 + + Fraction sum = lhs + rhs; + + BOOST_CHECK_EQUAL(sum.GetNumerator(), 3); + BOOST_CHECK_EQUAL(sum.GetDenominator(), 10); + BOOST_CHECK_EQUAL(sum.IsSimplified(), true); +} + BOOST_AUTO_TEST_CASE(util_Fraction_subtraction) { Fraction lhs(2, 3); diff --git a/src/util.h b/src/util.h index 8a300122f0..c591792c18 100644 --- a/src/util.h +++ b/src/util.h @@ -356,10 +356,60 @@ class Fraction { return Fraction(overflow_add(slhs.GetNumerator(), srhs.GetNumerator()), slhs.GetDenominator(), true); } - // Otherwise do the full pattern of getting a common denominator and adding, then simplify... - return Fraction(overflow_add(overflow_mult(slhs.GetNumerator(), srhs.GetDenominator()), - overflow_mult(slhs.GetDenominator(), srhs.GetNumerator())), - overflow_mult(slhs.GetDenominator(), srhs.GetDenominator()), + // Now the more complex case. In general, fraction addition follows this pattern: + // + // a c a * (d/g) + c * (b/g) + // - + - , g = gcd(b, d) => --------------------- where {(b/g), (d/g)} will be elements of the counting numbers. + // b d g * (b/g) * (d/g) + // + // (b/g) and (d/g) are divisible with no remainders precisely because of the definition of gcd. + // + // We have already covered the trivial common denominator case above before bothering to compute the gcd of the + // denominator. + int64_t denom_gcd = std::gcd(slhs.GetDenominator(), srhs.GetDenominator()); + + // We have two special cases. One is where g = b (i.e. d is actually a multiple of b). In this case, + // the expression simplifies to + // + // a * (d/b) + c + // ------------- + // d + if (denom_gcd == slhs.GetDenominator()) { + return Fraction(overflow_add(overflow_mult(slhs.GetNumerator(), srhs.GetDenominator() / slhs.GetDenominator()), + srhs.GetNumerator()), + srhs.GetDenominator(), + true); + } + + // The other is where g = d (i.e. b is actually a multiple of d). In this case, + // the expression simplifies to + // + // a + c * (b/d) + // ------------- + // b + if (denom_gcd == srhs.GetDenominator()) { + return Fraction(overflow_add(overflow_mult(srhs.GetNumerator(), slhs.GetDenominator() / srhs.GetDenominator()), + slhs.GetNumerator()), + slhs.GetDenominator(), + true); + } + + // Otherwise do the full pattern of getting a common denominator (pulling out the gcd of the denominators), + // and adding, then simplify... + // + // This approach is more complex than + // + // a * d + c * b + // ------------- + // b * d + // + // but has the advantage of being more resistant to overflows, especially when the two denominators are related by a large + // gcd. In particular in Gridcoin's application with Allocations, the largest denominator of the allocations is 10000, so + // every allocation denominator in reduced form must be divisible evenly into 10000. This means the majority of fraction + // additions will be the two simpler cases above. + return Fraction(overflow_add(overflow_mult(slhs.GetNumerator(), srhs.GetDenominator() / denom_gcd), + overflow_mult(srhs.GetNumerator(), slhs.GetDenominator() / denom_gcd)), + overflow_mult(denom_gcd, overflow_mult(slhs.GetDenominator() / denom_gcd, srhs.GetDenominator() / denom_gcd)), true); } From 4deec19f02b123afc907ccb866ff4a6e14b5627f Mon Sep 17 00:00:00 2001 From: "James C. Owens" Date: Tue, 6 Feb 2024 22:27:43 -0500 Subject: [PATCH 2/2] Enhance fraction multiplication overflow resistance This uses the "cross" gcds to do cancellation and increase resistance of overflows for things like (1000 / 999) * (999 / 1000). --- src/test/util_tests.cpp | 23 +++++++++++++++++++++++ src/util.h | 32 ++++++++++++++++++++++++++++++-- 2 files changed, 53 insertions(+), 2 deletions(-) diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp index fe5410094e..8f8bd204f2 100755 --- a/src/test/util_tests.cpp +++ b/src/test/util_tests.cpp @@ -1439,6 +1439,29 @@ BOOST_AUTO_TEST_CASE(util_Fraction_multiplication_with_internal_simplification) BOOST_CHECK_EQUAL(product.IsSimplified(), true); } +BOOST_AUTO_TEST_CASE(util_Fraction_multiplication_with_cross_simplification_overflow_resistance) +{ + + Fraction lhs(std::numeric_limits::max() - 3, std::numeric_limits::max() - 1, false); + Fraction rhs((std::numeric_limits::max() - 1) / (int64_t) 2, (std::numeric_limits::max() - 3) / (int64_t) 2); + + Fraction product; + + // This should NOT overflow + bool overflow = false; + try { + product = lhs * rhs; + } catch (std::overflow_error& e) { + overflow = true; + } + + BOOST_CHECK_EQUAL(overflow, false); + + if (!overflow) { + BOOST_CHECK(product == Fraction(1)); + } +} + BOOST_AUTO_TEST_CASE(util_Fraction_division_with_internal_simplification) { Fraction lhs(-2, 3); diff --git a/src/util.h b/src/util.h index c591792c18..66288783ad 100644 --- a/src/util.h +++ b/src/util.h @@ -435,8 +435,36 @@ class Fraction { Fraction slhs(*this, true); Fraction srhs(rhs, true); - return Fraction(overflow_mult(slhs.GetNumerator(), srhs.GetNumerator()), - overflow_mult(slhs.GetDenominator(), srhs.GetDenominator()), + // Gcd's can be used in multiplication for better overflow resistance as well. + // + // Consider + // a c + // - * -, where a/b and c/d are already simplified (i.e. gcd(a, b) = gcd(c, d) = 1. + // b d + // + // We can have g = gcd(a, d) and h = gcd(c, b), which is with the numerators reversed, since multiplication is + // commutative. This means we have + // + // (c / h) (a / g) + // ------- * ------- . + // (b / h) (d / g) + // + // If we form Fraction(c, b, true) and Fraction(a, d, true), the simplification will determine and divide the numerator and + // denominator by h and g respectively. + // + // A specific example is instructive. + // + // 1998 1000 999 1000 1000 999 1 1 + // ---- * ---- = ---- * ---- = ---- * --- = - * - + // 2000 999 1000 999 1000 999 1 1 + // + // This is a formal form of what grade school teachers called factor cancellation. :). + + Fraction sxlhs(srhs.GetNumerator(), slhs.GetDenominator(), true); + Fraction sxrhs(slhs.GetNumerator(), srhs.GetDenominator(), true); + + return Fraction(overflow_mult(sxlhs.GetNumerator(), sxrhs.GetNumerator()), + overflow_mult(sxlhs.GetDenominator(), sxrhs.GetDenominator()), true); }