Bug 385993

Summary: Error in investment interest (Jahresrendite)
Product: [Applications] kmymoney Reporter: Henser Deas <bruno.glaser>
Component: databaseAssignee: KMyMoney Devel Mailing List <kmymoney-devel>
Status: RESOLVED FIXED    
Severity: normal CC: lukasz.wojnilowicz, nate, ralf.habacker
Priority: NOR Keywords: usability
Version: 4.8.0   
Target Milestone: ---   
Platform: Microsoft Windows   
OS: Microsoft Windows   
Latest Commit: Version Fixed In: 4.8.3,5.0.3
Sentry Crash Report:
Attachments: Sometimes error in investment return calculation
libreoffice XIRR test case

Description Henser Deas 2017-10-20 15:48:17 UTC
Created attachment 108479 [details]
Sometimes error in investment return calculation

I have entered 39 booking for investments (buy, dividend, sell) and sometimes, there are obvious wrong calculations for investment return (Jahresrendite), i.e. there should be a clear positive result, but there is a strong negative results (around -98%). This happend to five out of my 39 bookings. I guess it has to to with assignment of starting date for my investments. Maybe the error can be solved by eliminating any date to investments analogous to a similar previous error.
Errors also change if I added new investments.

Many thanks for solving the problem as the function is essential to myself using the program. The error both occurs in KMyMoney 4.6.4 in Kubuntu and KMyMoney 8.2 in Win7.
Comment 1 wojnilowicz 2017-10-22 19:15:38 UTC
There are two columns on investment performance report: Annualized Return and Return On Investment. Which one displays wrong for you and for which mutual fund?
Comment 2 Henser Deas 2017-10-23 18:31:50 UTC
Dear Lukasz,

annualized return is wrong for following investments:

ACMGI American Growth Trends (buy: 3472,57, sell: -6051,36, annualized 
return: -92,78%

BGF Latin American A2 Fund (buy: 4335,75 €, sell: -6056,94 €, annualized 
return:-88,34%)

BGF World Mining Fund (buy: 3155,58 €, sell: -3768,71€, annualized 
return: -87,65%)

Carlson Asian Small Cap (buy: 2248,89 €, sell: -2828,80 €, annualized 
return: -92,75%)

db-xtracker Banks Short (buy: 6738,80 €, sell: -7375,06 €, annualized 
return: -98,26%)


The strange thing is that the rest of my investments were calculated 
correctly. I cross-checked this with MS Money99.

Many thanks for your help.

Best wishes

Bruno




On 10/22/17 21:15, NSLW wrote:
> https://bugs.kde.org/show_bug.cgi?id=385993
>
> NSLW <lukasz.wojnilowicz@gmail.com> changed:
>
>             What    |Removed                     |Added
> ----------------------------------------------------------------------------
>                   CC|                            |lukasz.wojnilowicz@gmail.co
>                     |                            |m
>
> --- Comment #1 from NSLW <lukasz.wojnilowicz@gmail.com> ---
> There are two columns on investment performance report: Annualized Return and
> Return On Investment. Which one displays wrong for you and for which mutual
> fund?
>
Comment 3 Ralf Habacker 2018-10-31 01:41:57 UTC
Created attachment 115997 [details]
libreoffice XIRR test case

The appended libreoffice test case using XIRR returns a value of 17,26% for the first entry, which indicates that the related calculation in kmymoney seems to be broken in CashFlowList::calculateXIRR().
Comment 4 Ralf Habacker 2018-10-31 22:45:10 UTC
Class CashFlowList has a dumpAll() method, which shows 

"2005-09-22"   "-347257/100" 
"2009-03-18"   "151284/25" 
"1989-11-01"   "0/1" 
"2017-08-11"   "0/1" 

Adding this values to an additional QueryTableTest method as: 

void QueryTableTest::testCashFlowAnalysis2()
{
  CashFlowList list;

  list += CashFlowListItem(QDate(2005, 9, 22), MyMoneyMoney(-3472.57));
  list += CashFlowListItem(QDate(2009, 3, 18), MyMoneyMoney(6051.36));
  list += CashFlowListItem(QDate(1998, 11, 1), MyMoneyMoney(0.0));
  list += CashFlowListItem(QDate(2017, 8, 11), MyMoneyMoney(0.0));

  MyMoneyMoney IRR(list.IRR(), 1000);

  QCOMPARE(IRR.toDouble(), MyMoneyMoney(173, 1000).toDouble());
}

returns:

FAIL!  : QueryTableTest::testCashFlowAnalysis2() Compared doubles are not the same (fuzzy compare)
   Actual (IRR.toDouble()): -0.928
   Expected (MyMoneyMoney(173, 1000).toDouble()): 0.173
   Loc: [/home/ralf/src/kmymoney-4.8/kmymoney/reports/querytabletest.cpp(365)]

Removing the ling 

  list += CashFlowListItem(QDate(2017, 8, 11), MyMoneyMoney(0.0));

let the test be successful.
Comment 5 Ralf Habacker 2018-11-01 20:18:13 UTC
>"1989-11-01"   "0/1" 
>"2017-08-11"   "0/1" 
These values are the Initial and Ending Market Values and are listed in the "Transaction by Account" report.

>  list += CashFlowListItem(QDate(2017, 8, 11), MyMoneyMoney(0.0));

Using the following line instead 

  list += CashFlowListItem(QDate(2014, 8, 11), MyMoneyMoney(0.0));

let 

  double value = list.IRR();

return a nan, which let the following code 

  MyMoneyMoney IRR(list.IRR(), 1000);

crash with a floating-point exception.

The question is whether the initially mentioned market values ​​are required, or whether they can be omitted if they are zero.
Comment 6 Ralf Habacker 2018-11-01 20:28:38 UTC
Another source of negative annual returns is incomplete transactions.
The investment font "Carmignac Emergents" e.g. will be sold in a transaction (31.7.2006) and bought back later (18.11.2007). No annual return can be determined between these transactions (the equivalent libreoffice XINTZINSFUSS function returns an error in this order)
In this case, in my opinion, the annual return should be "not identifiable"
Comment 7 Ralf Habacker 2018-11-06 09:47:27 UTC
related review request: https://phabricator.kde.org/D16700
Comment 8 Ralf Habacker 2018-11-09 12:43:04 UTC
Git commit 9f997d09d1b5384fb570dda87cf798910d6f0901 by Ralf Habacker.
Committed on 09/11/2018 at 12:41.
Pushed by habacker into branch '4.8-staging'.

Corrections for XIRR implementation to achieve more accurate values

Summary:
In the Investment Report, the XIRR() function is used to calculate
the annual return. The previous implementation contains some errors
that have been corrected with this commit. In addition, if XIRR
cannot calculate the annual return to notify the user of this case,
an empty column is displayed in the investment report.

The basis for this fix is an update of the XIRR implementation from
the KOffice project [1], which brings the calculation up to date and
a further improvement from the libreoffice XIRR implementation [2] to
get more solutions using a two-pass approach. The class CashFlowList
has been moved to separate files to have a cleaner separation.

The obsolete and unused NPV function has been removed.

Tests for XIRR have been extended to check for recognized issues.

[1] https://github.com/KDE/koffice/blob/master/kcells/functions/financial.cpp
[2] https://raw.githubusercontent.com/LibreOffice/core/master/scaddins/source/analysis/financial.cxx
FIXED-IN:4.8.3

Test Plan: compiled and tested on linux

Reviewers: tbaumgart, wojnilowicz

Differential Revision: https://phabricator.kde.org/D16700

M  +1    -0    kmymoney/reports/CMakeLists.txt
A  +162  -0    kmymoney/reports/cashflowlist.cpp     [License: GPL (v2+)]
A  +75   -0    kmymoney/reports/cashflowlist.h     [License: GPL (v2+)]
M  +5    -194  kmymoney/reports/querytable.cpp
M  +0    -87   kmymoney/reports/querytable.h
M  +47   -6    kmymoney/reports/querytabletest.cpp

https://commits.kde.org/kmymoney/9f997d09d1b5384fb570dda87cf798910d6f0901
Comment 9 Ralf Habacker 2018-11-09 13:40:30 UTC
Git commit 633fc9fc619593f9f86b5259b7e93db0c66a700b by Ralf Habacker.
Committed on 09/11/2018 at 13:39.
Pushed by habacker into branch '4.8-staging'.

Corrections for XIRR implementation to achieve more accurate values

Summary:
In the Investment Report, the XIRR() function is used to calculate
the annual return. The previous implementation contains some errors
that have been corrected with this commit. In addition, if XIRR
cannot calculate the annual return to notify the user of this case,
an empty column is displayed in the investment report.

The basis for this fix is an update of the XIRR implementation from
the KOffice project [1], which brings the calculation up to date and
a further improvement from the libreoffice XIRR implementation [2] to
get more solutions using a two-pass approach. The class CashFlowList
has been moved to separate files to have a cleaner separation.

The obsolete and unused NPV function has been removed.

Tests for XIRR have been extended to check for recognized issues.

[1] https://github.com/KDE/koffice/blob/master/kcells/functions/financial.cpp
[2] https://raw.githubusercontent.com/LibreOffice/core/master/scaddins/source/analysis/financial.cxx
FIXED-IN:4.8.3

Test Plan: compiled and tested on linux

Reviewers: tbaumgart, wojnilowicz

Differential Revision: https://phabricator.kde.org/D16700

M  +1    -0    kmymoney/reports/CMakeLists.txt
A  +164  -0    kmymoney/reports/cashflowlist.cpp     [License: GPL (v2+)]
A  +75   -0    kmymoney/reports/cashflowlist.h     [License: GPL (v2+)]
M  +5    -194  kmymoney/reports/querytable.cpp
M  +0    -87   kmymoney/reports/querytable.h
M  +47   -6    kmymoney/reports/querytabletest.cpp

https://commits.kde.org/kmymoney/633fc9fc619593f9f86b5259b7e93db0c66a700b
Comment 10 Ralf Habacker 2018-11-11 18:28:29 UTC
Git commit 4e3943cb7f183877a54ab9b8182762fb502f93ff by Ralf Habacker.
Committed on 11/11/2018 at 18:28.
Pushed by habacker into branch '4.8'.

Corrections for XIRR implementation to achieve more accurate values

In the Investment Report, the XIRR() function is used to calculate
the annual return. The previous implementation contains some errors
that have been corrected with this commit. In addition, if XIRR
cannot calculate the annual return to notify the user of this case,
an empty column is displayed in the investment report.

The basis for this fix is an update of the XIRR implementation from
the KOffice project [1], which brings the calculation up to date and
a further improvement from the libreoffice XIRR implementation [2] to
get more solutions using a two-pass approach. The class CashFlowList
has been moved to separate files to have a cleaner separation.

The obsolete and unused NPV function has been removed.

Tests for XIRR have been extended to check for recognized issues.

[1] https://github.com/KDE/koffice/blob/master/kcells/functions/financial.cpp
[2] https://raw.githubusercontent.com/LibreOffice/core/master/scaddins/source/analysis/financial.cxx
FIXED-IN:4.8.3
Reviewed By: wojnilowicz
Differential Revision: https://phabricator.kde.org/D16700

M  +1    -0    kmymoney/reports/CMakeLists.txt
A  +165  -0    kmymoney/reports/cashflowlist.cpp     [License: GPL (v2+)]
A  +75   -0    kmymoney/reports/cashflowlist.h     [License: GPL (v2+)]
M  +5    -194  kmymoney/reports/querytable.cpp
M  +0    -87   kmymoney/reports/querytable.h
M  +47   -6    kmymoney/reports/querytabletest.cpp

https://commits.kde.org/kmymoney/4e3943cb7f183877a54ab9b8182762fb502f93ff
Comment 11 Ralf Habacker 2018-11-16 11:07:38 UTC
Git commit 2910e97be0d2d1cfd13d7663c77baa19905f9eb3 by Ralf Habacker.
Committed on 15/11/2018 at 22:41.
Pushed by habacker into branch '5.0'.

Corrections for XIRR implementation to achieve more accurate values

Summary:
In the Investment Report, the XIRR() function is used to calculate
the annual return. The previous implementation contains some errors
that have been corrected with this commit. In addition, if XIRR
cannot calculate the annual return to notify the user of this case,
an empty column is displayed in the investment report.

The basis for this fix is an update of the XIRR implementation from
the KOffice project [1], which brings the calculation up to date and
a further improvement from the libreoffice XIRR implementation [2] to
get more solutions using a two-pass approach. The class CashFlowList
has been moved to separate files to have a cleaner separation.

The obsolete and unused NPV function has been removed.

Tests for XIRR have been extended to check for recognized issues.

[1] https://github.com/KDE/koffice/blob/master/kcells/functions/financial.cpp
[2] https://raw.githubusercontent.com/LibreOffice/core/master/scaddins/source/analysis/financial.cxx
FIXED-IN:4.8.3, 5.0.3

Test Plan: compiled and tested on linux

Reviewers: tbaumgart, wojnilowicz

Differential Revision: https://phabricator.kde.org/D16766

M  +1    -0    kmymoney/plugins/views/reports/core/CMakeLists.txt
A  +166  -0    kmymoney/plugins/views/reports/core/cashflowlist.cpp     [License: GPL (v2+)]
A  +75   -0    kmymoney/plugins/views/reports/core/cashflowlist.h     [License: GPL (v2+)]
M  +10   -205  kmymoney/plugins/views/reports/core/querytable.cpp
M  +2    -91   kmymoney/plugins/views/reports/core/querytable.h
M  +47   -6    kmymoney/plugins/views/reports/core/tests/querytable-test.cpp

https://commits.kde.org/kmymoney/2910e97be0d2d1cfd13d7663c77baa19905f9eb3
Comment 12 Ralf Habacker 2018-11-25 10:20:55 UTC
Git commit b87e02c13e8eb027211d0e69862d9c4370653025 by Ralf Habacker.
Committed on 25/11/2018 at 10:19.
Pushed by habacker into branch '5.0'.

Corrections for XIRR implementation to achieve more accurate values

In the Investment Report, the XIRR() function is used to calculate
the annual return. The previous implementation contains some errors
that have been corrected with this commit. In addition, if XIRR
cannot calculate the annual return to notify the user of this case,
an empty column is displayed in the investment report.

The basis for this fix is an update of the XIRR implementation from
the KOffice project [1], which brings the calculation up to date and
a further improvement from the libreoffice XIRR implementation [2] to
get more solutions using a two-pass approach. The class CashFlowList
has been moved to separate files to have a cleaner separation.

The obsolete and unused NPV function has been removed.

Tests for XIRR have been extended to check for recognized issues.

[1] https://github.com/KDE/koffice/blob/master/kcells/functions/financial.cpp
[2] https://raw.githubusercontent.com/LibreOffice/core/master/scaddins/source/analysis/financial.cxx
FIXED-IN:4.8.3, 5.0.3
Reviewed By: tbaumgart
Differential Revision: https://phabricator.kde.org/D16766

M  +1    -0    kmymoney/plugins/views/reports/core/CMakeLists.txt
A  +166  -0    kmymoney/plugins/views/reports/core/cashflowlist.cpp     [License: GPL (v2+)]
A  +75   -0    kmymoney/plugins/views/reports/core/cashflowlist.h     [License: GPL (v2+)]
M  +10   -205  kmymoney/plugins/views/reports/core/querytable.cpp
M  +2    -91   kmymoney/plugins/views/reports/core/querytable.h
M  +47   -6    kmymoney/plugins/views/reports/core/tests/querytable-test.cpp

https://commits.kde.org/kmymoney/b87e02c13e8eb027211d0e69862d9c4370653025