diff options
| -rw-r--r-- | doc/ledger3.texi | 2 | ||||
| -rw-r--r-- | src/amount.cc | 41 | ||||
| -rw-r--r-- | test/regress/2329.test | 83 | ||||
| -rw-r--r-- | test/unit/t_amount.cc | 12 | ||||
| -rw-r--r-- | test/unit/t_balance.cc | 10 |
5 files changed, 145 insertions, 3 deletions
diff --git a/doc/ledger3.texi b/doc/ledger3.texi index e9cadefb..02680caa 100644 --- a/doc/ledger3.texi +++ b/doc/ledger3.texi @@ -8660,7 +8660,7 @@ $ ledger -f expr.dat --format "%(account) %(roundto(amount, 1))\n" reg @end smallexample @smallexample @c output:B4DFB9F Assets:Cash ¤ -123,40 -Expenses:Office Supplies ¤ 123,50 +Expenses:Office Supplies ¤ 123,40 @end smallexample @end defun diff --git a/src/amount.cc b/src/amount.cc index 484c3099..604519df 100644 --- a/src/amount.cc +++ b/src/amount.cc @@ -684,8 +684,45 @@ void amount_t::in_place_roundto(int places) { if (! quantity) throw_(amount_error, _("Cannot round an uninitialized amount")); - double x=ceil(mpq_get_d(MP(quantity))*pow(10, places) - 0.49999999) / pow(10, places); - mpq_set_d(MP(quantity), x); + + // <https://github.com/ledger/ledger/issues/2362> + // _dup() call (see in_place_ceiling and in_place_floor) + // leads to 2 failures in the balance/testRound test, + // however in its absence this method affects copies of amount_t instances. + // Remove expected_failures from amount/testRound + // and update balance/testRound + // when uncommenting the following line. + // _dup(); + + mpz_t& scale(temp); + if (places) + mpz_ui_pow_ui(scale, 10, labs(places)); + + if (places > 0) { + mpz_mul(mpq_numref(MP(quantity)), mpq_numref(MP(quantity)), scale); + } else if (places < 0) { + mpz_mul(mpq_denref(MP(quantity)), mpq_denref(MP(quantity)), scale); + } + + auto whole(mpq_numref(tempq)); + auto reminder(mpq_denref(tempq)); + mpz_fdiv_qr(whole, reminder, mpq_numref(MP(quantity)), mpq_denref(MP(quantity))); + mpz_mul_2exp(reminder, reminder, 1); + const int rem_denom_cmp = mpz_cmp(reminder, mpq_denref(MP(quantity))); + if (rem_denom_cmp > 0 + || (rem_denom_cmp == 0 && mpz_odd_p(whole))) + mpz_add_ui(whole, whole, 1); + + if (places > 0) { + mpq_set_num(MP(quantity), whole); + mpq_set_den(MP(quantity), scale); + mpq_canonicalize(MP(quantity)); + } else if (places == 0) + mpq_set_z(MP(quantity), whole); + else { + mpq_set_ui(MP(quantity), 0, 1); + mpz_mul(mpq_numref(MP(quantity)), whole, scale); + } } void amount_t::in_place_unround() diff --git a/test/regress/2329.test b/test/regress/2329.test new file mode 100644 index 00000000..643ef0d1 --- /dev/null +++ b/test/regress/2329.test @@ -0,0 +1,83 @@ + +; roundto() result should not have large power 2 denominator. +; Pairs to test positive/negative value symmetry. +assert roundto(1.1, 1) == 1.1 +assert roundto(-1.1, 1) == -1.1 + +; positive places +assert roundto(1.13, 1) == 1.1 +assert roundto(1,17, 1) == 1.2 +assert roundto(1.10, 1) == 1.1 +assert roundto(-1.13, 1) == -1.1 +assert roundto(-1,17, 1) == -1.2 +assert roundto(-1.10, 1) == -1.1 + +; zero places +assert roundto(0.12, 0) == 0 +assert roundto(123.89, 0) == 124 +assert roundto(-0.12, 0) == 0 +assert roundto(-123.89, 0) == -124 + +; negative places +assert roundto(123.45, -1) == 120 +assert roundto(98765, -3) == 99000 +assert roundto(24500, -2) == 24500 +assert roundto(-123.45, -1) == -120 +assert roundto(-98765, -3) == -99000 +assert roundto(-24500, -2) == -24500 + +; round to even +assert roundto(0.5, 1) + roundto(1.5, 1) + roundto(2.5, 1) + roundto(3.5, 1) + roundto(4.5, 1) == roundto(0.5 + 1.5 + 2.5 + 3.5 + 4.5, 1) +assert roundto(-0.5, 1) + roundto(-1.5, 1) + roundto(-2.5, 1) + roundto(-3.5, 1) + roundto(-4.5, 1) == roundto(-0.5 - 1.5 - 2.5 - 3.5 - 4.5, 1) + +assert roundto(0.12345, 4) == 0.1234 +assert roundto(0.9875, 3) == 0.988 +assert roundto(-0.12345, 4) == -0.1234 +assert roundto(-0.9875, 3) == -0.988 +assert roundto(1234.5, 0) == 1234 +assert roundto(367.5, 0) == 368 +assert roundto(-1234.5, 0) == -1234 +assert roundto(-367.5, 0) == -368 +assert roundto(29500.00, -3) == 30000 +assert roundto(72250, -2) == 72200 +assert roundto(-29500.00, -3) == -30000 +assert roundto(-72250, -2) == -72200 + +; Assertion in transactions, no error: +comment + +While parsing posting: + Account:NOK 0 = -700.67 NOK + ^^^^^^^^^^^ +Error: Balance assertion off by -0.00 NOK (expected to see -700.67 NOK) +end comment + +2024-02-01 Regular precision + Account:NOK (-roundto(12.34 NOK * 56.78, 2)) + Expenses:NOK 700.67 NOK +2024-02-02 Assertion + Account:NOK 0 = -700.67 NOK + +; Transaction balance, no error: +comment + +> 2024-02-01 Excessive precision +> Account:USD (-roundto($12.34 * 56.78, 2)) +> Expenses:USD $700.67 +Unbalanced remainder is: + $0.0000000000000 +Amount to balance against: + $700.6700000000000 +Error: Transaction does not balance +end comment + +commodity $ + format $1000.0000000000000 +2024-02-01 Excessive precision + Account:USD (-roundto($12.34 * 56.78, 2)) + Expenses:USD $700.67 + +test bal --flat --no-total Account + -700.67 NOK Account:NOK + $-700.6700000000000 Account:USD +end test diff --git a/test/unit/t_amount.cc b/test/unit/t_amount.cc index b82de510..28bc56e9 100644 --- a/test/unit/t_amount.cc +++ b/test/unit/t_amount.cc @@ -1163,6 +1163,18 @@ BOOST_AUTO_TEST_CASE(testCommodityCeiling) BOOST_CHECK(x2.valid()); } +BOOST_AUTO_TEST_CASE(testRound, * boost::unit_test::expected_failures(1)) +{ + amount_t a1("$ 123.123"); + amount_t a2(a1); + a2.in_place_roundto(2); + // Fails due to missing _dup() call in amount_t::in_place_roundto(int). + // <https://github.com/ledger/ledger/issues/2362> + BOOST_CHECK_EQUAL(amount_t("$ 123.123"), a1); + // Should it be "$ 123.12"? + BOOST_CHECK_EQUAL(amount_t("$ 123.120"), a2); +} + #ifndef NOT_FOR_PYTHON #if 0 BOOST_AUTO_TEST_CASE(testReduction) diff --git a/test/unit/t_balance.cc b/test/unit/t_balance.cc index b503d02a..066f066e 100644 --- a/test/unit/t_balance.cc +++ b/test/unit/t_balance.cc @@ -451,6 +451,9 @@ BOOST_AUTO_TEST_CASE(testRound) b2 += a5; b2 += a6; + // <https://github.com/ledger/ledger/issues/2362> + // This block modifies b1 and b2, likely it is a bug, + // but otherwise some assertions fails. a1.in_place_roundto(2); a2.in_place_roundto(2); a3.in_place_roundto(2); @@ -465,11 +468,18 @@ BOOST_AUTO_TEST_CASE(testRound) b4 += a5; b4 += a6; + // After fix of #2362 rounded() and in_place_round() + // likely should be replaced with roundto(2) and + // in_place_roundto(2). + // It looks like rounded() and in_place_round() + // need some other tests, perhaps with unround(). BOOST_CHECK_EQUAL(b0.rounded(), b0); BOOST_CHECK_EQUAL(b2.rounded(), b4); + // Relies on b1 modified by amount_t::in_place_roundto(int). BOOST_CHECK_EQUAL(b1.rounded(), b4); b1.in_place_round(); + // Relies on b1 modified by amount_t::in_place_roundto(int). BOOST_CHECK_EQUAL(b1, b3); BOOST_CHECK(b0.valid()); |