Skip to content

Fix convertible dividend indexing after settlement - #2702

Open
wenn-id wants to merge 1 commit into
lballabio:masterfrom
wenn-id:fix/convertible-dividend-indexing
Open

Fix convertible dividend indexing after settlement#2702
wenn-id wants to merge 1 commit into
lballabio:masterfrom
wenn-id:fix/convertible-dividend-indexing

Conversation

@wenn-id

@wenn-id wenn-id commented Jul 31, 2026

Copy link
Copy Markdown

Summary

  • populate dividendValues_ from the already settlement-filtered dividend containers
  • keep the value and date indices aligned with the destination array
  • add a regression test with one dividend before settlement and one after settlement

This addresses Issue A in #2701. Issue B is intentionally out of scope because it requires a separate design decision about evaluation-date versus settlement-date spot adjustment.

Root cause

dividendValues_ was sized using dividends_, which excludes dividends that occurred before the bond settlement date, but populated by iterating the original unfiltered dividends schedule. Once an earlier dividend was filtered out, the raw index no longer matched the destination array and could write out of bounds.

TDD evidence

Before the production change, the regression test failed because the single retained slot contained the discounted pre-settlement dividend instead of the future dividend:

dividendValues()[0] = 0.99986112075572642
expected            = 9.4977726689391169

After the fix:

  • focused regression test: passed (1/1)
  • complete ConvertibleBondTests suite: passed (4/4)
  • Debug CMake/Ninja build: passed
  • git diff --check: passed
  • independent read-only review: APPROVE, no Critical/Important/Minor findings

A repository-wide Release build and test run also passed: 1,374/1,374 test cases and 12,423/12,423 assertions in 4 minutes 44 seconds.

Scope

Only these files change:

  • ql/pricingengines/bond/discretizedconvertible.cpp
  • test-suite/convertiblebonds.cpp

AI assistance

AI assistance was used to help draft the initial regression scenario and perform an independent review. The initial test was manually rejected because it mixed Issue B pricing semantics into Issue A; the final test was rewritten to inspect DiscretizedConvertible::dividendValues() directly. All changes and test results were reviewed and executed locally by the contributor.

@boring-cyborg

boring-cyborg Bot commented Jul 31, 2026

Copy link
Copy Markdown

Thanks for opening this pull request! It might take a while before we look at it, so don't worry if there seems to be no feedback. We'll get to it.

@CLAassistant

CLAassistant commented Jul 31, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@lballabio

Copy link
Copy Markdown
Owner

Thanks! A suggestion: if you want to run the test suite locally, compile it in Release mode. As you've seen, Debug mode is a lot slower, to the point of being unusable.

@wenn-id

wenn-id commented Jul 31, 2026

Copy link
Copy Markdown
Author

Thanks for the tip. I rebuilt the test suite in Release mode and reran it locally:

Tests completed in 4 m 44 s
1374/1374 test cases passed
12423/12423 assertions passed

That is indeed much more practical than the Debug run.

@coveralls

Copy link
Copy Markdown

Coverage Status

coverage: 74.922% (-0.001%) from 74.923% — wenn-id:fix/convertible-dividend-indexing into lballabio:master

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.

4 participants