Bug 69569 - Implementation of YEARFRAC function inconsistent with Excel
Summary: Implementation of YEARFRAC function inconsistent with Excel
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Calc (show other bugs)
Version:
(earliest affected)
unspecified
Hardware: Other All
: medium normal
Assignee: Winfried Donkers
URL:
Whiteboard: target:6.3.0 target:6.2.0.1
Keywords:
Depends on:
Blocks: Excel-Functions
  Show dependency treegraph
 
Reported: 2013-09-19 11:27 UTC by Christian Fries
Modified: 2022-09-10 16:57 UTC (History)
11 users (show)

See Also:
Crash report or crash signature:


Attachments
Attachment: Spreadsheet to verify implementation (requires LibreOffice Add-In Obba.oxt) (2.27 MB, application/x-zip)
2013-09-19 11:27 UTC, Christian Fries
Details
Excel vs Calc comparison (160.04 KB, application/vnd.openxmlformats-officedocument.spreadsheetml.sheet)
2017-02-07 18:08 UTC, Lucian Mocanu
Details
Excel vs. Calc (any Calc version) comparison (136.99 KB, application/vnd.oasis.opendocument.spreadsheet)
2018-12-08 18:58 UTC, Winfried Donkers
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Christian Fries 2013-09-19 11:27:16 UTC
Created attachment 86136 [details]
Attachment: Spreadsheet to verify implementation (requires LibreOffice Add-In Obba.oxt)

The implementation of YEARFRAC(start, end, basis) for basis = 1 does not agree with
a) the Excel implementation and
b) with the OASIS Documentation (remark: in addition it appears as if the OASIS Documentation has a typo) and
c) with the OpenOffice implementation (which doesn't agree with Excel either).

Remark: b) might have got fixed via bug 40100, but since the OASIS Documentation is not compliant with Excel a) remains (and it is likely that future "bug" reports will pop up) - see below.
Remark: Don't care about c), the implementation is even worse.

A re-implementaiton of the Excel 2013 YEARFRAC(start, end, basis) can be found here:
http://svn.finmath.net/finmath%20lib/trunk/src/main/java/net/finmath/time/daycount/DayCountConvention_ACT_ACT_YEARFRAC.java

A spreadsheet to test the day count methods can be found at
http://finmath.net/spreadsheets/Day%20%Count%Fractions.zip


** On the OASIS Documentation **

The algorithm documented in https://www.oasis-open.org/committees/document.php?document_id=39507 appears to be not compliant with Excel's implementation. For Procedure E line 65 states "if A and is-leap-year(year(date1)) then return 366". However, condition A is "year1 != year2". It appears as if this would imply the rule "if is-leap-year(year(date1)) and is-leap-year(year(date2)) then return 365" (which is not what OpenOffice is doing, neither LibreOffice, nor Excel - and which does not make sense).

For the implementation of Excel line 65 should read

8. Otherwise, if is-leap-year(year(date1)) and is-leap-year(year(date2)) return 366.

LibreOffice is a bit closer to Excel than OpenOffice is, but both are wrong. LibreOffice 4.1 implements in the rule 8. as "is-leap-year(year(date1)) OR is-leap-year(year(date2))"


** On the Excel Implementation **

In another comment it was claimed, that Excel implements ACT/ACT AFB. I do find a proof for this claim. In fact, I believe that ACT/ACT AFB is slightly different.
That said, I would like to remark, that in many financial applications act/act day count fraction are calculated using ACT/ACT ISDA. This method has some advantages and the algorithm is much simpler. An implementation of ACT/ACT ISDA can be found at http://svn.finmath.net/finmath%20lib/trunk/src/main/java/net/finmath/time/daycount/DayCountConvention_ACT_ACT_ISDA.java

See also http://svn.finmath.net/finmath%20lib/trunk/src/main/java/net/finmath/time/daycount/ and http://finmath.net/topics/daycountingandschedules


** Test Cases **

YEARFRAC(30.08.1984, 06.07.1990, 1)
OpenOffice  4.0:	5,850...	(NOT OK)
LibreOffice 4.1:	5,847...	(OK)
Excel 2013.....:	5,847...

YEARFRAC(30.12.1999, 04.01.2000, 1)
OpenOffice  4.0:	5/365		(OK)
LibreOffice 4.1:	5/366		(NOT OK)
Excel 2013.....:	5/365

YEARFRAC(30.12.2000, 04.01.2001, 1)
OpenOffice  4.0:	5/366		(NOT OK)
LibreOffice 4.1:	5/366		(NOT OK)
Excel 2013.....:	5/365


** Suggested Fixes **

- Make the implementation compliant with Excel's implementation (both do not implement a standard, so I would call Excel's implementation a reference).
- Make the documentation compliant with Excel's implementation.
- Consider adding ACT/ACT ISDA.
Comment 1 Christian Fries 2013-09-19 11:38:33 UTC
Note: There was a typo in the url to the test spreadsheets. The url is http://finmath.net/spreadsheets/Day%20Count%20Fractions.zip - but I attached that file anyway.
Comment 2 Christian Fries 2013-09-19 16:16:07 UTC
Note: I have checked the modifications made by Lionel Elie Mamane at http://cgit.freedesktop.org/libreoffice/core/commit/?id=0f002a14ecdc445deb2bc8d4cc6fbe4b724ac7f6&h=libreoffice-4-1
- just looking at the code, since I have no access to the daily build.

It appears as if this modification makes LibreOffice consistent with the documentation BUT not consistent with Excel's implementation of YEARFRAC.

To make that precise. If you take the implementation at
http://svn.finmath.net/finmath%20lib/trunk/src/main/java/net/finmath/time/daycount/DayCountConvention_ACT_ACT_YEARFRAC.java

the difference is in the line
			if(isStartLeapYear && isEndLeapYear)
here LibreOffice uses
			if(isStartLeapYear && !isEndLeapYear)
instead.
Comment 3 Robinson Tryon (qubit) 2014-07-03 15:09:04 UTC
(In reply to comment #2)
> Note: I have checked the modifications made by Lionel Elie Mamane...
> It appears as if this modification makes LibreOffice consistent with the
> documentation BUT not consistent with Excel's implementation of YEARFRAC.

It looks like we have two choices here
1) Change our code to become consistent with Excel's implementation
2) Document the difference, e.g. on https://help.libreoffice.org/Calc/YEARFRAC

Lionel - Thoughts?
Comment 4 Lionel Elie Mamane 2014-07-03 15:47:09 UTC
(In reply to comment #3)
> (In reply to comment #2)
> > Note: I have checked the modifications made by Lionel Elie Mamane...
> > It appears as if this modification makes LibreOffice consistent with the
> > documentation BUT not consistent with Excel's implementation of YEARFRAC.
> 
> It looks like we have two choices here
> 1) Change our code to become consistent with Excel's implementation
> 2) Document the difference, e.g. on
> https://help.libreoffice.org/Calc/YEARFRAC
> 
> Lionel - Thoughts?

I will not, under my decision / responsibility, make LibreOffice Calc depart from the ODF standard. Whether the ODF standard should be "fixed" to match Excel, or if the YEARFRAC option should be a different function in .ods files and in .xls(x) files is a question I punt to the Calc experts.

I consider authoritative ODF 1.2 http://docs.oasis-open.org/office/v1.2/os/OpenDocument-v1.2-os-part2.html#__RefHeading__1017922_715980110
as opposed to https://www.oasis-open.org/committees/document.php?document_id=39507

I came into bug 40100 as a *user* because LibreOffice was giving me wrong results for an "obvious" case like "=YEARFRAC(DATE(2023;1;1);DATE(2024;1;1);1)", so I fixed it. As long as I'm able to get sane results fom my LibreOffice in some way, I don't care much about Excel compatibility. I mean: if Excel gives results that are "wrong", then either give correct results in LibreOffice, or have "basis 5" be the "right result" in LibreOffice, so that I can do "=YEARFRAC(DATE(2023;1;1);DATE(2024;1;1);5)" and get the result I expect. But don't cripple my LibreOffice for the sake of Excel compatibility.
Comment 5 Christian Fries 2014-07-03 16:05:15 UTC
Maybe I would like to add that the ACT/ACT implementation of Excel AND LibreOffice is wrong - in the sense that is does not agree with the (most popular) ISDA definition of ACT/ACT.

I would vote for including new „basis“ definitions and have basis 4 be the ACT/ACT EXCEL.

Am 03.07.2014 um 17:47 schrieb bugzilla-daemon@freedesktop.org:

> Lionel Elie Mamane changed bug 69569 
> What	Removed	Added
> CC	  	kyoshida@slickedit.com, libreoffice@kohei.us, markus.mohrhard@googlemail.com            
> 
> Comment # 4 on bug 69569 from Lionel Elie Mamane
> (In reply to comment #3)
> > (In reply to comment #2)
> > > Note: I have checked the modifications made by Lionel Elie Mamane...
> > > It appears as if this modification makes LibreOffice consistent with the
> > > documentation BUT not consistent with Excel's implementation of YEARFRAC.
> > 
> > It looks like we have two choices here
> > 1) Change our code to become consistent with Excel's implementation
> > 2) Document the difference, e.g. on
> > https://help.libreoffice.org/Calc/YEARFRAC
> > 
> > Lionel - Thoughts?
> 
> I will not, under my decision / responsibility, make LibreOffice Calc depart
> from the ODF standard. Whether the ODF standard should be "fixed" to match
> Excel, or if the YEARFRAC option should be a different function in .ods files
> and in .xls(x) files is a question I punt to the Calc experts.
> 
> I consider authoritative ODF 1.2
> http://docs.oasis-open.org/office/v1.2/os/OpenDocument-v1.2-os-part2.html#__RefHeading__1017922_715980110
> as opposed to
> https://www.oasis-open.org/committees/document.php?document_id=39507
> 
> I came into bug 40100 as a *user* because LibreOffice was giving me wrong
> results for an "obvious" case like
> "=YEARFRAC(DATE(2023;1;1);DATE(2024;1;1);1)", so I fixed it. As long as I'm
> able to get sane results fom my LibreOffice in some way, I don't care much
> about Excel compatibility. I mean: if Excel gives results that are "wrong",
> then either give correct results in LibreOffice, or have "basis 5" be the
> "right result" in LibreOffice, so that I can do
> "=YEARFRAC(DATE(2023;1;1);DATE(2024;1;1);5)" and get the result I expect. But
> don't cripple my LibreOffice for the sake of Excel compatibility.
> 
> You are receiving this mail because:
> You reported the bug.
Comment 6 Lionel Elie Mamane 2014-07-03 16:38:38 UTC
I played a bit with Excel and LibreOffice. I found one example where they differ:

=YEARFRAC(DATE(2024;3;1);DATE(2025;3;1);1)

This, naively, should be "1". That's what Excel gives. LibreOffice gives 365/366...
Comment 7 Lionel Elie Mamane 2014-07-03 16:45:23 UTC
(In reply to comment #6)
> I played a bit with Excel and LibreOffice. I found one example where they
> differ:
> 
> =YEARFRAC(DATE(2024;3;1);DATE(2025;3;1);1)
> 
> This, naively, should be "1". That's what Excel gives. LibreOffice gives
> 365/366...

This is because of clause 8 of ODFv1.2 part 2, §4.11.7.7
I guess I'm slipping into the "ODF has the wrong definition/algorithm" camp.
Comment 8 Christian Fries 2014-07-03 19:26:45 UTC
(In reply to comment #7)
>
> This is because of clause 8 of ODFv1.2 part 2, §4.11.7.7
> I guess I'm slipping into the "ODF has the wrong definition/algorithm" camp.

If would like to add that both Excel and LibreOffice have a wrong algorithm - in the following sense that both algorithms do not comply with the ACT/ACT ISDA definition which is
a) the most popular definition of ACT/ACT with relevance to industry and
b) much simpler (=n1/365 + n2/366 where n1 = number of days falling in non-leap year and n2 = number of days falling in leap year).

So "naively should be" or "defined in ODF" is IMHO not very relevant. 

How about:
- use base 4 for the Excel compatible implementation
- introduce a new base or even a new function for ACT/ACT ISDA
- if necessary, implement LibreOffices method too (as part of the new function of base 6?)

Or:
- Have the definition of base 4 configurable.
Comment 9 Lionel Elie Mamane 2014-07-03 20:12:02 UTC
(In reply to comment #8)
> (In reply to comment #7)

>> This is because of clause 8 of ODFv1.2 part 2, §4.11.7.7
>> I guess I'm slipping into the "ODF has the wrong definition/algorithm" camp.

> How about:
> - use base 4 for the Excel compatible implementation

You mean "base 1" maybe? "base 4" is "European 30/360".
Comment 10 Robinson Tryon (qubit) 2014-07-27 02:14:10 UTC
(In reply to comment #7)
> (In reply to comment #6)
> > I played a bit with Excel and LibreOffice. I found one example where they
> > differ:
> > ...
> I guess I'm slipping into the "ODF has the wrong definition/algorithm" camp.

Sounds like this bug is relevant enough for us to carry on serious discussion about our implementation and the ODF spec, hence I'm going to move it to NEW.
Comment 11 QA Administrators 2015-09-04 02:48:07 UTC Comment hidden (obsolete)
Comment 12 QA Administrators 2016-09-20 10:29:09 UTC Comment hidden (obsolete)
Comment 13 Lucian Mocanu 2017-02-07 18:08:17 UTC
Created attachment 130995 [details]
Excel vs Calc comparison

Spreadsheet with formulas, Excel results, Calc results and comparison between the two.
Comment 14 Lucian Mocanu 2017-02-07 18:15:55 UTC
I can confirm that the bug is still present in LibreOffice 5.3.0.

The ODF specification (v1.2 part 2 section 4.11.7.7) describes (almost) the same behaviour as in Excel (there might be differences in ODF vs OOXML when the two dates are exactly one year apart), but the values calculated in LibreOffice are wrong (according to the standard).
Comment 15 QA Administrators 2018-07-25 02:39:34 UTC Comment hidden (obsolete)
Comment 16 Christian Fries 2018-07-30 20:57:32 UTC
Retested this issue with LIBRE Office 5.4.7 and 6.0.5. It still exists.
Comment 17 dahrion 2018-08-10 08:26:40 UTC
I had to implement the yearfrac function and can confirm that the error is in the specification.
Specifically in ODFv1.2 part 2, §4.11.7.7 line 8
it shoul be not(A)
also in line 9 the between is inclusive

How did i came to this conclusion ?
the specification is clearly based on the following pdf
https://www.dwheeler.com/yearfrac/excel-ooxml-yearfrac.pdf

if you check the appears_le_year(date1, date2) you have
y1==y2 || ((y1+1)==y2 && (m1>m2 || (m1==m2 && d1>=d2)))

the odf specify the inverse of that clause:

y1==y2 || ((y1+1)==y2 && (m1>m2 || (m1==m2 && d1>=d2)))
!(y1==y2) && !((y1+1)==y2 && (m1>m2 || (m1==m2 && d1>=d2)))
!(y1==y2) && (!(y1+1)==y2 || !(m1>m2 || (m1==m2 && d1>=d2)))
!(y1==y2) && (!(y1+1)==y2 || (!m1>m2 && !(m1==m2 && d1>=d2)))
!(y1==y2) && (!(y1+1)==y2 || (!m1>m2 && (!m1==m2 || !d1>=d2)))
(y1!=y2) && ((y1+1)!=y2 || (m1<=m2 && (m1!=m2 || d1<d2)))

(m1<=m2 && (m1!=m2 || d1<d2)) = (m1<=m2 && m1!=m2)||(m1<=m2 && d1<d2) = m1<m2 || (m1<=m2 && d1<d2) = m1<m2 || (m1=m2 && d1<d2)

(y1!=y2) && ((y1+1)!=y2 ||  m1<m2 || (m1=m2 && d1<d2))

which is line 6 in the spec: Evaluate F: (A and B) or (A and C) or (A and D and E)

also the pdf doesnt specify the feb29_between(date1, date2) function, nor the specification; especially its vagueness about the dates being inclusive or exclusive.

so we can consider the specification as a representation of the algorithm in the pdf
the pdf states that:
if (date1.year == date2.year and is_leap_year(date1.year)):
 year_length = 366.

which is equivalent to line 8 of the specs except with not(A) instead of A

The only inconsistency left is the 29 between the dates date1 and date2, we wrongly assume it is exclusive in both date based on line 10 in the specs wich is the equivalent of the test ||(date2.month == 2 and date2.day == 29) in the pdf
with lot of testing it appears that the date1 is included when checking feb29_between(date1, date2)

so far my implementation is the same as excel
Comment 18 Christian Fries 2018-08-10 20:21:35 UTC
I would like to add an aspect to this issue:

LibreOffice claims that it can open Excel Files. It claims compatibility.
https://www.libreoffice.org/discover/calc/

The Excel YEARFRAC function is mapped to the LibreOffice YEARFRAC function. So with respect to compatibility it is mandatory that LibreOffice YEARFRAC behaves like Excel.

I had provided the Java Code for Excel YEARFRAC in my comment 2013-09-19. (Unclear to me why this comment has been tagged as obsolete.)

If LibreOffice.org believes to add an additional ACT/ACT ISDA - then maybe as a separate function.
Comment 19 Winfried Donkers 2018-11-13 13:06:19 UTC
(In reply to dahrion from comment #17)
> I had to implement the yearfrac function and can confirm that the error is
> in the specification.
> Specifically in ODFv1.2 part 2, §4.11.7.7 line 8
> it shoul be not(A)
> also in line 9 the between is inclusive

There seems to be another aberration in  ODFv1.2 part 2, §4.11.7.7.
I'm checking this now.
Comment 20 Winfried Donkers 2018-11-14 10:16:11 UTC
(In reply to Winfried Donkers from comment #19)
> (In reply to dahrion from comment #17)
> > I had to implement the yearfrac function and can confirm that the error is
> > in the specification.
> > Specifically in ODFv1.2 part 2, §4.11.7.7 line 8
> > it shoul be not(A)
> > also in line 9 the between is inclusive
> 
> There seems to be another aberration in  ODFv1.2 part 2, §4.11.7.7.
> I'm checking this now.

That was a false alarm. It looked as if not all cases when line 7 does not apply (i.e. F is false) were covered, but they are.

I can confirm that line 8 is incorrect and should be
   8.Otherwise, if (not A) and is-leap-year(year(date1)) then return 366 .

I will try to have a proposal for change of ODFF part 2 §4.11.7.7 submitted.
And then I will change the source code for YEAFRAC to reflect the proposal.
Comment 21 Winfried Donkers 2018-12-08 18:58:45 UTC
Created attachment 147391 [details]
Excel vs. Calc (any Calc version) comparison

This document can be used to check if a new version has the same results as Excel has. The difference with attachment #130995 [details] is that the Calc- and diff-sheets have formulas.
The fix I'm working on no longer has any cell different from Excel.
Hope to finish the fix and tests soon.

The fix is based on the following changes (underlined) of ODF1.2  part 2, §4.11.7.7:
1) constraint to be added _date1__<=__date2_ (or swap them if not)
2) Procedure E, line 8 must be _!_A and isleapyear(year1) ... 
3) Procedure E, lines 9 and 10 to be combined to 
   if a February 29 occurs between date1 (inclusive) and date2 (inclusive) ...
A proposal for change has been submitted by now or will be very soon.
Comment 22 Eike Rathke 2018-12-10 19:52:10 UTC
Proposal submitted with https://lists.oasis-open.org/archives/office-comment/201812/msg00000.html

And thanks Winfried for taking care of this and the extensive test case document!
Comment 23 Winfried Donkers 2018-12-11 07:10:59 UTC
(In reply to Eike Rathke from comment #22)
> Proposal submitted with
> https://lists.oasis-open.org/archives/office-comment/201812/msg00000.html
> 

Thanks for submitting.

> And thanks Winfried for taking care of this and the extensive test case
> document!

Those thanks are largely meant for Lucian Mocanu who did all the groundwork with attachment #130995 [details]. I merely modified the document to make it useful for testing.
Comment 24 Commit Notification 2018-12-12 15:00:18 UTC
Winfried Donkers committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/+/22be500b1624753decec43c0de4ccb1c5f7a21d4%5E%21

tdf#69569 implement proposed change to ODF1.2 part 2 §4.11.7.7

It will be available in 6.3.0.

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

Affected users are encouraged to test the fix and report feedback.
Comment 25 Commit Notification 2018-12-13 13:18:16 UTC
Winfried Donkers committed a patch related to this issue.
It has been pushed to "libreoffice-6-2":

https://git.libreoffice.org/core/+/6441ffbeacc5b107430accbdf5f82f6ad4269f94%5E%21

tdf#69569 implement proposed change to ODF1.2 part 2 §4.11.7.7

It will be available in 6.2.0.1.

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

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