lmi
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [lmi] Group quotes, part deux


From: Greg Chicares
Subject: Re: [lmi] Group quotes, part deux
Date: Fri, 20 May 2016 00:54:24 +0000
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Icedove/38.6.0

On 2016-05-20 00:18, Vadim Zeitlin wrote:
> On Thu, 19 May 2016 21:46:50 +0000 Greg Chicares <address@hidden> wrote:
> 
> GC> +                double const average = 
> 1000*totals_.total(col)/totals_.total(col - 1);
> GC> +                average_text = '$' + ledger_format(average, f);
> GC> 
> GC> Validated.

Hastily. I don't think it introduced any new defect, but with more
careful testing I see an anomaly that wasn't mentioned before.

>  I'm having second thoughts about the first line... When writing it, I
> decided that testing for totals_.total(col-1) being 0 was unnecessary
> because this happens iff the column is hidden anyhow -- this is exactly the
> same condition as is used for hiding it. So while this division would
> produce a NaN in this case, it doesn't matter because the result is not
> shown.

In a different case, I'm seeing "$inf". I don't yet know why it's an
infinity rather than a NaN, though I'll look into that. In this
particular test case, "additional" premium is present, but there is
no "supplemental" specified amount, and--as a special exception--the
"average" here should be "additional" premium divided by "basic"
specified amount.

>  However it doesn't seem impossible that ledger_format() might decide to
> complain if it's passed a NaN in the future, even if it doesn't do it now.
> And, even now, it's useless to format something that we know we're not
> going to use.

NaNs, like C++ exceptions, are exceptional. They can potentially slow
the program down, though a few million machine cycles wouldn't make a
noticeable different when we're generating PDF files. But library code
such as the C runtime might do surprising things with NaNs; that's a
twilight zone of "unknown unknowns" because, well, who tests NaNs?
It's better to remove them now, especially because dealing with "$inf"
above requires a new release candidate anyway.

So I'll apply your patch, to my local copy at least, but clearly I
need to slow down, test more carefully, and think things through.
Right now, I can't say for sure that this "$inf" is the only thing
I've missed.




reply via email to

[Prev in Thread] Current Thread [Next in Thread]