Apache OpenOffice (AOO) Bugzilla – Issue 84934
ODFF: DAYS360 compliance
Last modified: 2013-08-07 15:15:24 UTC
Mail Reader Message Refers: http://www.openoffice.org/servlets/ReadMsg?list=dev&msgNo=21247 User Forum Message Refers: http://user.services.openoffice.org/en/forum/viewtopic.php?f=5&t=1289 In your dialogue you mentions that Excel has a bug in its implementation of 30U/360, and states that Calc MUST be compatible with Excel. Yes Excel has a bug, and NO CALC is NOT compatible with Excel. I have attached a spreadsheet * which compares Excel and Calc against 3OU/360 against a reasonable test cover showing how Excel deviates from 3OU/360 and Calc from Excel * Provides Basic functions which implements * The 3OU/360 Algo * The Excel Algo showing the bug and why it gets the wrong answer * The Calc Algo showing its bug and why it disagrees with Excel. The code in sc/source/core/tool/interpr2.cxx would be trivial to make compatible with Excel, so it seems a shame for this incompatibility to remain. Do you want me to propose a patch?
Created attachment 50619 [details] Spreadsheet showing bug and Basic for correct Algo
Having read the documentation and looked at the test data again, I feel that Calc does provide a sensible implementation of the 30US/360 aka SIA variant algorithm. It's only Excel that is wrong. If we are intending to maintain correctness and therefore Excel incompatibility, then this should at least be reflected in the online help and documented under MSOffice->Excel incompatibilities.
terrye, could you, please, specify two dates where we differ from Excel? Thanks.
Interesting. Needs further investigation, also relevant for ODFF spec. Might also be interesting to compare with older Excel97 implementation to see if anything changed.
kpalagin, I have updated the ODS to V2 to do the extra comparison, This includes the following sheets which evaluate this function for a 36x36 test vector: Excel Evaluated on Excel 2003 Calc Evaluated on Calc 2.3.1 USDays360 Basic implementation of 30/360 (SIA) algo ExDays360 Basic implementation of the Excel algo CalcAlgo360 Basic implementation of the Calc algo DiffExcelCalc Excel-Calc Delta DiffExcelUSDays360 Excel-30/360 Delta DiffCalcUSDays360 Calc-30/360 Delta DiffExcelExDays360 Excel-Its Basic Implementation Delta(==0) DiffCalcCalcAlgo360 Calc-Its Basic Implementation Delta(==0) Ignore my last post of Wed Jan 2 15:27:10. I had a mind fart. The algorithm is for two dates A,B where A<B. Excel gets dates wrong when comparing last days of leap-years. In the defined range Calc and Excel agree, so calc also gets it wrong. Also Microsoft has preserve this same bug since Excel 97 to ensure calculation compatibility. Where they DO differ is in the case where A>B, in that neither error but instead adopt different conventions: Calc flips the algo about the origin so DAYS360(A,B)=-DAYS360(B,A) and Excel just extrapolates (meaninglessly) backwards so that the two randomly differ by up to 2 days. The point is that users may use DAYS360 outside of its strict use in actuarial calcs and win such cases we might get A>B. In such cases loading a "working" (that is doing what the user thinks is OK) spreadsheet into Calc will give different results.
Created attachment 50643 [details] Version 2 of test cases
Here's the patch which makes Calc 100% compatible with Excel on this one: sc/source/core/tool/interpr2.cxx line 335 < if (nDate2 < nDate1) > if (bFlag && (nDate2 < nDate1)) Tested on my OOo 2.2.1 sandpit, but I don't think module this was changed in 2.3 or 2.3.1 so this same patch should apply.
terrye, I am not sure if you need to file Joint Copyright Agreement and attach .patch file for one-liner, but suggest doing so to speed-up acceptance of this change. I am also CCing Eike Rathke, project lead for Calc. Thanks for your effort!
Already signed the JPA, as this isn't my first fix. I'll submit the patch. One for er to ponder, one way of retaining Excel compatability for this function, plus allowing a 30/360 SIA compliant option would be to over load parameter 3. At the moment it is a boolean. Why not allow an integer argment also. Hence False = 0 = 30/360 Excel True = -1,1 = 30E/360 2 = 30/360 SIA compliant. Clearly export to XLS should map arg 3 back to T/F so you lose compliance.
Hi Eike, seems to be yours. Frank
Eike, I could give you a diff file for this, but I don't have a CVS acct to give you a cvs diff, so why don't just leave this to you. It's interpr2.cxx:336, change as above.
No problem with a one line change like the one above. However, to produce a cvs diff -pu you don't need a CVS account, anoncvs does as well, check out sources using cvs -d :pserver:anoncvs@anoncvs.services.openoffice.org:/cvs ... Are you sure that change doesn't break the other !bFlag case? I didn't check yet. Regarding > parameter 3. At the moment it is a boolean. Why not allow an integer argment > also. Hence > False = 0 = 30/360 Excel > True = -1,1 = 30E/360 > 2 = 30/360 SIA compliant. That wouldn't do. While zero is interpreted as False, any other value than zero is interpreted as True. Overloading that in future versions could break already existing documents that don't care if True is 1 or some other value than 0, e.g. as a result of an expression. ANother thing: there once was a reference document SMD_Fields_030802.pdf publicly available at some financial services provider or so that claimed the Excel method was called PSA 30 or NASD 30, and Excel was the only application implementing that. Unfortunately the document isn't available anymore. Do you happen to know if that is some "official" name, or whether there are public references one could point to? Thanks Eike
Created attachment 50766 [details] Ditto + 3 extra sheets givien 30E/360 test vectors + validation delta
> However, to produce a cvs diff -pu you don't need a CVS account, > anoncvs does as well, check out sources using > cvs -d :pserver:anoncvs@anoncvs.services.openoffice.org:/cvs ... Thanks, I should have thought about anoncvs. This gives enough to google up some references. > Are you sure that change doesn't break the other !bFlag case? I didn't check yet. No this guard condition is to do the flip *ONLY* if true. It therefore leaves the true case unchanged. "Trust but verify" I've added the extra 30E/360 check in the test spreadsheet to cover this one explicitly. The TRUE option used to and still works. >> ... parameter 3. ... > That wouldn't do. While zero is interpreted as False, any other value > than zero is interpreted as True. Fine, your the chief engineer for this module. > ANother thing: there once was a reference document SMD_Fields_030802.pdf > publicly available at some financial services provider or so that > claimed the Excel method was called PSA 30 or NASD 30, and Excel was the > only application implementing that. Unfortunately the document isn't > available anymore. Do you happen to know if that is some "official" > name, or whether there are public references one could point to? No primary references. These still seem to be in paper form only. Google "wiki 360 day calendar". I updated the Wikipedia article, and in researching ths came across two good secondary refs: http://www.financialcad.com/support/developerfunc/mathref/Daycount.htm http://www.sifma.org/services/publications/calculations-method-volume2.shtml
In cws odff03: sc/source/core/tool/interpr2.cxx 1.35.20.1
Reassigning to QA for verification.
verified in internal build cws_odff3
Adding a MS reference to this issue: http://support.microsoft.com/kb/916004 "When you use the DAYS360 function to calculate the number of days between two dates, an unexpected value is returned." The article continues: "when you use the DAYS360 function with a start date of February 28 and with an end date of March 28, a value of 28 days is returned. You expect a value of 30 days to be returned for every full month." and I point out that you do actually expect 28 days, not 30. Possibly MS are aware of their bug, but their authors have not explained it properly.
The number of days Excel returns in this constellation actually depends on two factors: - Whether it is a leap year. - The method given as a 3rd argument to DAYS360.
Created attachment 55697 [details] test case illustrating some Feb-28 behavior
closed because fix available in builds OOO300_m9 and DEV300_m33