TVM: Fix to the interest rate calculator#146
Merged
Merged
Conversation
14 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
READY FOR REVIEW
This fix was found by client's test case:
Present value: $1,000
Payment per period: -$50
Future value: $0
Number of periods: 60
Compounding frequency: weekly
Payments occur: different
Payment frequency: quarterly
---> our calculator outputs "Invalid result. Please check your inputs"
Claude's explaination.
Fix TVM RATE solver: incorrect Newton-Raphson seed for mismatched payment/compounding frequencies
Problem
The RATE solver returned "Invalid result. Please check your inputs." for valid inputs whenever
paymentFrequencyModewas "different" and the two frequencies were far apart (e.g. weekly compounding with quarterly payments). Confirmed with PV=$1,000, PMT=-$50, FV=$0, n=60 quarters, compounding weekly, payments quarterly. Expected output: 18.32%.Root cause
The Newton-Raphson loop seeds its initial guess with
0.1 / compFreq. Butguessrepresents the rate per payment period, since it's applied directly ton(a count of payment periods). WhencompFreqandpmtFreqdiverge significantly, this seed lands far from the true root, causing the solver to diverge. In this case it landed on a spurious negative root where1 + guess < 0, and the annualization step raises that negative base to a fractional exponent (pmtFreq / compFreq), producingNaN. ThatNaNthen trips the generic invalid-result check.Fix
0.1 / pmtFreq, which matches whatguessactually represents. No behavior change whenpaymentFrequencyModeis "same," sincepmtFreq === compFreqin that case.guessto-0.999999if it ever drops to or below -1 during iteration, to prevent the same NaN failure mode from resurfacing under other extreme inputs.Verification