Skip to content

Add Additional Method to XIRR if Newton-Raphson Does Not Converge#3262

Merged
oleibman merged 2 commits intoPHPOffice:masterfrom
oleibman:bisectxirr
Dec 27, 2022
Merged

Add Additional Method to XIRR if Newton-Raphson Does Not Converge#3262
oleibman merged 2 commits intoPHPOffice:masterfrom
oleibman:bisectxirr

Conversation

@oleibman
Copy link
Collaborator

@oleibman oleibman commented Dec 26, 2022

Fix #689. XIRR is calculated by making guesses which are hopefully better with each iteration. It is not guaranteed to succeed for Excel, PhpSpreadsheet, or any other implementation. PhpSpreadsheet uses the Newton-Raphson method for its guesses. So does Python package xirr (https://github.com/tarioch/xirr/), but, if Newton-Raphson fails to converge, Python tries Brent's method as an alternative. Two sets of non-converging data are noted in 689. For both, a solution does converge in Excel. For the first of the problems, a solution converges in Python with Newton-Raphson; but, for the second, a solution converges which requires Brent. For the Java package https://github.com/RayDeCampo/java-xirr on which Python was based, and which uses only Newton-Raphson, a solution converges for the first, and does not converge for the second.

To try to match the good results of the others, I added an alternate algorithm if Newton-Raphson fails. Brent's algorithm seems difficult to implement to me. I might have gone there regardless, but I first tried a slightly simpler alternative, bisection. This solved the problem for both of the cases in 689. Perhaps someone will one day report a problem that doesn't converge for Newton-Raphson or bisection, but does for Brent. We can review this decision then.

The new code causes 3 changes in the unit test. In all 3 tests, Excel and PhpSpreadsheet had not converged, but Python and/or Java had. I now believe that Python/Java is correct in those cases, and Excel is not. The new code aligns PhpSpreadsheet with Python/Java for those tests. It is, of course, impossible to know when Excel's implementation doesn't converge, so we aren't guaranteed to match its results in those hopefully rare situations.

This is:

- [x] a bugfix
- [ ] a new feature
- [ ] refactoring
- [x] additional unit tests

Checklist:

  • Changes are covered by unit tests
    • Changes are covered by existing unit tests
    • New unit tests have been added
  • Code style is respected
  • Commit message explains why the change is made (see https://github.com/erlang/otp/wiki/Writing-good-commit-messages)
  • CHANGELOG.md contains a short summary of the change
  • Documentation is updated as necessary

Why this change is needed?

Provide an explanation of why this change is needed, with links to any Issues (if appropriate).
If this is a bugfix or a new feature, and there are no existing Issues, then please also create an issue that will make it easier to track progress with this PR.

Fix PHPOffice#689. XIRR is calculated by making guesses which are hopefully better with each iteration. It is not guaranteed to succeed for Excel, PhpSpreadsheet, or any other implementation. PhpSpreadsheet uses the Newton-Raphson method for its guesses. So does Python package xirr (https://github.com/tarioch/xirr/), but, if Newton-Raphson fails to converge, Python tries Brent's method as an alternative. Two sets of non-converging data are noted in 689. For both, a solution does converge in Excel. For the first of the problems, a solution converges in Python with Newton-Raphson; but, for the second, a solution converges which requires Brent. For the Java package https://github.com/RayDeCampo/java-xirr on which Python was based, and which uses only Newton-Raphson, a solution converges for the first, and does not converge for the second.

To try to match the good results of the others, I added an alternate algorithm if Newton-Raphson fails. Brent's algorithm seems difficult to implement to me. I might have gone there regardless, but I first tried a slightly simpler alternative, bisection. This solved the problem for both of the cases in 689. Perhaps someone will one day report a problem that doesn't converge for Newton-Raphson or bisection, but does for Brent. We can review this decision then.

The new code causes 3 changes in the unit test. In all 3 tests, Excel and PhpSpreadsheet had not converged, but Python and/or Java had. I now believe that Python/Java is correct in those cases, and Excel is not. The new code aligns PhpSpreadsheet with Python/Java for those tests. It is, of course, impossible to know when Excel's implementation doesn't converge, so we aren't guaranteed to match its results in those hopefully rare situations.
@oleibman
Copy link
Collaborator Author

Not concerned about Scrutinizer "complexity" complaint.

@oleibman oleibman merged commit ec96ef3 into PHPOffice:master Dec 27, 2022
@oleibman oleibman deleted the bisectxirr branch February 10, 2023 05:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

XIRR returns Non Numeric value Encountered

1 participant