Repository navigation
Conversation
MackChainladder fit on an incremental triangle multiplied incremental amounts in the Mack recursion, so parameter_risk_, process_risk_, full_std_err_ and the standard errors built on them were wrong. fit, predict and full_std_err_ now use the cumulative form of an incremental triangle for these estimates, and summary_ takes the latest diagonal from the estimator, which sums an incremental triangle. X_ and full_triangle_ stay incremental. Closes casact#1571. Developed with an AI assistant (Árbol, running Muse).
Pyright Type CompletenessView the full Project (full
Other symbols referenced but not exported by
Symbols without documentation:
Patch (exported symbols added or changed by this PR): 0.0% fully typed (0 / 3)
Patch symbol details
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1572 +/- ##
==========================================
+ Coverage 94.76% 94.77% +0.01%
==========================================
Files 98 98
Lines 6318 6334 +16
Branches 804 806 +2
==========================================
+ Hits 5987 6003 +16
Misses 212 212
Partials 119 119
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| "mack_std_err_", | ||
| "full_std_err_", | ||
| ] | ||
| for sample in ["raa", "mw2008", "genins"]: |
There was a problem hiding this comment.
is the fixturizable using the existing test fixtures?
There was a problem hiding this comment.
Yes. The test is now a helper called by three tests that take the raa, mw2008 and genins fixtures, so each sample also runs on the sparse backend. mw2008 became a fixture in #1558, after this branch was cut, so I copied main's mw2008 and mw2014 entries into conftest.py unchanged, which keeps the branch mergeable with main without conflicts. If you'd rather I merge main into the branch, those lines drop out of the diff.
(This reply was made with Árbol, an AI assistant, under my direction.)
| raise ValueError("Triangle not compatible with MackChainladder") | ||
| # The Mack recursion multiplies cumulative amounts, so the risk | ||
| # estimates use the cumulative form of an incremental triangle. | ||
| X_cum = self.X_ if self.X_.is_cumulative else self.X_.incr_to_cum() |
There was a problem hiding this comment.
depending on is_cumulative, X_cum could be X_ or a copy of X_. it would be cleaner if that was consistent regardless of is_cumulative.
There was a problem hiding this comment.
Agreed. incr_to_cum() always returns a copy and leaves a cumulative triangle's values unchanged, so fit, predict and full_std_err_ now call it unconditionally and work on the copy. The cached full triangle is set only on that copy, so the del X_cum._full_triangle_ lines are gone.
On cumulative input the change made full_std_err_ build the full triangle twice, because the default in getattr(X, "_full_triangle_", self.full_triangle_) is evaluated even when X has the cache. _get_full_std_err_ now builds the fallback only when the cache is missing. Outputs are bit-identical to the previous commit on raa, mw2008, genins and clrd's CumPaidLoss (numpy and sparse, cumulative and incremental, fit and predict). On the full clrd triangle the extra copy takes 0.22–0.25 ms.
(This reply was made with Árbol, an AI assistant, under my direction.)
fit, predict and full_std_err_ now always take incr_to_cum(), which returns a copy and leaves a cumulative triangle's values unchanged, so the cached full triangle stays on the copy and the del lines are gone. _get_full_std_err_ builds its full_triangle_ fallback only when X has no cached full triangle; getattr evaluated it on every call, which would have doubled full_std_err_'s work on cumulative input. Outputs are bit-identical. The incremental test runs on the raa, mw2008 and genins fixtures, on the numpy and sparse backends. conftest.py gains main's mw2008 and mw2014 entries unchanged, so the branch still merges cleanly with main. Developed with an AI assistant (Árbol, running Muse).
fit, predict and full_std_err_ each cache the full triangle on X before calling _get_full_std_err_, so its fallback to self.full_triangle_ never ran and Codecov counted it as uncovered. Developed with an AI assistant (Árbol, running Muse).
Developed with an AI assistant (Árbol, running Muse).
Summary of Changes
MackChainladderfit on an incremental triangle ran the Mack recursion on incremental amounts, so its standard errors were wrong (mw2008 total: 58,365.28 instead of 108,732.16).fit,predictandfull_std_err_now compute the risk estimates from the cumulative form of an incremental triangle, the same conversion #1558's CDR code uses. The Latest column ofsummary_now comes from the estimator'slatest_diagonal, which sums an incremental triangle; before, it showed the last incremental diagonal.X_andfull_triangle_stay incremental, and results for cumulative input are unchanged.test_mack_incremental_matches_cumulativecompares the risk attributes,total_mack_std_err_andsummary_forfitandpredicton raa, mw2008 and genins.Related GitHub Issue(s)
Closes #1571.
AI/LLM Usage
This change was made with Árbol, an AI assistant running on Muse, working under my direction. It found the bug, wrote the fix and the test, ran the test suite and drafted this description. I reviewed the diff before submitting.
Additional Context for Reviewers
The change merges cleanly with #1558, #1416 and #1304, which also edit
mack.py.Submitter's Checklist
[FIX],[FEAT],[DOCS],[TST],[CHORE], or[BRK]).Reviewer's Checklist