Better manage truncation in GncRational::round_to_numeric.

Mike Alexander brought this up with a test case that failed to round down
sufficiently; he found that reducing the rounding denominator by 2 sufficed
to make his test case pass.

In fact the sizing of the replacement denominator by shifting the larger of
the numerator or denominator by an arbitrary 62 bits was not correct most
of the time, so instead we begin with a shift of the full lower leg worth,
try to do the conversion, and if the conversion is still “big” shift the
larger value one more and try the operation again, repeating until the
result will fit in a GncNumeric.
This commit is contained in:
John Ralls 2017-04-07 12:38:55 -07:00
parent a467d0d397
commit fd6234f58f
5 changed files with 94 additions and 31 deletions

View File

@ -149,6 +149,8 @@ GncRational::prepare_conversion (GncInt128 new_denom) const
auto red_conv = conversion.reduce();
GncInt128 old_num(m_num);
auto new_num = old_num * red_conv.num();
if (new_num.isOverflow())
throw std::overflow_error("Conversion overflow");
auto rem = new_num % red_conv.denom();
new_num /= red_conv.denom();
return {new_num, red_conv.denom(), rem};
@ -181,6 +183,7 @@ GncRational::reduce() const
GncRational
GncRational::round_to_numeric() const
{
unsigned int ll_bits = GncInt128::legbits;
if (m_num.isZero())
return GncRational(); //Default constructor makes 0/1
if (!(m_num.isBig() || m_den.isBig()))
@ -195,25 +198,53 @@ GncRational::round_to_numeric() const
<< "GncNumeric. Its integer value is too large.\n";
throw std::overflow_error(msg.str());
}
GncRational new_v(*this);
new_v = new_v.convert<RoundType::half_down>(m_den / (m_num.abs() >> 62));
GncRational new_v;
while (new_v.num().isZero())
{
try
{
new_v = convert<RoundType::half_down>(m_den / (m_num.abs() >> ll_bits));
if (new_v.is_big())
{
--ll_bits;
new_v = GncRational();
}
}
catch(const std::overflow_error& err)
{
--ll_bits;
}
}
return new_v;
}
auto quot(m_den / m_num);
if (quot.isBig())
return GncRational(); //Smaller than can be represented as a GncNumeric
auto divisor = m_den >> 62;
if (m_num.isBig())
GncRational new_v;
while (new_v.num().isZero())
{
GncInt128 oldnum(m_num), num, rem;
oldnum.div(divisor, num, rem);
auto den = m_den / divisor;
num += rem * 2 >= den ? 1 : 0;
GncRational new_rational(num, den);
return new_rational;
auto divisor = m_den >> ll_bits;
if (m_num.isBig())
{
GncInt128 oldnum(m_num), num, rem;
oldnum.div(divisor, num, rem);
auto den = m_den / divisor;
num += rem * 2 >= den ? 1 : 0;
if (num.isBig() || den.isBig())
{
--ll_bits;
continue;
}
GncRational new_rational(num, den);
return new_rational;
}
new_v = convert<RoundType::half_down>(m_den / divisor);
if (new_v.is_big())
{
--ll_bits;
new_v = GncRational();
}
}
GncRational new_v(*this);
new_v = new_v.convert<RoundType::half_down>(m_den / divisor);
return new_v;
}

View File

@ -150,9 +150,9 @@ public:
}
/** Numerator accessor */
GncInt128 num() { return m_num; }
GncInt128 num() const noexcept { return m_num; }
/** Denominator accessor */
GncInt128 denom() { return m_den; }
GncInt128 denom() const noexcept { return m_den; }
/** @defgroup gnc_rational_mutators
* @{
* Standard mutating arithmetic operators.
@ -275,5 +275,11 @@ inline GncRational operator/(GncInt128 a, GncRational b)
{
return GncRational(a, 1) / b;
}
inline std::ostream& operator<<(std::ostream& stream, const GncRational& val) noexcept
{
stream << val.num() << "/" << val.denom();
return stream;
}
/** @} */
#endif //__GNC_RATIONAL_HPP__

View File

@ -581,6 +581,7 @@ TEST(qofint128_functions, pow)
EXPECT_EQ (GncInt128(1), little.pow(0));
EXPECT_EQ (GncInt128(0), GncInt128(0).pow(123));
auto really_big = big * big;
EXPECT_EQ (big * big, big.pow(2));
EXPECT_EQ (GncInt128(UINT64_C(66326033898754),
UINT64_C(10251549987585143605)), little.pow(7));
@ -590,3 +591,17 @@ TEST(qofint128_functions, pow)
auto over = minus.pow(9);
EXPECT_TRUE(over.isOverflow());
}
TEST(qofint128_functions, shift)
{
GncInt128 a (UINT64_C(0xabcdabcd), UINT64_C(0xfe89fe89fe89fe89), 0);
EXPECT_EQ(GncInt128(UINT64_C(0xabcdabcdfe89), UINT64_C(0xfe89fe89fe890000), 0), a << 16);
EXPECT_EQ(GncInt128(UINT64_C(0xabcd), UINT64_C(0xabcdfe89fe89fe89), 0), a >> 16);
EXPECT_EQ(GncInt128(UINT64_C(0x1e89fe89fe89fe89), UINT64_C(0), 0), a << 64);
EXPECT_EQ(GncInt128(UINT64_C(0), UINT64_C(0xabcdabcd), 0), a >> 64);
GncInt128 b (UINT64_C(0xabcdabcd), UINT64_C(0xfe89fe89fe89fe89), 1);
EXPECT_EQ(GncInt128(UINT64_C(0xabcdabcdfe89), UINT64_C(0xfe89fe89fe890000), 1), b << 16);
EXPECT_EQ(GncInt128(UINT64_C(0xabcd), UINT64_C(0xabcdfe89fe89fe89), 1), b >> 16);
EXPECT_EQ(GncInt128(UINT64_C(0x1e89fe89fe89fe89), UINT64_C(0), 1), b << 64);
EXPECT_EQ(GncInt128(UINT64_C(0), UINT64_C(0xabcdabcd), 1), b >> 64);
}

View File

@ -332,17 +332,22 @@ TEST(gncnumeric_operators, test_multiplication)
GncNumeric a(123456789987654321, 1000000000);
GncNumeric b(65432198765432198, 100000000);
GncNumeric c = a * b;
EXPECT_EQ (4604488056206217807, c.num());
EXPECT_EQ (57, c.denom());
EXPECT_EQ (9208976112412435614, c.num());
EXPECT_EQ (114, c.denom());
a *= b;
EXPECT_EQ (4604488056206217807, a.num());
EXPECT_EQ (57, a.denom());
EXPECT_EQ (9208976112412435614, a.num());
EXPECT_EQ (114, a.denom());
GncNumeric d(215815575996, 269275978715);
GncNumeric e(1002837599929, 1);
GncNumeric f, g;
EXPECT_NO_THROW(f = d * e);
EXPECT_NO_THROW(g = f.convert<RoundType::half_up>(1));
GncNumeric h(133499999999, 136499999999);
GncNumeric i(60000000003, 100000000);
GncNumeric j;
EXPECT_NO_THROW(j = gnc_numeric_mul(h, i, 100000000, GNC_HOW_RND_ROUND));
EXPECT_EQ(58681318684, j.num());
}

View File

@ -149,25 +149,31 @@ rounding_predicate(GncInt128 expected, GncInt128 result)
TEST(gncrational_functions, test_round_to_numeric)
{
std::default_random_engine dre;
std::uniform_int_distribution<int64_t> di{INT64_C(0x100000000000),
INT64_C(0x7fffffffffffff)};
std::uniform_int_distribution<int64_t> di{INT64_C(0x1000000000000),
INT64_C(0x7ffffffffffffff)};
static const int reps{100};
for (auto i = 0; i < reps; ++i)
{
GncRational a(di(dre), di(dre));
GncRational b(di(dre), 100);
auto c = a * b;
auto expected = c;
expected = expected.convert<RoundType::bankers>(100);
auto rounded = c.round_to_numeric();
rounded = rounded.convert<RoundType::bankers>(100);
if (rounded.is_big())
{
--i;
continue;
try {
auto c = a * b;
auto expected = c;
expected = expected.convert<RoundType::bankers>(100);
auto rounded = c.round_to_numeric();
rounded = rounded.convert<RoundType::bankers>(100);
if (rounded.is_big())
{
--i;
continue;
}
EXPECT_PRED2(rounding_predicate, expected.num(), rounded.num());
EXPECT_TRUE(rounded.valid());
}
catch (const std::overflow_error& err)
{
std::cerr << "Overflow error from " << a << " * " << b << ".\n";
}
EXPECT_PRED2(rounding_predicate, expected.num(), rounded.num());
EXPECT_TRUE(rounded.valid());
}
}