Skip to content

[FIX] Preserve off-grid incremental rate changes in ParallelogramOLF - #1491

Open
kadyrbekovhamit-cyber wants to merge 4 commits into
casact:mainfrom
kadyrbekovhamit-cyber:codex/fix-parallelogram-offgrid
Open

kadyrbekovhamit-cyber wants to merge 4 commits into
casact:mainfrom
kadyrbekovhamit-cyber:codex/fix-parallelogram-offgrid

Conversation

@kadyrbekovhamit-cyber

@kadyrbekovhamit-cyber kadyrbekovhamit-cyber commented Oct 16, 2026 •

Copy link
Copy Markdown

Summary of Changes

With the default monthly approximation, an incremental rate change dated between month starts is dropped completely. A +20% change on 2001-07-15 produces an on-level factor of 1.0 for the wholly earlier 2000 year, instead of 1.2.

The current incremental branch reindexes changes to month starts using exact matches, losing off-grid dates. This patch compounds the changes on their original dates, then selects the latest effective level at each grid point, matching the as-of convention already used in the cumulative branch. Before the first change the level is 1.

Minimal public-API example (main 90c7b35):

import chainladder as cl
kw = dict(start_date="2000-01-01", end_date="2003-12-31", grain="Y")
print(cl.parallelogram_olf([0.20], ["2001-07-15"], **kw).loc["2000", "OLF"])
# before: 1.0; proposed fix: 1.2

For an entirely old-rate year, current rate / old rate = 1.20 / 1.00 independently gives 1.2. Monthly approximation is still monthly: a July 15 change first affects the August 1 grid point. This patch does not claim exact daily geometry in monthly mode.

The two off-grid tests fail before the source patch. An additional documented-history test pins independent rational references under the explicit monthly and daily conventions: monthly fails on the original base, daily is a passing control. With the fix, the selected suite has 128 passed, 1 skipped:

python -m pytest chainladder/adjustments/tests/test_parallelogram_documented_rate_history.py chainladder/adjustments/tests/test_parallelogram_offgrid.py chainladder/adjustments/tests/test_parallelogram.py chainladder/utils/tests

Tests compare the off-grid input to its next-month grid equivalent and compare incremental/cumulative histories through public ParallelogramOLF for monthly/daily grain and both vertical-line modes. Existing approximation and policy-length tests are retained. The full project suite was not run; dependency warnings remain. Local repository-wide Ruff 0.16.1 lint and format checks pass (198 files). The historical-reference derivation and explicit calendar assumptions are documented in #1491 (comment); the numerical algorithm remains limited to the off-grid fix.

Related GitHub Issue(s)

Closes #1492. Related prior work: #1181 added cumulative histories with as-of matching, but retained exact reindexing for incremental histories. #524 concerns origin-grain/leap-year handling and #686 policy length. This patch concerns off-grid incremental effective dates.

AI/LLM Usage

This submission is initiated at the explicit request of the human account owner. OpenAI Codex assisted with the code, tests, reproduction, prior-art search and this description. The tests above were actually run locally, sequentially on CPU. Independent human code review and human authorship of this form have not been established, so the human-authorship and full-governance-compliance boxes below are intentionally left unchecked. The owner requested submission unchanged after being informed of these requirements. Please advise if this contribution needs a different process; no exemption or human-review claim is implied.

Additional Context for Reviewers

Small synthetic reproduction; no production insurance data or measured financial-loss claim. AI-assisted investigation credited to Xamit Kadirbekov and Daniyal Kadirbekov. Maintainer acceptance and independent review are pending.

Submitter's Checklist

  • I have reviewed and am adhering to the standards outlined in the project Governing Doc.
  • The PR subject title summarizes the changes, with one proper prefix ([FIX], [FEAT], [DOCS], [TST], [CHORE], or [BRK]).
  • I am a human (not a bot), and this PR form is written by a human.

Reviewer's Checklist

  • The implementation addresses the associated issue(s).
  • The implementation is appropriate, maintainable, and follows ARCHITECTURE.md.
  • PR subject title has the proper prefix and the subject is appropriate.
  • Relevant issue(s) are linked.
  • AI/LLM usage is disclosed and appropriate.
  • Documentation and tests are appropriate.
  • CI tests passed, or any failures are acceptable.
  • Leave a comment with the final recommendation (e.g. approve as is, request a secondary review, or flag an area for more review).

@github-actions

github-actions Bot commented Oct 17, 2026 •

Copy link
Copy Markdown

Pyright Type Completeness

View the full pyright --verifytypes output for this commit

Project (full chainladder package, at this PR's head): 16.2% of exported symbols fully typed (242 / 1492)

Known Ambiguous Unknown Total
Project (head) 242 106 1144 1492

Other symbols referenced but not exported by chainladder: 13

Known Ambiguous Unknown Total
Other (head) 3 1 9 13

Symbols without documentation:

  • Functions without docstring: 338
  • Functions without default param: 0
  • Classes without docstring: 8

Patch (exported symbols added or changed by this PR): 75.0% fully typed (6 / 8)

Known Ambiguous Unknown Total
Patch 6 0 2 8
Patch symbol details
Symbol Status Change
chainladder.adjustments.tests.test_parallelogram.L0 ✅ known new
chainladder.adjustments.tests.test_parallelogram.L1 ✅ known new
chainladder.adjustments.tests.test_parallelogram.L2 ✅ known new
chainladder.adjustments.tests.test_parallelogram.L3 ✅ known new
chainladder.adjustments.tests.test_parallelogram.L4 ✅ known new
chainladder.adjustments.tests.test_parallelogram.test_documented_rate_history_independent_geometry ✅ known new
chainladder.adjustments.tests.test_parallelogram.test_off_grid_incremental_change_is_not_dropped ❌ unknown new
chainladder.adjustments.tests.test_parallelogram.test_off_grid_incremental_matches_cumulative ❌ unknown new

@codecov

codecov Bot commented Oct 17, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.36%. Comparing base (90c7b35) to head (671c87e).
⚠️ Report is 34 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1491      +/-   ##
==========================================
+ Coverage   94.34%   94.36%   +0.02%     
==========================================
  Files          96       97       +1     
  Lines        5973     6198     +225     
  Branches      768      798      +30     
==========================================
+ Hits         5635     5849     +214     
- Misses        219      225       +6     
- Partials      119      124       +5     
Flag Coverage Δ
unittests 94.36% <100.00%> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@henrydingliu

Copy link
Copy Markdown
Member

@kadyrbekovhamit-cyber this bug is at least a year old. here is the (presumbly correct) numbers from 0.8.x.

image

the doc build for this PR shows that your fix puts the package much closer but not exact. of course, given the number of fixes that's been attempted on this particular block of code, it's entirely possible that 0.8.x was incorrect. could you please investigate and demonstrate which is the absolute correct number? after which, we should hardocde the correct results into one of these tests to definitively guard against any future drift.

Add fixed rational references for the documented monthly and daily
conventions in response to review on casact#1491. AI assistance disclosed
in the PR; no contribution declarations are changed.
@kadyrbekovhamit-cyber

kadyrbekovhamit-cyber commented Oct 17, 2026 •

Copy link
Copy Markdown
Author

Thanks for pointing out the archived example. I traced the difference to a change in averaging conventions and derived independent references for the current documented model.

The archived rounded results match the older implementation: it first computes current_rate / rate, removes February 29, then averages those factors. That produces [1.140326666729218, 1.104799411052730, 1.084196983861888, 1.098557327828861, 1.034456370801276]. Averaging individual factors is different from dividing the current rate by the average earned rate. For uniformly issued equal exposures with uniform earning over the policy term, the latter is the appropriate denominator.

I derived the reference by integrating earned exposure, using exact rational arithmetic, and independently checked it by integrating the overlap of each policy cohort with each origin year. A rate-level jump Δr at date d contributes

Δr [F(b−d)−F(a−d)] / [T(b−a)],

where F(u) is 0 for u≤0, u²/2 for 0<u<T, and Tu−T²/2 otherwise. The factor is the final rate (1.142757 in this example) divided by the resulting mean earned rate.

For the current equal-month/as-of convention (T=12, July 15 mapped to August 1 and October 31 to November 1), the exact factors for 2016–2020 are:

Year Exact reference Decimal
2016 10284813/9015625 1.140776485268631
2017 934983/846250 1.104854357459380
2018 201663/186040 1.083976564179746
2019 792/721 1.098474341192788
2020 792/767 1.032594524119948

For example, the 2020 share at the new rate is 1−(10/12)²/2=47/72, so its factor is 1.1/(1+0.1×47/72)=792/767. These references agree with the PR, not merely with its rounded display.

Daily resolution gives [1.140296895169280, 1.104040237672231, 1.083933328519156, 1.098415346746896, 1.032374231770578] under the current convention of a 365-day term for non-leap origins and 366 for leap origins. That is a separate explicit convention; neither vector is a universal exact calendar-anniversary result for all policy earning assumptions.

I added test_parallelogram_documented_rate_history.py with hardcoded rational references for both conventions. Against unchanged base 90c7b353, the monthly case fails and the daily control passes. Both pass with this PR; the selected suite (the new reference, off-grid tests, existing parallelogram tests and utils) is 128 passed, 1 skipped. The full project suite was not rerun. This keeps the off-grid fix focused and guards the independently derived values; changing the calendar or exposure convention would need its own explicit decision.

Added regression test (commit ea203263af7a). Repository-wide local Ruff 0.16.1 lint and format checks pass (198 files); commit b44d340b4529 also formats two expressions in the earlier off-grid test, with no executable AST changes. This is local validation, not a claim that the new remote CI run has finished.

AI assistance was used in the investigation, code and writing. The original contribution-process disclosures remain unchanged.

Wrap two long expressions. Executable AST is unchanged; repository-wide
Ruff check and format checks pass (198 files).
@@ -0,0 +1,77 @@
# This Source Code Form is subject to the terms of the Mozilla Public

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you please put this test inside the existing test_parallelogram.py?

(
"M",
[
10284813 / 9015625,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you please also document the elementary expressions, i.e. 1.1/(1+0.1×47/72)?

@@ -0,0 +1,47 @@
import numpy as np

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you please put these tests inside the existing test_parallelogram.py?

Comment thread chainladder/utils/utility_functions.py Resolved
Address review on casact#1491: the off-grid tests and the documented
rate-history reference test now live in the existing
test_parallelogram.py, and the two separate files are removed.

The expected on-level factors of the documented example are written as
elementary parallelogram areas, e.g. 2020 monthly is
L4 / (L3 * _tri(10, 12) + L4 * (1 - _tri(10, 12))) = 1.1/(1+0.1*47/72),
with a comment per year naming which writings are at which rate level
and the exact rational value. No source change.

Co-authored-by: Xamit Kadirbekov <kadyrbekovkhamit@gmail.com>
Co-authored-by: Daniyal Kadirbekov <daniyal.kadirbekov@gmail.com>
Signed-off-by: kadyrbekovhamit-cyber <288885044+kadyrbekovhamit-cyber@users.noreply.github.com>
@kadyrbekovhamit-cyber

Copy link
Copy Markdown
Author

@henrydingliu thanks for the review. Pushed 671c87e:

  1. Tests moved into test_parallelogram.py. The two off-grid tests and the documented rate-history test now live at the end of the existing file. test_parallelogram_offgrid.py and test_parallelogram_documented_rate_history.py are deleted. The tests themselves are the same.
  2. Elementary expressions. The expected values in test_documented_rate_history_independent_geometry are now written as the parallelogram areas, not as bare fractions, e.g. 2020 monthly is L4 / (L3 * _tri(10, 12) + L4 * (1 - _tri(10, 12))), i.e. 1.1/(1+0.1×47/72). Here L0..L4 are the cumulative rate levels and _tri(n, term) = (n/term)**2/2 is the corner triangle. Each year has a one-line comment saying which writings are at which level (e.g. "L0 policies written Jan-Jul 2016 still earn during Jan-Jul 2017"), and the exact fraction is kept as a comment (= 792 / 767). The daily case uses day counts in the same way (e.g. _tri(170, 366) for 2016-07-15..2016-12-31). Each expression matches its fraction to within 1 ulp; the tolerance is unchanged (rtol=1e-13).
  3. side="right". Following your follow-up comment I left that line unchanged. I can add a pointer comment there if you'd still like one.

Local run (Python 3.12, pandas 3.1.0, numpy 2.5.4): test_parallelogram.py + chainladder/utils/tests: 128 passed, 1 skipped. Same selection on the previous head b44d340 with the two separate files: 128 passed, 1 skipped (same tests, new location). With utility_functions.py reverted to 90c7b35, the two off-grid tests and the monthly reference fail; the daily reference passes as a control. Ruff 0.16.1 check and format --check are clean on the changed file. The branch also merges cleanly with current main (cf665f3), and the parallelogram tests pass on that merge (134 passed, 1 skipped); I did not merge main into the branch.

No change to the source fix.

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.

[BUG] Mid-month incremental rate changes disappear from monthly ParallelogramOLF

2 participants