[GNC-dev] gnc_numeric division with how=GNC_HOW_DENOM_LCD

[Geert Janssens]

Op zaterdag 19 mei 2018 17:49:10 CEST schreef John Ralls:
> > On May 19, 2018, at 2:28 AM, Geert Janssens <geert.gnucash at kobaltwit.be>
> > wrote:
> > 
> > Christopher Lam is busy writing unit tests for our business reports which
> > is great.
> > 
> > However he has run into  a peculiar issue in that he can't create a test
> > invoice entries that properly handle tax or percentages.
> > 
> > I have been tracing in the debugger and I ran into a peculiar bug for
> > which I need some feedback.
> > 
> > The core of the issue is in this line:
> > https://github.com/Gnucash/gnucash/blob/maint/libgnucash/engine/
> > gncEntry.c#L1139
> > 
> > This division is to convert a number as entered by the user into a
> > percentage value.
> > The user enters "10", which the code should interpret as 10%, or 0.01
> > 
> > This works fine with numbers entered by real users in the user interface,
> > however it fails when generating these same numbers via scheme code.
> > 
> > And this is because both get a different denominator: all user entered
> > numbers (in this context) will be stored with a denom of 100000. However
> > when you make a scheme rational 1000000/100000 (so equivalent to 10), it
> > gets reduced to 10/1 by the time it's used in the line above. Clearly
> > guile automatically reduces rationals.
> > 
> > The net effect is that for user entered numbers the line above will divide
> > 1000000/100000 by 100/1 while for scheme generated numbers the division
> > becomes 10/1 by 100/1.
> > 
> > Combining this with how=GNC_HOW_DENOM_LCD makes that the denominator will
> > be fixed to 1 causing 0.1/1 to be rounded to 0.
> > 
> > That's the symptom. But I do have some difficulty determining the actual
> > cause: is gnc_numeric_div doing the wrong thing with GNC_HOW_DENOM_LCD ?
> > That would make it a bug in gnc_numeric_div. In a division I would expect
> > the nominator of the divisor to be taken into account to calculate the
> > LCD but perhaps my (accounting related) math is wrong.
> > Or perhaps GNC_HOW_DENOM_LCD just doesn't make sense for a division and
> > another rounding method should be used here? That would make it a bug in
> > the gncEntryComputeValueInt function.
> > Or both are right and I should just ensure to have a proper common
> > denominator somehow. I could maybe set percent in the calculation above
> > to 10000000/100000 instead of to 100/1.
> > Or I could avoid gnc_numeric_div altogether by multiplying with 1/100
> > instead.
> > 
> > However I wanted some feedback before applying one of the workarounds to
> > be
> > sure I'm not masking a a more important bug that way.
> 
> For reference, the line in question is
> 
> tpercent = gnc_numeric_div (tpercent, percent, GNC_DENOM_AUTO,
>                                             GNC_HOW_DENOM_LCD);
> 
> Note in particular the warning at
> https://github.com/Gnucash/gnucash/blob/maint/libgnucash/engine/gnc-numeric
> .h#L80
> <https://github.com/Gnucash/gnucash/blob/maint/libgnucash/engine/gnc-numeri
> c.h#L80> that the fractional part of a number may be discarded if you don’t
> specify a rounding policy. This call has a denominator setting but no
> rounding policy.
> 
> I think GNC_HOW_DENOM_LCD doesn’t make sense for most divisions, it’s very
> likely that one wants more precision than the smaller of the operands'
> denominators. I think for this case you want GNC_HOW_DENOM_EXACT |
> GNC_HOW_ROUND_NEVER or GNC_HOW_DENOM_REDUCE | GNC_HOW_ROUND_NEVER: That
> preserves all of the precision that a user enters, though it does present
> the (small) possibility of an overflow error. GNC_HOW_DENOM_REDUCE |
> GNC_HOW_ROUND_RND will trade the small possibility of an overflow for the
> small possibility of a rounding error in the actual tax calculation.
> 
> Multiplying 10/1  by 1/100 won’t change anything: The LCD is still 1.

LCD yes, but reading the LDC code in more detail we're actually using LCM 
(least common multiplier) which in this case would be 100.

It doesn't matter though. I think GNC_HOW_DENOM_REDUCE | GNC_HOW_ROUND_NEVER 
will be find in this case. We're talking about tax percentages and discounts. 
If those overflow it must have been a very unusual number.

Thanks for your feedback.

Geert