Re: percentage discount not calculating properly

[Chris Travers <..hidden..>]

On Wed, Feb 24, 2010 at 10:17 PM, Luke <..hidden..> wrote:
> On Wed, 24 Feb 2010, Chris Travers wrote:
>
>> On Wed, Feb 24, 2010 at 6:50 PM, Bob Miller <..hidden..> wrote:
>>> your test:
>>> quantity 7, unit price $100.050, discount 15%, extended price $595.29.
>>>
>>> my calculator says:
>>> 7*100.05*.85=595.2975, so I think the correct answer should be
>>> $595.30...
>>
>> change the price to $100.0500 and you will get an extended price of $595.30
>
> but that's only because the deducted 15 percent results in a 4 place
> right of decimal number.  If it had resulted in, say, a 6 place one, the
> only way to cover the case would be to set your product sellprices to
> 100.050000.  More zeros cover more unusual cases.
>
> To me, that seems like something the user really shouldn't have to do,
> although I guess it's better than the alternative of no options.
>
> However...
>
> Starting on line 307 of IS.pm (1.2.18), the linetotal is rounded to 2
> decimal places, no matter what it was before.  Could this be changed to
> use $decimalplaces without causing problems down stream?  That would get
> my stage 2 from a previous message.

I am pretty sure it could.  However, that would be a behavior change
that we might not necessarily want to make for everyone without
getting a lot of input first.  Hence a patch that can be
downloaded/installed would be better.
>
> $decimalplaces, for that matter, is determined from the cellprice, in
> lines 293-295.
> Could we draw that from a default setting instead?

In 1.3, anything which does not draw from
$LedgerSMB::Sysconfig::decimal_places should be be considered a bug
and will be fixed.

>
> Or, for users like me and apparently Bob's customer who want this kind of
> accuracy, can we change the default set in those lines to something like
> 10, without causing problems?  Even changing it to 4, should do the same
> thing as updating the sellprice on all products to pad to 4 zeros to the
> right, shouldn't it? (What is the maximum precision?)

For customers who want something specific prior to 1.4, we should
probably allow a patch which will round this after discount to
something like 10 decimal places.  That should be a pretty easy fix.
For 1.4, we need to have a pretty extensive discussion as to how
exactly we want this handled.  Rounding bias could cause an issue.
For example, if I buy 9 items for a total of 9.99999, that would
display as $10.00 total, and $1.11 each (for $9.99 total if rounded
individually on a report).  The default behavior avoids this problem.
>
> This is Easier for me than for Bob's client, I imagine.
>
> If any of these do cause problems, I'm thinking they could be solved by
> changing line 345:
>
> $form->{total} += $linetotal;
>
> To:
>
> $form->{total} += $form->round_amount($linetotal, 2);
>
> although I'd rather do that rounding after all the items are iterated, I
> just haven't found where yet.  (Line 605 maybe?)
>
>
> As an aside, I've noticed that in perl native, without the use of
> math::BigFloat:
>
> 1.05 with a 91% discount (1.05 - (1.05 * 0.91)) = 0.0944999999999999
> It should be 0.0945.

perl -MMath::BigFloat -e 'print Math::BigFloat->new(1.05) -
(Math::BigFloat->new(1.05) * Math::BigFloat->new(0.91)), "\n"'

0.0945

>
> (perl -e 'print 1.05 - (1.05 * 0.91), "\n";')
>
> I'm sure BigFloat handles that correctly in the Form.pm functions, but
> does it also handle it in the case of IS.pm, and the code which starts on
> line 297?  (1.2.18)
>
> my $discount = $form->round_amount($sellprice *
> $form->parse_amount( $myconfig, $form->{"discount_$i"} ) /
> 100, $decimalplaces);

parse_amount turns the text into a BigFloat so yes this is handled.
Sellprice has already been turned into a BigFloat.

Rounding occurs through BigFloat's native capabilities.

Best Wishes,
Chris Travers