Skip to content

Commit 95d9cc9

Browse files
oleibmanoleibman
authored
Refinement for XIRR (#2487)
* Refinement for XIRR Fix #2469. The algorithm used for XIRR is known not to converge in some cases, some of which are because the value is legitimately unsolvable; for others, using a different guess might help. The algorithm uses continual guesses at a rate to hopefully converge on the solution. The code in Python package xirr (https://github.com/tarioch/xirr/) suggests a refinement when this rate falls below -1. Adopting this refinement solves the problem for the data in issue 2469 without any adverse effect on the existing tests. My thanks to @tarioch for that refinement. The data from 2469 is, of course, added to the test cases. The user also mentions that an initial guess equal to the actual result doesn't converge either. A test is also added to confirm that that case now works. The test cases are changed to run in the context of a spreadsheet rather than by direct calls to XIRR calculation routine. This revealed some data validation errors which are also cleaned up with this PR. This suggests that other financial tests might benefit from the same change; I will look into that. * More Unit Tests From https://github.com/RayDeCampo/java-xirr/blob/master/src/test/java/org/decampo/xirr/XirrTest.java https://github.com/tarioch/xirr/blob/master/tests/test_math.py Note that there are some cases where the PHP tests do not converge, but the non-PHP tests do. I have confirmed in each of those cases that Excel does not converge, so the PhpSpreadsheet results are good, at least for now. The discrepancies are noted in comments in the test member.
1 parent 1509097 commit 95d9cc9

File tree

5 files changed

+310
-39
lines changed

5 files changed

+310
-39
lines changed

‎phpstan-baseline.neon

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -930,11 +930,6 @@ parameters:
930930
count: 1
931931
path: src/PhpSpreadsheet/Calculation/Financial/CashFlow/Constant/Periodic/InterestAndPrincipal.php
932932

933-
-
934-
message: "#^Parameter \\#4 \\$x2 of static method PhpOffice\\\\PhpSpreadsheet\\\\Calculation\\\\Financial\\\\CashFlow\\\\Variable\\\\NonPeriodic\\:\\:xirrPart3\\(\\) expects float, mixed given\\.$#"
935-
count: 1
936-
path: src/PhpSpreadsheet/Calculation/Financial/CashFlow/Variable/NonPeriodic.php
937-
938933
-
939934
message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Calculation\\\\Financial\\\\CashFlow\\\\Variable\\\\Periodic\\:\\:presentValue\\(\\) has parameter \\$args with no type specified\\.$#"
940935
count: 1
@@ -7915,11 +7910,6 @@ parameters:
79157910
count: 1
79167911
path: tests/PhpSpreadsheetTests/Calculation/Functions/Financial/XNpvTest.php
79177912

7918-
-
7919-
message: "#^Parameter \\#3 \\$message of static method PHPUnit\\\\Framework\\\\Assert\\:\\:assertEquals\\(\\) expects string, mixed given\\.$#"
7920-
count: 1
7921-
path: tests/PhpSpreadsheetTests/Calculation/Functions/Financial/XirrTest.php
7922-
79237913
-
79247914
message: "#^Part \\$number \\(mixed\\) of encapsed string cannot be cast to string\\.$#"
79257915
count: 1

‎src/PhpSpreadsheet/Calculation/Financial/CashFlow/Variable/NonPeriodic.php

Lines changed: 24 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ class NonPeriodic
1212

1313
const FINANCIAL_PRECISION = 1.0e-08;
1414

15+
const DEFAULT_GUESS = 0.1;
16+
1517
/**
1618
* XIRR.
1719
*
@@ -25,21 +27,25 @@ class NonPeriodic
2527
* @param mixed[] $dates A series of payment dates
2628
* The first payment date indicates the beginning of the schedule of payments
2729
* All other dates must be later than this date, but they may occur in any order
28-
* @param float $guess An optional guess at the expected answer
30+
* @param mixed $guess An optional guess at the expected answer
2931
*
3032
* @return float|string
3133
*/
32-
public static function rate($values, $dates, $guess = 0.1)
34+
public static function rate($values, $dates, $guess = self::DEFAULT_GUESS)
3335
{
3436
$rslt = self::xirrPart1($values, $dates);
3537
if ($rslt !== '') {
3638
return $rslt;
3739
}
3840

3941
// create an initial range, with a root somewhere between 0 and guess
40-
$guess = Functions::flattenSingleValue($guess);
42+
$guess = Functions::flattenSingleValue($guess) ?? self::DEFAULT_GUESS;
43+
if (!is_numeric($guess)) {
44+
return Functions::VALUE();
45+
}
46+
$guess = ($guess + 0.0) ?: self::DEFAULT_GUESS;
4147
$x1 = 0.0;
42-
$x2 = $guess ?: 0.1;
48+
$x2 = $guess + 0.0;
4349
$f1 = self::xnpvOrdered($x1, $values, $dates, false);
4450
$f2 = self::xnpvOrdered($x2, $values, $dates, false);
4551
$found = false;
@@ -54,9 +60,11 @@ public static function rate($values, $dates, $guess = 0.1)
5460

5561
break;
5662
} elseif (abs($f1) < abs($f2)) {
57-
$f1 = self::xnpvOrdered($x1 += 1.6 * ($x1 - $x2), $values, $dates, false);
63+
$x1 += 1.6 * ($x1 - $x2);
64+
$f1 = self::xnpvOrdered($x1, $values, $dates, false);
5865
} else {
59-
$f2 = self::xnpvOrdered($x2 += 1.6 * ($x2 - $x1), $values, $dates, false);
66+
$x2 += 1.6 * ($x2 - $x1);
67+
$f2 = self::xnpvOrdered($x2, $values, $dates, false);
6068
}
6169
}
6270
if (!$found) {
@@ -104,11 +112,13 @@ private static function bothNegAndPos(bool $neg, bool $pos): bool
104112
*/
105113
private static function xirrPart1(&$values, &$dates): string
106114
{
107-
if (!is_array($values) && !is_array($dates)) {
108-
return Functions::NA();
109-
}
110115
$values = Functions::flattenArray($values);
111116
$dates = Functions::flattenArray($dates);
117+
$valuesIsArray = count($values) > 1;
118+
$datesIsArray = count($dates) > 1;
119+
if (!$valuesIsArray && !$datesIsArray) {
120+
return Functions::NA();
121+
}
112122
if (count($values) != count($dates)) {
113123
return Functions::NAN();
114124
}
@@ -219,7 +229,11 @@ private static function xnpvOrdered($rate, $values, $dates, bool $ordered = true
219229
if (!is_numeric($dif)) {
220230
return $dif;
221231
}
222-
$xnpv += $values[$i] / (1 + $rate) ** ($dif / 365);
232+
if ($rate <= -1.0) {
233+
$xnpv += -abs($values[$i]) / (-1 - $rate) ** ($dif / 365);
234+
} else {
235+
$xnpv += $values[$i] / (1 + $rate) ** ($dif / 365);
236+
}
223237
}
224238

225239
return is_finite($xnpv) ? $xnpv : Functions::VALUE();

‎tests/PhpSpreadsheetTests/Calculation/Functions/Financial/AllSetupTeardown.php

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,116 @@
1+
<?php
2+
3+
namespace PhpOffice\PhpSpreadsheetTests\Calculation\Functions\Financial;
4+
5+
use PhpOffice\PhpSpreadsheet\Calculation\Exception as CalcException;
6+
use PhpOffice\PhpSpreadsheet\Calculation\Functions;
7+
use PhpOffice\PhpSpreadsheet\Cell\DataType;
8+
use PhpOffice\PhpSpreadsheet\Spreadsheet;
9+
use PhpOffice\PhpSpreadsheet\Worksheet\Worksheet;
10+
use PHPUnit\Framework\TestCase;
11+
12+
class AllSetupTeardown extends TestCase
13+
{
14+
/**
15+
* @var string
16+
*/
17+
private $compatibilityMode;
18+
19+
/**
20+
* @var ?Spreadsheet
21+
*/
22+
private $spreadsheet;
23+
24+
/**
25+
* @var ?Worksheet
26+
*/
27+
private $sheet;
28+
29+
protected function setUp(): void
30+
{
31+
$this->compatibilityMode = Functions::getCompatibilityMode();
32+
}
33+
34+
protected function tearDown(): void
35+
{
36+
Functions::setCompatibilityMode($this->compatibilityMode);
37+
$this->sheet = null;
38+
if ($this->spreadsheet !== null) {
39+
$this->spreadsheet->disconnectWorksheets();
40+
$this->spreadsheet = null;
41+
}
42+
}
43+
44+
protected static function setOpenOffice(): void
45+
{
46+
Functions::setCompatibilityMode(Functions::COMPATIBILITY_OPENOFFICE);
47+
}
48+
49+
protected static function setGnumeric(): void
50+
{
51+
Functions::setCompatibilityMode(Functions::COMPATIBILITY_GNUMERIC);
52+
}
53+
54+
/**
55+
* @param mixed $expectedResult
56+
*/
57+
protected function mightHaveException($expectedResult): void
58+
{
59+
if ($expectedResult === 'exception') {
60+
$this->expectException(CalcException::class);
61+
}
62+
}
63+
64+
/**
65+
* @param mixed $value
66+
*/
67+
protected function setCell(string $cell, $value): void
68+
{
69+
if ($value !== null) {
70+
if (is_string($value) && is_numeric($value)) {
71+
$this->getSheet()->getCell($cell)->setValueExplicit($value, DataType::TYPE_STRING);
72+
} else {
73+
$this->getSheet()->getCell($cell)->setValue($value);
74+
}
75+
}
76+
}
77+
78+
protected function getSpreadsheet(): Spreadsheet
79+
{
80+
if ($this->spreadsheet !== null) {
81+
return $this->spreadsheet;
82+
}
83+
$this->spreadsheet = new Spreadsheet();
84+
85+
return $this->spreadsheet;
86+
}
87+
88+
protected function getSheet(): Worksheet
89+
{
90+
if ($this->sheet !== null) {
91+
return $this->sheet;
92+
}
93+
$this->sheet = $this->getSpreadsheet()->getActiveSheet();
94+
95+
return $this->sheet;
96+
}
97+
98+
/**
99+
* Adjust result if it is close enough to expected by ratio
100+
* rather than offset.
101+
*
102+
* @param mixed $result
103+
* @param mixed $expectedResult
104+
*/
105+
protected function adjustResult(&$result, $expectedResult): void
106+
{
107+
if (is_numeric($result) && is_numeric($expectedResult)) {
108+
if ($expectedResult != 0) {
109+
$frac = $result / $expectedResult;
110+
if ($frac > 0.999999 && $frac < 1.000001) {
111+
$result = $expectedResult;
112+
}
113+
}
114+
}
115+
}
116+
}

‎tests/PhpSpreadsheetTests/Calculation/Functions/Financial/XirrTest.php

Lines changed: 44 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -2,35 +2,60 @@
22

33
namespace PhpOffice\PhpSpreadsheetTests\Calculation\Functions\Financial;
44

5-
use PhpOffice\PhpSpreadsheet\Calculation\Financial;
6-
use PhpOffice\PhpSpreadsheet\Calculation\Functions;
7-
use PHPUnit\Framework\TestCase;
8-
9-
class XirrTest extends TestCase
5+
class XirrTest extends AllSetupTeardown
106
{
11-
protected function setUp(): void
12-
{
13-
Functions::setCompatibilityMode(Functions::COMPATIBILITY_EXCEL);
14-
}
15-
167
/**
178
* @dataProvider providerXIRR
189
*
1910
* @param mixed $expectedResult
20-
* @param mixed $message
11+
* @param string $message
12+
* @param mixed $values
13+
* @param mixed $dates
14+
* @param mixed $guess
2115
*/
22-
public function testXIRR($expectedResult, $message, ...$args): void
16+
public function testXIRR($expectedResult, $message, $values = null, $dates = null, $guess = null): void
2317
{
24-
$result = Financial::XIRR(...$args);
25-
if (is_numeric($result) && is_numeric($expectedResult)) {
26-
if ($expectedResult != 0) {
27-
$frac = $result / $expectedResult;
28-
if ($frac > 0.999999 && $frac < 1.000001) {
29-
$result = $expectedResult;
18+
$this->mightHaveException($expectedResult);
19+
$sheet = $this->getSheet();
20+
$formula = '=XIRR(';
21+
if ($values !== null) {
22+
if (is_array($values)) {
23+
$row = 0;
24+
foreach ($values as $value) {
25+
++$row;
26+
$sheet->getCell("A$row")->setValue($value);
27+
}
28+
$formula .= "A1:A$row";
29+
} else {
30+
$sheet->getCell('A1')->setValue($values);
31+
$formula .= 'A1';
32+
}
33+
if ($dates !== null) {
34+
if (is_array($dates)) {
35+
$row = 0;
36+
foreach ($dates as $date) {
37+
++$row;
38+
$sheet->getCell("B$row")->setValue($date);
39+
}
40+
$formula .= ",B1:B$row";
41+
} else {
42+
$sheet->getCell('B1')->setValue($dates);
43+
$formula .= ',B1';
44+
}
45+
if ($guess !== null) {
46+
if ($guess !== 'C1') {
47+
$sheet->getCell('C1')->setValue($guess);
48+
}
49+
$formula .= ', C1';
3050
}
3151
}
3252
}
33-
self::assertEquals($expectedResult, $result, $message);
53+
$formula .= ')';
54+
$sheet->getCell('D1')->setValue($formula);
55+
$result = $sheet->getCell('D1')->getCalculatedValue();
56+
$this->adjustResult($result, $expectedResult);
57+
58+
self::assertEqualsWithDelta($expectedResult, $result, 0.1E-7, $message);
3459
}
3560

3661
public function providerXIRR(): array

0 commit comments

Comments
 (0)