Re: [lmi] rate_table_tool: merge a whole directory

[Greg Chicares]

On 2016-11-30 15:08, Vadim Zeitlin wrote:
> On Wed, 30 Nov 2016 08:15:56 +0000 Greg Chicares <address@hidden> wrote:
> 
> GC> On 2016-11-30 00:27, Greg Chicares wrote:
> GC> [...]
> GC> >   expected: 70000000 * 0.04551 = 3185700.00 exactly
> GC> >   observed: 3185700.00 with old_bin [exactly as expected]
> GC> >   observed: 3185699.99000000022352 with new_bin [anomaly]

Correction: the 'old_bin' and 'new_bin' labels were reversed, so,
at least for this particular discrepancy, the result is actually
improved by using the new table.

> GC> > lmi rounds the calculated seven-pay premium down to cents, so a
> GC> > difference of 1 ulp in the binary number read from the table could
> GC> > account for this anomaly. I imagine a 1 ulp difference could arise
> GC> > in table_impl::parse_single_value() [showing only the most relevant
> GC> > lines]:
> GC> > 
> GC> >     auto const res_int_part = strict_parse_number(current);
> GC> >     auto const res_frac_part = strict_parse_number(res_int_part.end + 
> 1);
> GC> > 
> GC> >     double value = res_frac_part.num;
> GC> >     value /= std::pow(10, *num_decimals_);
> GC> >     value += res_int_part.num;
> GC> >     return value;

0.0455099999999999949019 Saved7PPRate <-- read from OLD binary table
0.0455100000000000018407 Saved7PPRate <-- read from NEW binary table

  0.0455100000000000000000
- 0.0455099999999999949019
= 0.0000000000000000050981
50981 / 455100000000000000000 = 1.1e-16 ~= double-precision epsilon

18407 / 455100000000000000000 = 4e-17 < epsilon

New hypothesis: The original binary tables may be slightly inaccurate.
Certainly some were added by means of the old gcc-compiled program
that I built from hacked SOA sources. Very likely others were added
by means of a 1990s binary distributed by the SOA, which would at best
have been compiled with some old version of "turbo C++" (which supplied
its own C RTL), or at worst with "turbo pascal".

I have plenty more anomalies to investigate--this one just seemed
especially convenient--so I might have more hypotheses later.

> GC> > My hypothesis then, comparing floating-point numbers for strict
> GC> > equality, is that:
> GC> >   assert(0.04551 != 4551 / 10000)
> 
>  This is definitely true, but, AFAIR, this is exactly why I do the number
> parsing in this roundabout way. Again, the details are a bit hazy by now
> (and I definitely should have recorded them in a comment here), but I think
> this was necessary to ensure that the text form remains unchanged after
> "txt -> bin -> txt" round trip test when using 16 decimal places (this
> problem is not going to appear with only 5 or 6 of them). I could try to
> make a test for this, but OTOH it's clear that this is not a good approach
> neither if it results in errors such as above :-(

I can't demonstrate that it does cause any anomaly, at least not with this
particular test case.

>  What I still don't understand is why the unit test test_to_from_text()
> fail to catch this bug, after all this is exactly what it is testing for.

Yet it would fail to detect an anomaly consistent with my latest hypothesis,
such an anomaly quite properly being outside its scope.

> GC> > I suspect it's necessary to parse text as above in order to write
> GC> > helpful diagnostics, but I think the return value should be the
> GC> > applicable character substring converted by some other means, e.g.,
> GC> >   value_cast<double>(std::string const&)

That gives exactly the same result.

>  Yes, this would work if you don't mind value_cast<> overhead.

Doesn't any modern compiler optimize all the overhead away, leaving only
a call to std::strtod()?