numpy / numpy-financial

Standalone package of the NumPy financial functions
BSD 3-Clause "New" or "Revised" License
338 stars 80 forks source link

BUG: npf.pmt() algorithm future value sign is flipped. #130

Open RlndVt opened 4 weeks ago

RlndVt commented 4 weeks ago

Describe the issue:

When calculating the payment required to end with a remaining open balance, aka with a final_value, the calculated payments are larger than the payments required for a achieving final_value=0.

Workaround is inputting the final_value in a flipped state.

In the current state, if a positive pv means a outstanding loan, a negative fv means a remaining outstanding debt; whilst a positive fv implies having 'overpaid' the loan. This is confusing.

Either the algorithm is wrong, and the sign should be flipped; or the documentation should be updated to clarify that pv and fv signs have opposite meaning.

Reproduce the code example:

import numpy_financial as npf

interest_rate = 0.05 / 12
principal_loan = 200_000
compounding_periods = 360
payment_value_fv0 = npf.pmt(
    interest_rate,
    compounding_periods,
    principal_loan,
    0,
)
payment_value_fv50 = npf.pmt(
    interest_rate,
    compounding_periods,
    principal_loan,
    50_000,
)
payment_value_fvn50 = npf.pmt(
    interest_rate,
    compounding_periods,
    principal_loan,
    -50_000,
)
print("{:.2f}".format(payment_value_fv50))
print("{:.2f}".format(payment_value_fv0))
print("{:.2f}".format(payment_value_fvn50))

assert abs(payment_value_fv50) < abs(payment_value_fv0),\
       "Payment with remaining balance should be lower than fully paying off loan, but found {:.2f} > {:.2f}".format(abs(payment_value_fv50),abs(payment_value_fv0))

Error message:

-1133.72
-1073.64
-1013.57
Traceback (most recent call last):
  File "[...]/debugPmt.py", line 28, in <module>
    assert abs(payment_value_fv50) < abs(payment_value_fv0),\
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: Payment with remaining balance should be lower than fully paying off loan, but found 1133.72 > 1073.64

Runtime information:

2.1.1 3.12.6 (main, Sep 9 2024, 00:00:00) [GCC 14.2.1 20240801 (Red Hat 14.2.1-1)] [{'numpy_version': '2.1.1', 'python': '3.12.6 (main, Sep 9 2024, 00:00:00) [GCC 14.2.1 20240801 (Red ' 'Hat 14.2.1-1)]', 'uname': uname_result(system='Linux', node='[...]', release='6.10.9-200.fc40.x86_64', version='#1 SMP PREEMPT_DYNAMIC Sun Sep 8 17:23:55 UTC 2024', machine='x86_64')}, {'simd_extensions': {'baseline': ['SSE', 'SSE2', 'SSE3'], 'found': ['SSSE3', 'SSE41', 'POPCNT', 'SSE42', 'AVX', 'F16C', 'FMA3', 'AVX2'], 'not_found': ['AVX512F', 'AVX512CD', 'AVX512_KNL', 'AVX512_KNM', 'AVX512_SKX', 'AVX512_CLX', 'AVX512_CNL', 'AVX512_ICL']}}, {'architecture': 'Haswell', 'filepath': '/home/[...]/.local/lib/python3.12/site-packages/numpy.libs/libscipyopenblas64-ff651d7f.so', 'internal_api': 'openblas', 'num_threads': 8, 'prefix': 'libscipy_openblas', 'threading_layer': 'pthreads', 'user_api': 'blas', 'version': '0.3.27'}] None

Context for the issue:

No response

RlndVt commented 4 weeks ago

FWIW libreoffice calc and MS Excel show the same behaviour. To stay in line with those implementations, updating the documentation may be the better solution.

I still find it a confusing configuration and would argue that having the PV and FV signs represent the same is more correct, but I can understand the argument that keeping the implementation inline with other is desirable.


Examining Calc's and Excel's documentation; Calc:

PV is [...] the present value of the amount borrowed or invested.

FV is [...] the future value of the cash balance desired at the end of the term.

Excel:

Pv The present value, or the total amount that a series of future payments is worth now; also known as the principal.

Fv The future value, or a cash balance you want to attain after the last payment is made.

A distinction is made between the two types of 'value'. PV describing the borrowed/invested amount, and FV the final cash balance. Again, I would argue confusing; but nonetheless correct distinction.