Created attachment 127702 [details]
Document showing problems

Attached document was based on function test document.
Changes are that the column 'Expected' contains the proper results that should be returned (when result is known).
Also some extra tests have been added.

Sheet 'precision issues' shows problems with floating point precision, where MOD doesn't produce correct results and where I can't see the cause of the problem as the values are less than the maximum integer value for double precision IEEE754 numbers.

Fix currently produces correct results for all tests in sheet2 that have a white background in column 'Expected'.

(In reply to Winfried Donkers from comment #0)
> Actual Results:  
> 5
> 
> 
> 
> 
> 
> Reset User Profile?No

In the empty lines read
 Expected Results:
 0

Apache OpenOffice has the same code in ScMod in interpr2.cxx and it returns the correct values for =MOD((3^31)-1;14), =MOD((3^31)-2;14) and =MOD((3^31)-1;14). Therefore the problem is not directly in that method. I wonder where LibreOffice is different to Apache OpenOffice.

The if-part, which handles pure integer numbers should be able to handle values up to 53 bit correctly. Removing that part is not the solution to fix the wrong results in the 3^31 cases.

The case =MOD(1.485*1000;5) is a rounding problem. 1.485*1000 does not result in a pure integer, and so it goes to the else-part. If you use the direct integer =MOD(1485;5), the result is correct. So it will not help to remove that part, which handles pure integer numbers.

(In reply to Regina Henschel from comment #3)

Regina, thank you for your input.

The cause is not (only) in the ScMod() function, it is in ::rtl::math::approxXxxx() functions (as well) as not all these go up to 53 bit values. 
The 3^31 cases are caused by these math functions, some tests show that they can be fixed there. AOO probably has different code in e.g. approxEqual(x, y).

WRT MOD(1.485*1000;5), the else part returns the correct result.

WRT the if part in ScMod(): it uses floor(), where ::rtl::math::approxFloor() is used in the else part. Whe retaining the if part I would at least unify the use of floor functions.

I had written the if-part to catch those values, which are true integers and very large, and handle them with fmod. The related issue is https://bz.apache.org/ooo/show_bug.cgi?id=59153.

Do you have a build to test, whether in LO in case =MOD((3^31)-1;14) the if-part is really used?

Perhaps Eike has an idea, why the very same code works in AOO and not in LO. I refer to the cases =MOD((3^31)-1;14). 

The problem MOD(1.485*1000;5) is different.

In a new document I cannot reproduce the "=MOD((3^31)-1;14) results 0"-error. I always get result 2 using LO4.2, LO4.5, LO5.2, LO5.3, all in Windows7 32bit.

(In reply to Regina Henschel from comment #6)
> In a new document I cannot reproduce the "=MOD((3^31)-1;14) results
> 0"-error. I always get result 2 using LO4.2, LO4.5, LO5.2, LO5.3, all in
> Windows7 32bit.

Regina, I will not contradict your findings. 
I have not submitted a definite fix yet, there are currently only ongoing investigations. What I e.g. did was remove all extra code and just retain the pure mathematical definition of a modulo function to be able to test that at least that is working 100%, because currently http://bugs.documentfoundation.org/attachment.cgi?id=127702 opened in version 5.2 or 5.3 show unexpected results in Sheet2, cells A9..A13, (A16,) A20, A23.

If there is no easy fix to make the interpreter support large number, we should return an error like '#NUM!' when values overflow.

=MOD((3^31),14)=#NUM!

Is much better than an erroneous result. In your workbook A16 and A28:A31, expected value should be #NUM!.

(In reply to Luke from comment #8)
> If there is no easy fix to make the interpreter support large number, we
> should return an error like '#NUM!' when values overflow.

In case of overflow, Calc returns an error.
But it is not always possible to represent a value exactly in a binary value, for fractional values, but also for large values. In these cases a binary value as close as possible to the decimal value is used. This is general behaviour for all numeric operations in Calc (and in other applications).
Making MOD() return an error -apart from the problem to determine _if_ the result is incorrect or inaccurate- would be inconsistent with the return values of the other numeric operations in Calc.

Calc uses IEE754 double precision values, which is 64bit.

(In reply to Winfried Donkers from comment #9)

> Making MOD() return an error -apart from the problem to determine _if_ the
> result is incorrect or inaccurate- would be inconsistent with the return
> values of the other numeric operations in Calc.
> 
> Calc uses IEE754 double precision values, which is 64bit.

Then shouldn't we make them consistent and improve our overflow detection? I just tested '=MOD((3^31),14)' on Google Sheets, Excel, and WPS Sheets. They all return #NUM!. Returning an error is far superior than retuning the wrong result. We seem to have a huge range of numbers that generate invalid results. Calc doesn’t start correctly start warning the user of #NUM! until ‘=MOD((3^700),14).  


If our underlying data structure should be able to handle slightly larger calculations, it would be a nice enhancement to improve it. But if that’s all you plan to do to fix this bug, should I file a separate bug report on the more serious issue of retuning invalid results?

(In reply to Luke from comment #10)
> Then shouldn't we make them consistent and improve our overflow detection? I
> just tested '=MOD((3^31),14)' on Google Sheets, Excel, and WPS Sheets. They
> all return #NUM!. Returning an error is far superior than retuning the wrong
> result. We seem to have a huge range of numbers that generate invalid
> results. Calc doesn’t start correctly start warning the user of #NUM! until
> ‘=MOD((3^700),14).  

That #NUM! error was caused by Calc recognising an invalid result, i.e. a negative value. For direct integer values one could set a maximum value and return an error with numerator values above that limit, but:
-the problem also applied for fractional values, where no limit value can be defined;
-when the argument does not contain a value, but a reference, calculation or formula, it is not possible to determine if it is an integer value. MOD(1.485*1000,5) returns an incorrect result (before the fix) because of this.
 
 
> If our underlying data structure should be able to handle slightly larger
> calculations, it would be a nice enhancement to improve it. But if that’s
> all you plan to do to fix this bug, should I file a separate bug report on
> the more serious issue of retuning invalid results?

Yes, as it is a generic problem -and a very valid one- a separate bug report would be best.

Fwiw, I can not reproduce a problem with =MOD((3^31),14) and the like, after hard recalc the results in rows 28 to 31 are 3,2,1,0 for me, in 5.1 and 5.2 and master.

The problem with =MOD(1.485*1000,5) is different because 1.485*1000 does not produce a real integer but 1484.9999999999998 because already 1.485 is not exactly representable but 1.4849999999999999 instead, so fmod(1484.9999999999998,5) returns 4.9999999999997726, which then for display is rounded to 5. Check with number format 0.00000000000000E+00 that displays 4.99999999999977E+00

Btw, always applying ROUND(...,12) to the results to test equality is wrong. Results like 2.27373675443232E-13 are not equal to 0 and in this context unexpected.

Winfried Donkers committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=d8257535e348fa0b0a5c269d1aafa710585421d6

tdf#102742 fix wrong results for MOD function

It will be available in 5.3.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.