[lmi-commits] [lmi] master f2e2cd7 01/10: Refactor to do less work

[Greg Chicares]

branch: master
commit f2e2cd7b6be5d0a0a43dd5eafcabab095377e857
Author: Gregory W. Chicares <gchicares@sbcglobal.net>
Commit: Gregory W. Chicares <gchicares@sbcglobal.net>

    Refactor to do less work
    
    Reinitialized only selected members of class LedgerInvariant, rather
    than the whole thing.
    
    Set specamt OAOO. The "if(!SolvingForGuarPremium)" conditional is no
    longer needed on redundant calls, which no longer exist.
    
    These changes improve speed by about five percent:
    
    $wine ./lmi_cli_shared.exe --accept --data_path=/opt/lmi/data --selftest
    [before]
      naic, no solve      : 5.489e-02 s mean;     54549 us least of  19 runs
      naic, specamt solve : 9.845e-02 s mean;     97713 us least of  11 runs
      naic, ee prem solve : 9.000e-02 s mean;     89512 us least of  12 runs
      finra, no solve     : 2.112e-02 s mean;     20990 us least of  48 runs
      finra, specamt solve: 6.092e-02 s mean;     60615 us least of  17 runs
      finra, ee prem solve: 5.657e-02 s mean;     56294 us least of  18 runs
    [after]
      naic, no solve      : 5.139e-02 s mean;     51085 us least of  20 runs
      naic, specamt solve : 9.241e-02 s mean;     91846 us least of  11 runs
      naic, ee prem solve : 8.445e-02 s mean;     84028 us least of  12 runs
      finra, no solve     : 2.084e-02 s mean;     20696 us least of  48 runs
      finra, specamt solve: 5.834e-02 s mean;     57865 us least of  18 runs
      finra, ee prem solve: 5.413e-02 s mean;     53815 us least of  19 runs
---
 ihs_acctval.cpp           | 32 ++++++++-----------------
 ledger_invariant.hpp      |  1 +
 ledger_invariant_init.cpp | 60 +++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 71 insertions(+), 22 deletions(-)

diff --git a/ihs_acctval.cpp b/ihs_acctval.cpp
index ce1a871..c8bf08f 100644
--- a/ihs_acctval.cpp
+++ b/ihs_acctval.cpp
@@ -100,6 +100,12 @@ AccountValue::AccountValue(Input const& input)
     ,OldDBOpt              {mce_option1}
     ,YearsDBOpt            {mce_option1}
 {
+    SetInitialValues();
+    LMI_ASSERT(InforceYear < methuselah);
+    PerformSpecAmtStrategy();
+    PerformSupplAmtStrategy();
+    InvariantValues().Init(this);
+
     // Explicitly initialize antediluvian members. It's generally
     // better to do this in the initializer-list, but here they can
     // all be kept together.
@@ -329,31 +335,13 @@ void AccountValue::InitializeLife(mcenum_run_basis 
a_Basis)
     RunBasis_ = a_Basis;
     set_cloven_bases_from_run_basis(RunBasis_, GenBasis_, SepBasis_);
 
+    // Many values set by SetInitialValues() are never subsequently
+    // changed. SOMEDAY !! Factor those out into a function that can
+    // be called OAOO.
     SetInitialValues();
 
-    // TODO ?? This is a nasty workaround. It seems that some or all strategies
-    // should be performed at a much higher level, say in Run*(). Without
-    // the conditional here, guar prem is wrong for 7-pay spec amt strategy.
-    // It's wasteful to call PerformSpecAmtStrategy() once per basis,
-    // but the result is always the same (because the premium is).
-    if(!SolvingForGuarPremium)
-        {
-        PerformSpecAmtStrategy();
-        PerformSupplAmtStrategy();
-        }
-
-    // Call Init() here. The solve routines can change parameters in
-    // class BasicValues or objects it contains, parameters which
-    // determine ledger values that are used by the solve routines.
-    // It might be more appropriate to treat such parameters instead
-    // as local state of class AccountValue itself, or of a contained
-    // class smaller than the ledger hierarchy--which we need anyway
-    // for 7702 and 7702A. Or perhaps the solve functions should
-    // manipulate the state of just those elements of the ledgers
-    // that it needs to, to avoid the massive overhead of
-    // unconditionally reinitializing all elements.
     VariantValues().Init(*this, GenBasis_, SepBasis_);
-    InvariantValues().Init(this);
+    InvariantValues().ReInit(this);
 
     // Default initial values assume that the policy never lapses or
     // becomes a MEC, so that the lapse and MEC durations are the last
diff --git a/ledger_invariant.hpp b/ledger_invariant.hpp
index e8f576d..a3c9faf 100644
--- a/ledger_invariant.hpp
+++ b/ledger_invariant.hpp
@@ -50,6 +50,7 @@ class LMI_SO LedgerInvariant final
     ~LedgerInvariant() override;
 
     void Init(BasicValues const*);
+    void ReInit(BasicValues const*);
 
     LedgerInvariant& PlusEq(LedgerInvariant const& a_Addend);
 
diff --git a/ledger_invariant_init.cpp b/ledger_invariant_init.cpp
index d5d2421..2900000 100644
--- a/ledger_invariant_init.cpp
+++ b/ledger_invariant_init.cpp
@@ -683,3 +683,63 @@ void LedgerInvariant::Init(BasicValues const* b)
     // encompassed by 'FullyInitialized'.
     FullyInitialized = true;
 }
+
+/// TODO ?? Temporary kludge.
+///
+/// Objects of this class should be used only to store final values
+/// that result from monthiversary processing, and to use those values
+/// to generate reports. Therefore, they should need to be initialized
+/// only once. However, they've been (ab)used to store intermediate
+/// values during monthiversary processing, and thus, defectively,
+/// some data members need to be reinitialized when calculations are
+/// to be performed again on a different basis.
+///
+/// Complete reinitialization is costly, so this function does it only
+/// for data members that actually require it. Each such member
+/// represents a defect. When the last such defect is resolved, this
+/// function will be empty and can itself be removed, along with the
+/// following commentary transplanted from the call site:
+///
+/// // Call Init() here. The solve routines can change parameters in
+/// // class BasicValues or objects it contains, parameters which
+/// // determine ledger values that are used by the solve routines.
+/// // It might be more appropriate to treat such parameters instead
+/// // as local state of class AccountValue itself, or of a contained
+/// // class smaller than the ledger hierarchy--which we need anyway
+/// // for 7702 and 7702A. Or perhaps the solve functions should
+/// // manipulate the state of just those elements of the ledgers
+/// // that it needs to, to avoid the massive overhead of
+/// // unconditionally reinitializing all elements.
+
+void LedgerInvariant::ReInit(BasicValues const* b)
+{
+    HasSupplSpecAmt            = false;
+    if(b->yare_input_.TermRider)
+        {
+        TermSpecAmt            .assign(Length, b->yare_input_.TermRiderAmount);
+        }
+    else if(b->database().query<bool>(DB_TermIsNotRider))
+        {
+        TermSpecAmt            = b->DeathBfts_->supplamt();
+        if(!each_equal(TermSpecAmt, 0.0))
+            {
+            HasSupplSpecAmt    = true;
+            }
+        }
+    else
+        {
+        TermSpecAmt            .assign(Length, 0.0);
+        }
+    SpecAmt                    = b->DeathBfts_->specamt();
+
+    InitBaseSpecAmt            = b->DeathBfts_->specamt()[0];
+    InitTermSpecAmt            = TermSpecAmt[0];
+
+    IsMec                      = false;
+    MecMonth                   = 11;
+    MecYear                    = Length;
+
+    ModalMinimumPremium        .assign(Length, 0.0);
+    EeModalMinimumPremium      .assign(Length, 0.0);
+    ErModalMinimumPremium      .assign(Length, 0.0);
+}