[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[lmi-commits] [lmi] valyuta/000 e9efb62: See 'README.branch'
|
From: |
Greg Chicares |
|
Subject: |
[lmi-commits] [lmi] valyuta/000 e9efb62: See 'README.branch' |
|
Date: |
Tue, 11 Aug 2020 18:08:44 -0400 (EDT) |
branch: valyuta/000
commit e9efb62472281faac7965276adc1eb148c20fc4a
Author: Gregory W. Chicares <gchicares@sbcglobal.net>
Commit: Gregory W. Chicares <gchicares@sbcglobal.net>
See 'README.branch'
---
README.branch | 75 +++++++++++++++++++++++++++++++++++++++++++++
account_value.hpp | 75 +++++++++++++++++++++++----------------------
accountvalue.cpp | 2 +-
currency.hpp | 91 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
currency_test.cpp | 18 +++++++++++
ihs_avmly.cpp | 56 +++++++++++++++++++---------------
ledger_base.hpp | 2 +-
7 files changed, 255 insertions(+), 64 deletions(-)
diff --git a/README.branch b/README.branch
new file mode 100644
index 0000000..bdb4da1
--- /dev/null
+++ b/README.branch
@@ -0,0 +1,75 @@
+// README for this branch.
+//
+// Copyright (C) 2020 Gregory W. Chicares.
+//
+// This program is free software; you can redistribute it and/or modify
+// it under the terms of the GNU General Public License version 2 as
+// published by the Free Software Foundation.
+//
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with this program; if not, write to the Free Software Foundation,
+// Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA
+//
+// https://savannah.nongnu.org/projects/lmi
+// email: <gchicares@sbcglobal.net>
+// snail: Chicares, 186 Belle Woods Drive, Glastonbury CT 06033, USA
+
+This branch explores replacing 'double' with a currency class. Several
+dozen data members of class AccountValue were changed to the new type.
+Those members were chosen as representative, for experimentation;
+many others should also be of currency type.
+
+The currency class is a coarse exploratory attempt.
+
+Various code changes were made:
+ - some explicit type conversions;
+ - std::max<double>, with template parameter explicitly specified, to
+ make it work with one double and one currency argument;
+ - an intermediate variable (max_loan) of type double, to hold some
+ calculations that are necessarily floating point before posting the
+ final result to the (currency) MaxLoan variable;
+ - similarly, restructuring of the NAAR calculation.
+
+The regression test does not pass. Some variables have never been
+rounded, though they are certainly integral numbers of cents in the
+problem domain:
+ AVSepAcct especially; also AVGenAcct sometimes
+ DacTaxLoad, PremTaxLoad
+ SpecAmtLoad
+ RiderCharges
+ GrossPmt (for testdeck 'sample')
+and those are defects that should be fixed in the trunk.
+
+Future directions:
+
+The ledger class too often contains values that are plainly intended
+to represent an integral number of cents, but whose values look like
+ 17696.499999999996362
+where a compiler might choose either closest representable neighbor,
+the choice depending on the phase of the moon; such false positives
+make regression testing too laborious. One idea is to "de-round" these
+values, but that would be to let the tail wag the dog.
+
+It seems far better to use a fixed-point class for values that are
+supposed to be an integral number of cents. The underlying type can
+be either integer or floating point; that's an implementation detail
+that needn't be resolved now. The crucial idea is that the internal
+representation should be cents; users of course expect to see dollars
+and cents, but that's a View concern--the Model wants to use cents.
+With a fixed-point class, a value such as 17696.499999999996362 that
+had actually been rounded would be stored as 1769650 exactly, and
+printed as 17696.50 (i.e., with an explicit decimal point) for
+regression testing (and of course for end-user report generation).
+
+This coarse implementation runs slowly:
+
+/opt/lmi/bin[0]$wine ./lmi_cli_shared.exe --accept --data_path=/opt/lmi/data
--selftest
+Test solve speed: 1.805e-01 s mean; 180125 us least of 6 runs
+
+versus about 89000 us for production, i.e., half as fast. Numeric
+conversions probably account for most of that overhead.
diff --git a/account_value.hpp b/account_value.hpp
index a0a11b7..4ae858a 100644
--- a/account_value.hpp
+++ b/account_value.hpp
@@ -25,6 +25,7 @@
#include "config.hpp"
#include "basic_values.hpp"
+#include "currency.hpp"
#include "oecumenic_enumerations.hpp"
#include "so_attributes.hpp"
@@ -386,17 +387,17 @@ class LMI_SO AccountValue final
// Reproposal input.
int InforceYear;
int InforceMonth;
- double InforceAVGenAcct;
- double InforceAVSepAcct;
- double InforceAVRegLn;
- double InforceAVPrfLn;
- double InforceRegLnBal;
- double InforcePrfLnBal;
- double InforceCumNoLapsePrem;
- double InforceBasis;
- double InforceCumPmts;
- double InforceTaxBasis;
- double InforceLoanBalance;
+ currency InforceAVGenAcct;
+ currency InforceAVSepAcct;
+ currency InforceAVRegLn;
+ currency InforceAVPrfLn;
+ currency InforceRegLnBal;
+ currency InforcePrfLnBal;
+ currency InforceCumNoLapsePrem;
+ currency InforceBasis;
+ currency InforceCumPmts;
+ currency InforceTaxBasis;
+ currency InforceLoanBalance;
// Intermediate values.
int Year;
@@ -405,24 +406,24 @@ class LMI_SO AccountValue final
bool daily_interest_accounting;
int days_in_policy_month;
int days_in_policy_year;
- double AVGenAcct;
- double AVSepAcct;
- double SepAcctValueAfterDeduction;
+ currency AVGenAcct;
+ currency AVSepAcct;
+ currency SepAcctValueAfterDeduction;
double GenAcctPaymentAllocation;
double SepAcctPaymentAllocation;
- double NAAR;
- double CoiCharge;
- double RiderCharges;
- double NetCoiCharge;
- double SpecAmtLoadBase;
+ currency NAAR;
+ currency CoiCharge;
+ currency RiderCharges;
+ currency NetCoiCharge;
+ currency SpecAmtLoadBase;
double DacTaxRsv;
- double AVUnloaned; // Antediluvian.
+ currency AVUnloaned; // Antediluvian.
double NetMaxNecessaryPremium;
double GrossMaxNecessaryPremium;
- double NecessaryPremium;
- double UnnecessaryPremium;
+ currency NecessaryPremium;
+ currency UnnecessaryPremium;
// 7702A CVAT deemed cash value.
double Dcv;
@@ -446,21 +447,21 @@ class LMI_SO AccountValue final
mcenum_mode pmt_mode; // Antediluvian.
int ModeIndex; // Antediluvian.
- double GenAcctIntCred;
- double SepAcctIntCred;
- double RegLnIntCred;
- double PrfLnIntCred;
- double AVRegLn;
- double AVPrfLn;
- double RegLnBal;
- double PrfLnBal;
- double MaxLoan;
- double UnusedTargetPrem;
- double AnnualTargetPrem;
- double MaxWD;
- double GrossWD;
- double NetWD;
- double CumWD;
+ currency GenAcctIntCred;
+ currency SepAcctIntCred;
+ currency RegLnIntCred;
+ currency PrfLnIntCred;
+ currency AVRegLn;
+ currency AVPrfLn;
+ currency RegLnBal;
+ currency PrfLnBal;
+ currency MaxLoan;
+ currency UnusedTargetPrem;
+ currency AnnualTargetPrem;
+ currency MaxWD;
+ currency GrossWD;
+ currency NetWD;
+ currency CumWD;
double wd; // Antediluvian.
double mlyguarv; // Antediluvian.
diff --git a/accountvalue.cpp b/accountvalue.cpp
index d4afefb..b20ac71 100644
--- a/accountvalue.cpp
+++ b/accountvalue.cpp
@@ -979,7 +979,7 @@ void AccountValue::TxTakeLoan()
double IntAdj = std::pow((1.0 + YearsRegLnIntDueRate), 12 - Month);
IntAdj = (IntAdj - 1.0) / IntAdj;
MaxLoan *= 1.0 - IntAdj;
- MaxLoan = std::max(0.0, MaxLoan);
+ MaxLoan = std::max<double>(0.0, MaxLoan);
MaxLoan = round_loan()(MaxLoan);
// IHS !! Preferred loan calculations would go here: implemented in lmi.
diff --git a/currency.hpp b/currency.hpp
index a3a3fc2..9dea3e8 100644
--- a/currency.hpp
+++ b/currency.hpp
@@ -24,6 +24,95 @@
#include "config.hpp"
-using currency = double;
+#include "bourn_cast.hpp"
+#include "round_to.hpp"
+
+#include <cstdint> // int64_t
+#include <iostream> // ostream
+
+class currency
+{
+ using data_type = std::int64_t;
+ friend std::ostream& operator<<(std::ostream&, currency const&);
+ friend class currency_test;
+
+ public:
+ currency() = default;
+ currency(currency const&) = default;
+ ~currency() = default;
+
+ explicit currency(double d) {m_ = from_double(d);}
+
+ currency& operator=(currency const&) = default;
+ currency& operator=(double d) {m_ = from_double(d); return *this;}
+
+ operator double() const {return to_double();}
+
+ // Is this better, with 'const&'?
+// bool operator==(currency const& z) const {return z.m_ == m_;}
+
+ bool operator< (currency z) const {return m_ < z.m_;}
+ bool operator<=(currency z) const {return m_ <= z.m_;}
+ bool operator==(currency z) const {return m_ == z.m_;}
+ bool operator!=(currency z) const {return m_ != z.m_;}
+ bool operator> (currency z) const {return m_ > z.m_;}
+ bool operator>=(currency z) const {return m_ >= z.m_;}
+
+ bool operator< (double d) const {return to_double() < d;}
+ bool operator<=(double d) const {return to_double() <= d;}
+ bool operator==(double d) const {return to_double() == d;}
+ bool operator!=(double d) const {return to_double() != d;}
+ bool operator> (double d) const {return to_double() > d;}
+ bool operator>=(double d) const {return to_double() >= d;}
+
+ // Is this the ideal signature for this operator?
+// currency operator-() const {return currency(-m_);}
+// currency& operator-() {m_ = -m_; return *this;}
+
+ currency& operator+=(currency z) {m_ += z.m_; return *this;}
+ currency& operator-=(currency z) {m_ -= z.m_; return *this;}
+ // NOPE!
+// currency& operator*=(currency z) {m_ *= z.m_; return *this;}
+
+ currency& operator+=(double z) {m_ += from_double(z); return *this;}
+ currency& operator-=(double z) {m_ -= from_double(z); return *this;}
+ // NOPE!
+// currency& operator*=(double z) {m_ *= from_double(z); return *this;}
+ // Check result range (and avoid multiplying by 100/100):
+// currency& operator*=(double z) {m_ = bourn_cast<data_type>(100.0 *
to_double() * z); return *this;}
+ // Far too permissive:
+// currency& operator*=(double z) {m_ = static_cast<data_type>(100.0 *
to_double() * z); return *this;}
+ double operator*=(double z) {return to_double() * z;}
+
+ private:
+ // Want something just slightly more permissive:
+// data_type from_double(double d) const {return bourn_cast<data_type>(100.0
* d);}
+ // Far too permissive:
+// data_type from_double(double d) const {return static_cast<data_type>(100.0
* d);}
+ // ...and a bit insidious:
+// data_type from_double(double d) const {return
static_cast<data_type>(100.000000000001 * d);}
+ // ...less bad:
+ data_type from_double(double d) const {return round(100.0 * d);}
+ double to_double() const {return bourn_cast<double>(m_) / 100.0;}
+
+ data_type round(double d) const
+ {
+ static round_to<double> const r(0, r_to_nearest);
+ return static_cast<data_type>(r(d));
+ }
+
+ data_type m_ = {0};
+};
+
+inline currency operator+(currency lhs, double rhs) {return lhs +=
currency(rhs);}
+inline currency operator-(currency lhs, double rhs) {return lhs -=
currency(rhs);}
+//inline currency operator*(currency lhs, double rhs) {return lhs *=
currency(rhs);}
+inline double operator*(currency lhs, double rhs) {return lhs *=
currency(rhs);}
+
+inline std::ostream& operator<<(std::ostream& os, currency const& c)
+{
+// return os << c.m_ << ' ' << c.to_double();
+ return os << c.to_double();
+}
#endif // currency_hpp
diff --git a/currency_test.cpp b/currency_test.cpp
index 9920b13..e7af97f 100644
--- a/currency_test.cpp
+++ b/currency_test.cpp
@@ -41,6 +41,24 @@ void currency_test::test()
void currency_test::test_something()
{
+ currency a0;
+ std::cout << a0 << std::endl;
+ BOOST_TEST(0.00 == a0.operator double());
+ BOOST_TEST( 0 == a0.m_);
+
+// Figure out what to do about this:
+// currency a1(3.14);
+
+ currency a1(3.25);
+ BOOST_TEST(3.25 == a1.operator double());
+ BOOST_TEST( 325 == a1.m_);
+ BOOST_TEST( 0 == a1 * a0);
+ a1 += a1;
+ BOOST_TEST(6.50 == a1.operator double());
+ BOOST_TEST( 650 == a1.m_);
+ a1 *= a0;
+ BOOST_TEST(0.00 == a1.operator double());
+ BOOST_TEST( 0 == a1.m_);
}
int test_main(int, char*[])
diff --git a/ihs_avmly.cpp b/ihs_avmly.cpp
index b36e4cd..b4433e6 100644
--- a/ihs_avmly.cpp
+++ b/ihs_avmly.cpp
@@ -404,7 +404,7 @@ void AccountValue::process_distribution(double decrement)
void AccountValue::DecrementAVProportionally(double decrement)
{
- if(materially_equal(decrement, AVGenAcct + AVSepAcct))
+ if(materially_equal(decrement, (AVGenAcct + AVSepAcct).operator double()))
{
AVGenAcct = 0.0;
AVSepAcct = 0.0;
@@ -413,8 +413,8 @@ void AccountValue::DecrementAVProportionally(double
decrement)
double general_account_proportion = 0.0;
double separate_account_proportion = 0.0;
- double general_account_nonnegative_assets = std::max(0.0, AVGenAcct);
- double separate_account_nonnegative_assets = std::max(0.0, AVSepAcct);
+ double general_account_nonnegative_assets = std::max<double>(0.0,
AVGenAcct);
+ double separate_account_nonnegative_assets = std::max<double>(0.0,
AVSepAcct);
if
( 0.0 == general_account_nonnegative_assets
&& 0.0 == separate_account_nonnegative_assets
@@ -462,23 +462,26 @@ void AccountValue::DecrementAVProgressively
,oenum_increment_account_preference preferred_account
)
{
- if(materially_equal(decrement, AVGenAcct + AVSepAcct))
+ if(materially_equal(decrement, (AVGenAcct + AVSepAcct).operator double()))
{
AVGenAcct = 0.0;
AVSepAcct = 0.0;
return;
}
+// Can this be made to work?
+// currency z = decrement;
+ currency z(decrement);
switch(preferred_account)
{
case oe_prefer_general_account:
{
- AVGenAcct -= progressively_reduce(AVGenAcct, AVSepAcct, decrement);
+ AVGenAcct -= progressively_reduce(AVGenAcct, AVSepAcct, z);
}
break;
case oe_prefer_separate_account:
{
- AVGenAcct -= progressively_reduce(AVSepAcct, AVGenAcct, decrement);
+ AVGenAcct -= progressively_reduce(AVSepAcct, AVGenAcct, z);
}
break;
}
@@ -1471,11 +1474,12 @@ void AccountValue::TxLoanRepay()
// TODO ?? This idiom seems too cute. And it can return -0.0 .
// Maximum repayment is total debt.
- ActualLoan = -std::min(-RequestedLoan, RegLnBal + PrfLnBal);
+ ActualLoan = -std::min<double>(-RequestedLoan, RegLnBal + PrfLnBal);
process_distribution(ActualLoan);
- LMI_ASSERT(0.0 == progressively_reduce(AVRegLn , AVPrfLn , -ActualLoan));
- LMI_ASSERT(0.0 == progressively_reduce(RegLnBal, PrfLnBal, -ActualLoan));
+ currency z(-ActualLoan);
+ LMI_ASSERT(0.0 == progressively_reduce(AVRegLn , AVPrfLn , z));
+ LMI_ASSERT(0.0 == progressively_reduce(RegLnBal, PrfLnBal, z));
// This seems wrong. If we're changing something that's invariant among
// bases, why do we change it for each basis?
@@ -1508,7 +1512,7 @@ void AccountValue::TxSetBOMAV()
)
: yare_input_.InforceSpecAmtLoadBase
;
- SpecAmtLoadBase = std::min(SpecAmtLoadBase, SpecAmtLoadLimit);
+ SpecAmtLoadBase = std::min<double>(SpecAmtLoadBase, SpecAmtLoadLimit);
}
// These assignments must happen every month.
@@ -1708,6 +1712,7 @@ void AccountValue::EndTermRider(bool convert)
void AccountValue::TxSetCoiCharge()
{
// Net amount at risk is the death benefit discounted one month
+ // e.g. divided by (1 + 0.0024663)
// at the guaranteed interest rate, minus account value iff
// nonnegative (a negative account value mustn't increase NAAR);
// but never less than zero. The account value could be negative
@@ -1717,11 +1722,13 @@ void AccountValue::TxSetCoiCharge()
// than zero because the corridor factor can be as low as unity,
// but it's constrained to be nonnegative to prevent increasing
// the account value by deducting a negative mortality charge.
- NAAR = material_difference
- (DBReflectingCorr * DBDiscountRate[Year]
- ,std::max(0.0, TotalAccountValue())
+ NAAR = round_naar()
+ (material_difference
+ (DBReflectingCorr * DBDiscountRate[Year]
+ ,std::max(0.0, TotalAccountValue())
+ )
);
- NAAR = std::max(0.0, round_naar()(NAAR));
+ NAAR = std::max(currency(0), NAAR);
// TODO ?? This doesn't work. We need to reconsider the basic transactions.
// double naar_forceout = std::max(0.0, NAAR - MaxNAAR);
@@ -1759,10 +1766,11 @@ void AccountValue::TxSetCoiCharge()
)
);
double retention_rate = round_coi_rate()(coi_rate * CoiRetentionRate);
- retention_charge = round_coi_charge()(NAAR * retention_rate);
+ retention_charge = round_coi_charge()(NAAR.operator double() *
retention_rate);
}
- CoiCharge = round_coi_charge()(NAAR * ActualCoiRate);
+ // Yields zero without operator double():
+ CoiCharge = round_coi_charge()(NAAR.operator double() * ActualCoiRate);
NetCoiCharge = CoiCharge - retention_charge;
YearsTotalCoiCharge += CoiCharge;
@@ -2267,7 +2275,7 @@ void AccountValue::SetMaxWD()
{
MaxWD = 0.0;
}
- MaxWD = std::max(0.0, MaxWD);
+ MaxWD = std::max<double>(0.0, MaxWD);
}
/// Take a withdrawal.
@@ -2312,7 +2320,7 @@ void AccountValue::TxTakeWD()
if(Solving || mce_run_gen_curr_sep_full == RunBasis_)
{
- NetWD = std::min(RequestedWD, MaxWD);
+ NetWD = std::min<double>(RequestedWD, MaxWD);
OverridingWD[Year] = NetWD;
}
else
@@ -2438,7 +2446,7 @@ void AccountValue::TxTakeWD()
return;
}
- GrossWD = NetWD + std::min(WDFee, NetWD * WDFeeRate);
+ GrossWD = NetWD + std::min<double>(WDFee, NetWD * WDFeeRate);
// Free partial surrenders: for instance, the first 20% of account
// value might be withdrawn each policy year free of surrender
@@ -2456,7 +2464,7 @@ void AccountValue::TxTakeWD()
LMI_ASSERT(AVPrfLn == PrfLnBal);
LMI_ASSERT(av == AVGenAcct + AVSepAcct);
double free_wd = FreeWDProportion[Year] * av;
- non_free_wd = std::max(0.0, GrossWD - free_wd);
+ non_free_wd = std::max<double>(0.0, GrossWD - free_wd);
}
double partial_surrchg = non_free_wd * surrchg_proportion;
GrossWD += partial_surrchg;
@@ -2559,7 +2567,7 @@ void AccountValue::TxTakeWD()
// Calculate maximum permissible total loan (not increment).
void AccountValue::SetMaxLoan()
{
- MaxLoan =
+ double max_loan =
(AVGenAcct + AVSepAcct) * MaxLoanAVMult
+ (AVRegLn + AVPrfLn)
- anticipated_deduction(MaxLoanDed_)
@@ -2598,7 +2606,7 @@ void AccountValue::SetMaxLoan()
// the end of the policy year--but does not guarantee that, e.g.
// because the specified amount may change between anniversaries,
// even on illustrations.
- MaxLoan -=
+ max_loan -=
RegLnBal * reg_loan_factor
+ PrfLnBal * prf_loan_factor
;
@@ -2608,9 +2616,9 @@ void AccountValue::SetMaxLoan()
// plausible but unasserted assumption that that factor is more
// liberal than the preferred-loan factor?
//
- MaxLoan *= 1.0 - (reg_loan_factor) / (1.0 + reg_loan_factor);
+ max_loan *= 1.0 - (reg_loan_factor) / (1.0 + reg_loan_factor);
- MaxLoan = round_loan()(MaxLoan);
+ MaxLoan = round_loan()(max_loan);
// I do not think we want a MaxLoan < current level of indebtedness.
MaxLoan = std::max((AVRegLn + AVPrfLn), MaxLoan);
diff --git a/ledger_base.hpp b/ledger_base.hpp
index c22370f..e471313 100644
--- a/ledger_base.hpp
+++ b/ledger_base.hpp
@@ -243,7 +243,7 @@ template<typename T> void SpewVector
std::ostream_iterator<T> osi(os, "\n");
os << name << '\n';
- os << std::setprecision(DECIMAL_DIG);
+ os << std::setprecision(15);
std::copy(elements.begin(), elements.end(), osi);
}