[jayzed80]

Fixes #3959

Root cause

All four odd-period functions take the quasi-coupon period's normal length from normal_length
(crates/functions/src/financial/securities.rs:321). It returns 360/frequency for basis 2 and
365/frequency for basis 3. Excel's values in the issue show two things this gets wrong:

• ODDLPRICE / ODDLYIELD (odd_last, securities.rs:464 on main): Excel measures each
  quasi-coupon period of an odd last period by its actual length under bases 2 and 3. The issue's
  basis-2 example gives exactly the basis-1 price (100.157877308959). With NL = 360, GridCraft
  gave 100.160017355362.
• ODDFPRICE / ODDFYIELD (odd_first, securities.rs:371 on main): Excel keeps NL =
  360/frequency (the COUPDAYS value) for a long first period. But it counts every whole
  quasi-coupon period after the one holding the issue date as NL days, which is exactly one
  period. GridCraft counted actual days instead (181/180, 184/180, …). Solving the issue's example
  for the single unknown Σ DCi/NLi gives 3.00555555555547, which is (180 + 180 + 181) / 180 to
  within 1e-13. The accrued-days sum and DSC/E stay as they were. With this rule the price
  matches Excel's 102.126937578431.

Fix

Two narrow changes in securities.rs, both limited to bases 2 and 3:

• odd_last: NL is the quasi-period's actual length (end - start).
• odd_first: a quasi-period that starts after the issue date counts as NL days in Σ DCi/NLi.
  The period that holds the issue date still counts its actual days from issue.

Bases 0, 1 and 4 are unchanged. For basis 1 both rules are already true, so its results stay the
same, and the test checks the basis-1 control value.

Tests

New test financial::tests::odd_periods_on_actual_day_bases_follow_excel
(crates/functions/src/financial/tests.rs):

• ODDLPRICE of the issue's example under bases 1, 2 and 3 = 100.157877308959 (Excel)
• ODDFPRICE of the issue's long-first example under basis 2 = 102.126937578431 (Excel), and the
  basis-1 control stays at 102.194911009281
• ODDFYIELD and ODDLYIELD give back the 4% yield from the corrected prices

Before the fix (on the branch made from upstream/main, test added, code unchanged):

test financial::tests::odd_periods_on_actual_day_bases_follow_excel ... FAILED
got 100.16001735536176, expected 100.157877308959

With only the ODDFPRICE change taken out (the ODDLPRICE change still in):

got 102.19524544133458, expected 102.126937578431
test result: FAILED. 0 passed; 1 failed

With the fix:

test financial::tests::odd_periods_on_actual_day_bases_follow_excel ... ok
test financial::tests::odd_periods ... ok
test financial::tests::odd_periods_survive_hostile_input ... ok

Checks run (Windows)

• cargo fmt -p gridcraft-functions: no changes outside the edited lines
• cargo clippy -j 4 -p gridcraft-functions --all-targets -- -D warnings: clean
• cargo test -j 4 -p gridcraft-functions: 181 passed, 1 failed. The failure is
  engineering::tests::complex_numbers, which expects ...+0.707106781186547i and gets
  ...+0.707106781186548i. That test only exercises complex-number text, which this change does
  not touch. It is most likely a last-digit difference in the Windows maths library. I did not
  re-run it on unmodified main.

Not run

• Full cargo xtask ci (wasm, layers, assets) and other crates' tests
• The issue mentions a 210-formula matrix that also covers basis 3 and quarterly frequencies.
  I only had Excel values for the two basis-2 examples. The basis-3 behaviour follows the same
  rule but has no Excel value to check against: the ODDLPRICE basis-3 result is only tested to
  equal basis 1. Odd-first periods whose issue date falls inside a quasi-period, under bases 2 and
  3, are also inferred rather than checked against Excel.

[coderabbitai]

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 41 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 8 included reviews currently available.

Learn how review limits work.

Review configuration:

Run configuration

• Source mode: Auto
• Repository: storytold/gridcraft
• Branch: main
• File: .coderabbit.yaml
• Commit: c0ee120
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: 4b7b9ec3-2822-469a-89f3-5fd8db04d2a0

Commits

Reviewing files that changed from the base of the PR and between c0ee120 and 53e0143.

Files selected for processing: 2

• crates/functions/src/financial/securities.rs
• crates/functions/src/financial/tests.rs

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

• X
• Mastodon
• Reddit
• LinkedIn