Skip to content

[FIX] Guard zero case reserve - #1557

Open
priyam0k wants to merge 3 commits into
mainfrom
fix/issue-1509-zero-case-reserve
Open

priyam0k wants to merge 3 commits into
mainfrom
fix/issue-1509-zero-case-reserve

Conversation

@priyam0k

@priyam0k priyam0k commented Nov 9, 2026 •

Copy link
Copy Markdown
Contributor

Summary of Changes

  • Wrapped the prior case denominator in CaseOutstanding.case_to_prior_case_ and paid_to_prior_case_ with num_to_nan() to guard against divide-by-zero when carried case reserves run off to zero.
  • Replaced duplicate .mean(2) calls in fit() with direct mean(axis=2) assignment, defaulting residual case_ldf_ NaNs to 1.0 and paid_ldf_ NaNs to 0.0.
  • Expanded the empty / all-NaN slice RuntimeWarning filter in TrianglePandas.agg_func across all axes so triangle aggregations match pandas behavior.
  • Added unit tests in test_outstanding.py covering tail runoff on friedland_xyz_auto_bi and zero prior case reserves under strict warning checks.

Related GitHub Issue(s)

Closes #1509

AI/LLM Usage

Used basic model for drafting initial unit test assertions and lint checks.

Additional Context for Reviewers

Defaulting residual case_ldf_ NaNs to 1.0 matches existing behavior in _set_ldf() and follows conventions in Development and MunichAdjustment.

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).

Note

Medium Risk
Changes reserve-ratio math and default LDF fill behavior in CaseOutstanding fit(), which can shift projections when case reserves hit zero; scope is limited to that estimator plus new tests.

Overview
CaseOutstanding now treats zero prior case reserves as invalid denominators by passing prior-case values through num_to_nan() in case_to_prior_case_ and paid_to_prior_case_, avoiding divide-by-zero when case has fully runoff.

During fit(), pattern means are taken directly from weighted, NaN-masked triangles (mean(axis=2)), with case_ldf_ NaNs filled to 1.0 and paid_ldf_ NaNs filled to 0 via fillzero(). RuntimeWarnings from empty or all-NaN slices while aggregating are suppressed in that block.

Tests assert fitting on real and synthetic triangles with zero case reserves completes without RuntimeWarning and yields non-NaN case_ldf_ / valid paid_ldf_.

Reviewed by Cursor Bugbot for commit 841a1ac. Bugbot is set up for automated code reviews on this repo. Configure here.

@github-actions

github-actions Bot commented Nov 9, 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.7% of exported symbols fully typed (265 / 1588)

Known Ambiguous Unknown Total
Project (head) 265 101 1222 1588

Other symbols referenced but not exported by chainladder: 24

Known Ambiguous Unknown Total
Other (head) 11 1 12 24

Symbols without documentation:

  • Functions without docstring: 328
  • Functions without default param: 0
  • Classes without docstring: 10

Patch (exported symbols added or changed by this PR): 0.0% fully typed (0 / 2)

Known Ambiguous Unknown Total
Patch 0 0 2 2
Patch symbol details
Symbol Status Change
chainladder.development.tests.test_outstanding.test_case_outstanding_synthetic_zero_prior_case_warning ❌ unknown new
chainladder.development.tests.test_outstanding.test_case_outstanding_zero_case_reserve_warning ❌ unknown new

@codecov

codecov Bot commented Nov 9, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.59%. Comparing base (8e2e53c) to head (841a1ac).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1557   +/-   ##
=======================================
  Coverage   94.59%   94.59%           
=======================================
  Files          98       98           
  Lines        6233     6237    +4     
  Branches      799      799           
=======================================
+ Hits         5896     5900    +4     
  Misses        216      216           
  Partials      121      121           
Flag Coverage Δ
unittests 94.59% <100.00%> (+<0.01%) ⬆️

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.

@priyam0k
priyam0k marked this pull request as ready for review November 9, 2026 16:05
@henrydingliu

Copy link
Copy Markdown
Member

converting to draft due to branch conflict

@henrydingliu
henrydingliu marked this pull request as draft November 9, 2026 17:48
@priyam0k
priyam0k marked this pull request as ready for review November 10, 2026 15:14
Comment thread chainladder/core/pandas.py Outdated
)
obj.values = func(obj.values, axis=axis, *args, **kwargs)
else:
with warnings.catch_warnings():

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.

the intent of the if statement is to only let the empty slice or All-NaN slice warnings fire when there is an empty origin or an empty development. i'm concerned that removing the condition results in the warning being silenced too often.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Localizing the filter inside CaseOutstanding.fit around the mean call keeps pandas.py untouched. I can update this here.

Comment thread chainladder/development/outstanding.py Outdated
self.case_ldf_.values = case_to_prior_case_naned.mean(axis=2).values
self.case_ldf_ = case_to_prior_case_naned.mean(axis=2)
xp = self.case_ldf_.get_array_module()
self.case_ldf_.values[xp.isnan(self.case_ldf_.values)] = 1.0

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 this be routed through the existing fillna method?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Routing through self.case_ldf_.fillna(1.0) works cleanly here and avoids touching the array values directly

Comment thread chainladder/development/outstanding.py Outdated
self.paid_ldf_.values = paid_to_prior_case_naned.mean(axis=2).values
self.paid_ldf_ = paid_to_prior_case_naned.mean(axis=2)
xp = self.paid_ldf_.get_array_module()
self.paid_ldf_.values[xp.isnan(self.paid_ldf_.values)] = 0.0

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 this be routed through the existing fillzero method?

if you are thinking, why do we have a separate fillna and fillzero? well, ideally we don't.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Switching to fillzero() makes sense here. Learned something new about the fillna(0) history

assert model.paid_ldf_ is not None
assert not model.case_ldf_.to_frame().isna().any().any()

# Synthetic triangle with zero prior case

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.

should this be two tests if we are using two datasets?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Splitting into two separate tests (one for the XYZ auto tail runoff and one for the synthetic intermediate zero case) will make failures much easier to isolate.

@henrydingliu henrydingliu self-assigned this Nov 10, 2026

# Synthetic triangle with zero prior case

def test_case_outstanding_synthetic_zero_prior_case_warning():

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.

use test fixture?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Will do, I will pull the synthetic zero-case setup into a pytest fixture to keep the test body clean

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.

i meant the genins sample

Comment thread chainladder/development/outstanding.py Resolved
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] Guard zero case reserve division

2 participants