[lmi-commits] [lmi] master f9276c0 8/8: Rework minimum bounds for solves

[Greg Chicares]

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

    Rework minimum bounds for solves
    
    This reverts immediately preceding commit 34aca342c515e (in large part),
    as foreshadowed by its commit message.
    
    It also reverts commit 2a7f97248034f of 20171114T1739Z, which requires
    some discussion because that older commit fixed a defect, yet reverting
    it now does not reintroduce the defect.
    
    In personal email sent 2017-11-14T17:38Z, I said:
    
    | - First, test this testdeck's middle cell with lmi as of this
    | email's timestamp or earlier: it fails. You can look into the
    | failure if you want, e.g. by adding 'idiosyncrasyT'--you'll
    | see that (as always) it starts with a priori upper and lower
    | bounds, and moves them toward each other to bracket an answer,
    | but moving the lower bound up from zero to (e.g.) 1, 10, 100
    | has no effect (because of the $50000 minimum)...so instead the
    | algorithm reduces the upper bound, and declares victory when
    | it has in effect solved for lapse at the given duration. It
    | thinks this is right because, as 'idiosyncrasyT' will show,
    | the bracketing values of the input give outputs with opposite
    | sign, so is appears that a root has been bracketed to within
    | the rounding tolerance for specamt--even though we can see
    | that this isn't what we want.
    
    An 'idiosyncrasyT' trace now shows:
    
    iteration 0 iterand 0 value 100.560000000000002274
    iteration 1 iterand 1000000000 value -10141058.679999999702
    iteration 2 iterand 9916 value 80.6400000000000005684
    iteration 3 iterand 50058 value -0.630000000000000004441
    iteration 4 iterand 49747 value 0.56999999999999995115
    iteration 5 iterand 49895 value 0.25
    iteration 6 iterand 49978 value 0.0800000000000000016653
    iteration 7 iterand 50009 value -0.0599999999999999977796
    iteration 8 iterand 49996 value 0.0400000000000000008327
    iteration 9 iterand 50001 value 0.0200000000000000004163
    iteration 10 iterand 50005 value -0.0400000000000000008327
    iteration 11 iterand 50002 value 0.0100000000000000002082
    iteration 12 iterand 50003 value 0.0100000000000000002082
    iteration 13 iterand 50004 value -0.0400000000000000008327
    
    where 'value' is negative (positive) if 'iterand' is above (below) the
    verified solve result, 50003.
    
    It's been almost half a decade since commit 2a7f97248034f, and the
    likeliest explanation is that some latent defect in the monthiversary
    loop has been removed: either that, or some such defect remains, but
    is sidestepped now for reasons unknown. But the principle explained in
    the comments here committed remains valid: if an objective function
    changes sign, a root has been bracketed, and if that root is deemed
    not to be a "true" root, then the fault lies in the objective function
    (which might, e.g., be fixed by imposing a penalty for lapsing, so that
    a "bad" iteration cannot return a zero value). But decimal_root()'s job
    is only to find a zero; if it finds a zero that we don't like, we should
    simply change the objective function.
    
    With this change, the test case above takes fourteen iterations, as
    opposed to five without it. That's okay: both produce the same (valid)
    answer, and it would be a mistake to tune global parameters in order to
    optimize a single test case.
---
 ihs_avsolve.cpp | 31 +++++++++++++++++--------------
 1 file changed, 17 insertions(+), 14 deletions(-)

diff --git a/ihs_avsolve.cpp b/ihs_avsolve.cpp
index a2f8f25..13fd8e0 100644
--- a/ihs_avsolve.cpp
+++ b/ihs_avsolve.cpp
@@ -361,11 +361,25 @@ currency AccountValue::Solve
     LMI_ASSERT(0 < SolveTargetDuration_);
     LMI_ASSERT(    SolveTargetDuration_ <= BasicValues::GetLength());
 
-    // Default bounds (may be overridden in some cases).
+    // Default bounds.
+    //
+    // These are 'const' to discourage replacing them when narrower
+    // bounds can be determined in context (as is often the case for
+    // mce_solve_specamt and mce_solve_wd). The objective function,
+    // AccountValue::SolveTest(), must already embody such knowledge,
+    // and returns a value that is carefully crafted to facilitate
+    // solves--in particular, for iterations that violate certain
+    // product rules (e.g., see "ullage" above). Imposing a product
+    // minimum here can vitiate such optimizations; it is unlikely to
+    // make solves faster (finding a zero of x^2-1e8 in (0,1e9] is
+    // not materially harder than in [500,1e9], e.g.); it is certain
+    // to entail non-negligible coding and maintenance costs; and it
+    // introduces new opportunities for mistkaes.
+    //
     // Solve results are constrained to be nonnegative.
-    double lower_bound = 0.0;
+    double const lower_bound = 0.0;
     // No amount solved for can plausibly reach one billion dollars.
-    double upper_bound = 999999999.99;
+    double const upper_bound = 999999999.99;
 
     root_bias bias =
         mce_solve_for_tax_basis == SolveTarget_
@@ -386,16 +400,6 @@ currency AccountValue::Solve
             {
             solve_set_fn = &AccountValue::SolveSetSpecAmt;
             decimals     = round_specamt().decimals();
-            // Generally, base and term are independent, and it is
-            // the base specamt that's being solved for here, so set
-            // the minimum as though there were no term.
-            lower_bound = dblize
-                (minimum_specified_amount
-                    (  0 == SolveBeginYear_
-                    && yare_input_.EffectiveDate == yare_input_.InforceAsOfDate
-                    ,false
-                    )
-                );
             }
             break;
         case mce_solve_ee_prem:
@@ -431,7 +435,6 @@ currency AccountValue::Solve
                     ,round_loan      ().decimals()
                     );
                 }
-            lower_bound = dblize(MinWD);
             }
             break;
         }