Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix mirr function #733

Open
wants to merge 1 commit into
base: trunk
Choose a base branch
from

Conversation

alexjansons
Copy link

Summary

This PR resolves 2 issues in the MIRR function implementation:

  • formula calculation issue
  • input parameter order

Formula Calculation Issue

The current MIRR implementation produces inaccurate results due to an error in the present value calculation.
Let's take an example from Wikipedia: Modified internal rate of return

Input data:

Cash flow values: -1000, -4000, 5000, 2000
Finance rate: 10%
Reinvestment rate: 12%

Calculation result:

  • Output from current MIRR implementation: 20.57%
  • Expected output based on Wikipedia example: 17.91%

The discrepancy appears due to including finance and reinvestment rates in the present value calculation. Only the finance rate should be considered.
Here is a related code fragment:

if (anIn < 0) {
   pv += anIn / Math.pow(1 + financeRate + reinvestRate, indexN++);
}

Input Parameter Order

The current implementation uses a non-standard order for finance and reinvestment rates, unlike common spreadsheet applications (Excel, Google Sheets).
Current order: (values, reinvestRate, financeRate).
Proposed order: (values, financeRate, reinvestRate).

I.e. to calculate the MIRR for the formula mentioned above one have pass the following values to Mirr#evaluate(double[] values) function:
-1000D, -4000D, 5000D, 2000D, 0.12D, 0.1D
however more the order according to most popular spreadsheet applications would be:
-1000D, -4000D, 5000D, 2000D, 0.1D, 0.12D

Testing

A test from Wikipedia has been added to verify the formula accuracy.
All other tests are left intact except for parameter order.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant