falhenaki opened a new pull request, #1216: URL: https://github.com/apache/poi/pull/1216
Follow-up to [bug 64137](https://bz.apache.org/bugzilla/show_bug.cgi?id=64137), which I reported back in 2020. The fix at the time raised `MAX_ITERATION_COUNT` from 20 to 1000, and that was enough for the cash flow in the report (from the default guess it overshoots to -0.992, right next to the -100% singularity, then needs about 225 iterations to crawl back out). But the underlying issue is still there: `Irr.irr()` is an unguarded Newton-Raphson iteration, and when it diverges you still get wrong answers today. I ran into two ways this happens: 1. It can converge to a rate below -100%. Roots of the NPV polynomial at rates <= -1 are financially meaningless and Excel never returns them, but Newton-Raphson happily lands on one. For example `Irr.irr(new double[]{-2, 1, 1}, -1.4)` currently returns -1.5, i.e. -150%. The correct IRR for that stream is 0%. 2. It can return NaN even though a perfectly ordinary root exists. A far-off guess throws the iterate way outside the domain, where the shared power-of-(1+x) denominator overflows and the computed derivative collapses to zero. `Irr.irr(new double[]{-1000, 0,0,0,0,0,0,0,0, 6000}, 9.0)` gives NaN after two iterations; the actual IRR is about 22.03%. In a sweep of 20,000 random cash flows the current code fails to find an existing, numerically verifiable root in roughly 1,900 cases. The change keeps the existing Newton-Raphson loop as the first attempt, completely unchanged, so every cash flow it already solves keeps its exact current result. Only when that loop returns NaN or a rate <= -1 does the code fall back to a bracketed Newton-Raphson (the classic "rtsafe" safeguard): first find a sign change of NPV on a fixed grid over (-1, 10000], dense near -1 where the troublesome roots sit, then take Newton steps only while they stay inside the bracket and keep shrinking it fast enough, bisecting otherwise. That cannot diverge and cannot leave the domain. The fallback computes NPV via powers of 1/(1+x), so rates right next to -1 overflow to +/-infinity (still fine for sign tests) instead of underflowing a shared denominator to zero. One small behavioural note: the "Returning NaN" warn logs moved from the main loop into the fallback, since a NaN from the first stage is no longer a final answer. Testing: the existing `TestIrr` suite passes untouched, including the exact-value `bug64137()` test, which confirms the fast path really is byte-for-byte the old algorithm. I added tests for the two failure modes above and for cash flows with no valid IRR at all (those still return NaN). I also compared old vs new over the 20,000-case random sweep: no changed results where the current code succeeds, no new failures, and the ~1,900 previously failing cases now solve. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
