Dean Rasheed <dean.a.rasheed@gmail.com> writes:
> On 12 November 2015 at 21:01, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> I started to look at this patch, and was immediately bemused by the
>> comment in estimate_ln_weight:
> That's nonsense. The comment is perfectly correct. It's not saying the
> logarithm is negative, it's saying that the *weight* of the logarithm
> is negative.
Ah, you're right --- I'd gotten confused about the distinction between
ln(x) and ln(ln(x)). Nevermind ...
Next question: in the additional range-reduction step you added to ln_var,
why stop there, ie, what's the rationale for this magic number:
if (Abs((x.weight + 1) * DEC_DIGITS) > 10)
Seems like we arguably should do this whenever the weight isn't zero,
so as to minimize the number of sqrt() steps. (Yes, I see the point
about not getting into infinite recursion, but that only says that
the threshold needs to be more than 10, not that it needs to be 10^10.)
Also, it seems a little odd to put the recursive calculation of ln(10)
where you did, rather than down where it's used, ie why not
mul_var(result, &fact, result, local_rscale);
ln_var(&const_ten, &ln_10, local_rscale); int64_to_numericvar((int64) pow_10, &ni); mul_var(&ln_10, &ni, &xx,
local_rscale); add_var(result, &xx, result);
round_var(result, rscale);
As you have it, ln_10 will be calculated with possibly a smaller rscale
than is used in this stanza. That might be all right but it seems dubious
--- couldn't the lower-precision result leak into digits we care about?
regards, tom lane