Skip to content

Friedland Recreation Chapter 8 - #1121

Open
kennethshsu wants to merge 312 commits into
mainfrom
#588_chapter8
Open

Friedland Recreation Chapter 8#1121
kennethshsu wants to merge 312 commits into
mainfrom
#588_chapter8

Conversation

@kennethshsu

@kennethshsu kennethshsu commented Jul 14, 2026

Copy link
Copy Markdown
Member

Summary of Changes

Related GitHub Issue(s)

#588
#919

Additional Context for Reviewers

Preview here

  • I passed tests locally for both code (uv run pytest) and documentation changes (uv run jb build docs --builder=custom --custom-builder=doctest)

Note

Medium Risk
Changes core Triangle date parsing for a specific development_format, which can affect lag/grain for datasets using calendar-year-as-year-end; sample data changes are low risk.

Overview
Adds Friedland chapter 8 to the docs table of contents (friedland/chapter_8.ipynb).

Triangle development dates: When development_format is %Y-12-31, development values are normalized to calendar year-end via to_period("Y").to_timestamp(how="e") after parsing—addressing year-end calendar-year development handling (related to #919).

Sample friedland_gl_self_insurer: Manifest and CSV now include a Population column alongside reported/paid claims; the sample already uses development_format: "%Y-12-31".

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

@kennethshsu

Copy link
Copy Markdown
Member Author

That looked horrible and I fixed it. I copied the results from the actual run and forgot to replace with the values from the text.

by raw screen estate, the analysis is not even 10%. it's print statements, hardcoded dev patterns, and columns and columns of numbers in single file. by contrast, here is the current state of chapter 11 that i tried to suggest as an alternative

I don't know that this is a fair comment lol, you are looking at columns that are just data, of course it's not "useful".

When I was constructing this workbook, I was thinking what would be most useful to a reader if they wanted to recreate the examples in the text, and do only that. The chapter 11 stuff is doing so much more than teaching users how to use the package, it's actually doing reconstruction with exhibits, and imo, is too much. It's actually harder for the users to figure out exactly which line of code will recreate the numbers. I'm not saying we shouldn't go there, but I don't think that is a priority now. If we think this is the MVP and are all aligned, than I will do that, but we should all come to an agreement here.

Anyways, we have a few chapters in now, and they all look widely different. We will need to decide how to standardize them.

Please take another look and see if I missed anything...

@kennethshsu
kennethshsu marked this pull request as ready for review July 21, 2026 18:28
@henrydingliu

Copy link
Copy Markdown
Member

The chapter 11 stuff is doing so much more than teaching users how to use the package, it's actually doing reconstruction with exhibits

but it is not. please take a moment and read the cell under "Exhibit 1 Analysis". it's not printing anything. it's not doing any pandas manipulation. it's using chainladder API and only chainladder API.

image

When I was constructing this workbook, I was thinking what would be most useful to a reader if they wanted to recreate the examples in the text, and do only that.

That's a worthy approach. but this notebook is not actually accomplishing that. right now it's very hard for a reader to follow. when you read this notebook yourself, i'm sure you are scrolling straight past the print statements and focusing on the chainladder bits. But a new user does not have that visual filter baked in.

If we think this is the MVP and are all aligned, than I will do that, but we should all come to an agreement here.

I'm not saying that well-formatted exhibits is the MVP. i'm saying that (1) printing individual columns is distracting; (2) if you dont want to print full exhibits, then do a version of what I'm doing in chapter 11, i.e. do all the analysis, and just print out the final ultimate.

Anyways, we have a few chapters in now, and they all look widely different. We will need to decide how to standardize them.

I think having different coding styles are fine. it showcases that this is a community project, and that all coding styles are welcome. right now this chapter 8 is the only one that looks wildly different, because of the individual columns.

@kennethshsu
kennethshsu marked this pull request as draft July 22, 2026 19:46

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 577a718. Configure here.

Comment thread docs/friedland/chapter_8.ipynb Outdated
Comment thread docs/friedland/chapter_8.ipynb Outdated
@kennethshsu

Copy link
Copy Markdown
Member Author

Ok. I reworked the exhibits and now they look much more like Chapter 7's now. Please take a look and let me know of any other feedback.

New doc

@kennethshsu
kennethshsu marked this pull request as ready for review July 28, 2026 05:28
@henrydingliu

Copy link
Copy Markdown
Member

as_series is called a whopping 48 times. we should add it as a util function into the package, or maybe change the behavior of to_frame. alternatively, give cl.model_diagnostics a chance

  • page 140
exhibit_i_s1["CDF Reported (4)"] = origin_cdf(auto_bi["Reported Claims"], reported_pattern).values
exhibit_i_s1["CDF Paid (5)"] = origin_cdf(auto_bi["Paid Claims"], paid_pattern).values

i've been harping on this to the other collaborators, so i gotta be fair here and give you the same feedback. we should use the chainladder API whenever we can. so either reported_bi.ldf_, or cl.model_diagonstics(cl.Chainladder().fit(reported_bi)).

@henrydingliu

Copy link
Copy Markdown
Member

@kennethshsu didn't notice you moved it out of draft. we consolidated the two scenarios for the us auto dataset. could you please update your load_sample?

@kennethshsu

Copy link
Copy Markdown
Member Author

Sorry, I meant to tag you again...

Good call on the pattern stuff, I updated that and you probably saw already. And I guess now you made your point about having both cdf/ldf in cl.model_diagonstics() 😄

I'll create a ticket on the "as_series", maybe cl.to_series() and even cl.to_values()? We can discuss this on that ticket there. A few decisions will need to be made though.

And I migrated the auto dataset from main.

Feel free to take another look and let me know if there's any other feedback. Thanks sir!! @henrydingliu

@kennethshsu

Copy link
Copy Markdown
Member Author

Opened #1190

Comment thread chainladder/core/base.py
period_end=True,
date_format=development_format
)
if development_format == "%Y-12-31":

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.

after doing some research, this looks to be only applicable to the friedland_gl_self_insurer dataset. what was the issue that needed this line to resolve? Do we have a bigger gap here when the development date is at the end of the month?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I think maybe because it's the only "1D" triangle (i.e. no development, latest valuation only), so it defaults to OYDM instead of OYDY, and OYDM will get you Jan 31 instead. If you have developments (which is really valuations lol), those are mapped/inferred correctly because the "D" will map to the same "end" on the origin ("Y").

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.

5 participants