Re: [lmi] _M_range_check fails when running "new" PDF

[Greg Chicares]

On 2018-02-14 00:38, Greg Chicares wrote:
> On 2018-02-13 23:07, Vadim Zeitlin wrote:
[...]
>>  The problem is due to "IrrCsv_GuaranteedZero" vector being empty in this
> 
> Oh. That thing.
[...]
>>  So my preferred solution would be to fill IrrCsv_GuaranteedZero (and other
>> similar vectors) with zeroes when LedgerInvariant::IrrCsvGuar0 (and others)
>> is empty. What do you think of this proposal?
> 
> I think it sounds good. Instead of zero, I'd like to fill it with negative
> one, i.e., -100%
[...]
> I imagine that the fix is something like:
> 
>     // Calculate these IRRs only for ledger types that actually use a
>     // basis with a zero percent separate-account rate. This is a
>     // matter not of efficiency but of validity: values for unused
>     // bases are not dependably initialized.
>     //
>     // This calculation should be distributed among the variant
>     // ledgers, so that it gets run for every basis actually used.
>     if
>         (0 == std::count
>             (LedgerValues.GetRunBases().begin()
>             ,LedgerValues.GetRunBases().end()
>             ,mce_run_gen_curr_sep_zero
>             // Proxy for mce_run_gen_guar_sep_zero too.
>             )
>         )


Spoiler: for the failing testcase, the condition above is false, so
the code in the following block isn't reached; that's why no such
change can fix the problem.

>         {
> +       IrrCsvGuar0 = std::vector(the_appropriate_length, -1);
> +       IrrDbGuar0  = likewise
> +       IrrCsvCurr0 = likewise
> +       IrrDbCurr0  = likewise
>         return;
>         }
> 
> but let me sleep on it--I'll give you an answer in the morning.


Well, that doesn't work. I did essentially that in the second part
of this patch--and then added the first part to guard against any
possible mistake in the second part, but it still didn't work:

---------8<--------8<--------8<--------8<--------8<--------8<--------8<-------
diff --git a/ledger_invariant.cpp b/ledger_invariant.cpp
index 2d40216df..4bd948919 100644
--- a/ledger_invariant.cpp
+++ b/ledger_invariant.cpp
@@ -1263,6 +1263,16 @@ void LedgerInvariant::CalculateIrrs(Ledger const& 
LedgerValues)
 {
     int max_length = LedgerValues.GetMaxLength();
 
+    // Make sure none of these is uninitialized.
+    IrrCsvGuar0    .resize(max_length, -1.0);
+    IrrDbGuar0     .resize(max_length, -1.0);
+    IrrCsvCurr0    .resize(max_length, -1.0);
+    IrrDbCurr0     .resize(max_length, -1.0);
+    IrrCsvGuarInput.resize(max_length, -1.0);
+    IrrDbGuarInput .resize(max_length, -1.0);
+    IrrCsvCurrInput.resize(max_length, -1.0);
+    IrrDbCurrInput .resize(max_length, -1.0);
+
     LedgerVariant const& Curr_ = LedgerValues.GetCurrFull();
     LedgerVariant const& Guar_ = LedgerValues.GetGuarFull();
     irr
@@ -1317,6 +1327,10 @@ void LedgerInvariant::CalculateIrrs(Ledger const& 
LedgerValues)
             )
         )
         {
+        IrrCsvGuar0.resize(max_length, -1.0);
+        IrrDbGuar0 .resize(max_length, -1.0);
+        IrrCsvCurr0.resize(max_length, -1.0);
+        IrrDbCurr0 .resize(max_length, -1.0);
         return;
         }
 
--------->8-------->8-------->8-------->8-------->8-------->8-------->8-------

I'd hesitate to make such a change anyway--especially the first
section in the patch above--because in 'ledger_text_formats.cpp'
we have this code:

    if(length != invar_.IrrCsvCurrInput.size())
        {
...
        if(...something else too...)
            {
            unclean.CalculateIrrs(ledger_);
            }
        else
            {
            unclean.IrrCsvCurrInput.resize(length, -1.0);
            [etc.]

which is conditional on an IRR variable having an unexpected size,
for some reason that must have seemed good at the time but isn't
apparent from `git show 6703fd36` or explained in 'ChangeLog'.
This command:
  grep 'IrrCsv.*size\|IrrDb.*size' *.?pp |less -S
suggests that no such conditional is used anywhere else. Although
it's written as:
    if(length != invar_.IrrCsvCurrInput.size())
perhaps the idea was really:
    if(invar_.IrrCsvCurrInput.empty())
because I can't think of any size it could have other than zero
or 'length'. In that case, the condition apparently means
    if(CalculateIrrs() hasn't been called yet)
but I can't see how it can already have been called. Even if it
had, calling it again couldn't make the IRR values incorrect.
I really think that this conditional was intended only as a
speed optimization, but never fulfilled such an intention:
"das ist nicht nur nicht richtig, es ist nicht einmal falsch!"

I have an idea to try next, but I'll try it before writing about it.

>>  Also, would you know of any other vectors used for table columns which
>> might not have the values for all (or any, as is the case here) years?
> 
> Not off the top of my head, but I'll take a look tomorrow.


In 'ledger_evaluator.cpp', following the call to CalculateIrrs(),
you'll find:

    std::vector<double> PolicyYear;
    std::vector<double> AttainedAge;

which apparently aren't members of any ledger class (and
which want to be initialized in some less tasteless way);

    std::vector<double> InitAnnLoanDueRate(max_duration);

which bizarrely takes InterestRates::RegLnDueRate(), discards
all elements except the first, and then replicates that to
the original length, and then...

  ledger_pdf_generator_wx.cpp:
  ,{ "InitAnnLoanDueRate"  , "Assumed\nLoan Interest"      ,     "99.99%" }

...presents it with a column title that seems appropriate only
for the original InterestRates::RegLnDueRate(); and

    // TODO ?? A really good design would give users the power to
    // define and store their own derived-column definitions. For now,
    // however, code changes are required, and this is as appropriate
    // a place as any to make them.
...
    std::vector<double> PremiumLoads(max_duration);
    std::vector<double> AdminCharges(max_duration);

which are dynamically generated.

Then, in 'ledger_invariant.hpp', you'll see these red-flag items:

    // Special-case vectors (not <double>, or different length than others).
    std::vector<mce_mode> EeMode;
    std::vector<mce_mode> ErMode;
    std::vector<mce_dbopt>DBOpt;

where the three above are just non-double vectors that you're
presumably formatting correctly already, but the rest:

    std::vector<double>      FundNumbers;
    std::vector<std::string> FundNames;
    std::vector<int>         FundAllocs; // Obsolete--spreadsheet only.
    std::vector<double>      FundAllocations;

    std::vector<double> InforceLives;

have their own special lengths. You probably don't need any of
the "Fund*" vectors, but that last one has one more element
than you might expect.