mages / ChainLadder

Claims reserving models in R
https://mages.github.io/ChainLadder/
77 stars 63 forks source link

ChainLadder doesn't like dates as origins #12

Closed PirateGrunt closed 8 years ago

PirateGrunt commented 8 years ago

The following code will generate an error:

op <- as.Date(paste0(2001:2010, "-01-01"))
lags <- 1:10

triangle <- expand.grid(op, lags)
names(triangle) <- c("origin", "dev")
set.seed(1234)
triangle$value <- rnorm(100)

triCL <- ChainLadder::as.triangle(triangle)
plot(triCL, lattice=TRUE)

The error is:

Error in .as.LongTriangle(x, na.rm) : 
  The origin and dev. period columns have to be of type numeric
 or a character which can be converted into numeric.

The origin column in the triangle is numeric. A call to typeof will return "double" and class will return "Date".

fabioconcina commented 8 years ago

Hi,

your origin column is indeed a character:

dimnames(triCL)$origin [1] "2001-01-01" "2002-01-01" "2003-01-01" [4] "2004-01-01" "2005-01-01" "2006-01-01" [7] "2007-01-01" "2008-01-01" "2009-01-01" [10] "2010-01-01"

typeof(dimnames(triCL)$origin[1]) [1] "character"

You should use a numeric type instead, as you have done for the development period.

Hope this helps!

Fabio

2016-08-05 21:12 GMT+02:00 Brian Fannin notifications@github.com:

The following code will generate an error:

op <- as.Date(paste0(2001:2010, "-01-01")) lags <- 1:10

triangle <- expand.grid(op, lags) names(triangle) <- c("origin", "dev") set.seed(1234) triangle$value <- rnorm(100)

triCL <- ChainLadder::as.triangle(triangle) plot(triCL, lattice=TRUE)

The error is:

Error in .as.LongTriangle(x, na.rm) : The origin and dev. period columns have to be of type numeric or a character which can be converted into numeric.

The origin column in the triangle is numeric. A call to typeof will return "double" and class will return "Date".

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/mages/ChainLadder/issues/12, or mute the thread https://github.com/notifications/unsubscribe-auth/AML4Bsdqg5PausomijZgMoOF6SQwhJsbks5qc4sfgaJpZM4Jd-kj .

PirateGrunt commented 8 years ago

Fabio,

Thanks for your response. The data frame being fed to the "as.triangle" function has an origin column which is indeed numeric. See the results of the call to "typeof" in the code snippet below. Is it possible that the date field is converted during the "as.triangle" function?

op <- as.Date(paste0(2001:2010, "-01-01"))
lags <- 1:10

triangle <- expand.grid(op, lags)
names(triangle) <- c("origin", "dev")
set.seed(1234)
triangle$value <- rnorm(100)

lapply(triangle, typeof)
mages commented 8 years ago

Brian, I guess, I can this issue now, thanks to your fix.

trinostics commented 8 years ago

I have a few questions/issues, which may be late to the party.

Curious why you used intersect rather than inherits, which would yield a logical. E.g. isDate <- inherits(Triangle[[origin]], "Date") if (isDate) { etc.

The error doesn't occur in the as.data.frame.triangle function, but occurs in the plot method when lattice=TRUE, because triCL <- ChainLadder::as.triangle(triangle) plot(triCL) both work without an error. Furthermore, Date's are convertible to numeric, so that shouldn't be the problem, so I wouldn't recommend a modification to as.triangle.data.frame to solve an issue in plot.triangle(lattice=TRUE).

I have not looked at the plot.triangle method for the case when lattice=TRUE, but a more general solution to the issue would be to

  1. test whether origin is convertible to numeric (assuming that's the issue)
  2. if not, first convert origin to a factor, then to numeric. (That should always work, with some attention being paid to NA's.) That will at the minimum generate integers 1 thru number of levels, which should hopefully solve lattice's issue. Then, within the plot.triangle method the code could be further enhanced to use the levels of the factor as more helpful plot labels, if necessary.

However this is resolved, there should be a unit test for the new functionality.

Finally, the help for ChainLadder has documentation for the method 'as.triangle.data.frame'. However, as.triangle.data.frame(GenInsLong) gives the error Error: could not find function "as.triangle.data.frame" (because that name is not exported) whereas the following works correctly as.triangle(GenInsLong, origin="accyear", dev="devyear", value="incurred claims") The help should be corrected so that as.triangle.data.frame doesn't show up as exported (I realize this issue is unrelated to the plot issue).

PirateGrunt commented 8 years ago

Hey Dan,

I didn't use inherits because, embarrassingly, I was either never aware of it, or had forgotten about it. I'll need to brush up on my functions to do with classes.

I agree that as.triangle.data.frame does not cause an error. However, when the data frame is constructed, it'll assign the "origin" field to row names. This converts the dates to strings. At a later point, we'll try to convert the row names to numeric and this will produce NAs for the origin. Try as.numeric("2014-01-01") at the console and you should get NA as a result. The plot(lattice=TRUE) was the first downstream error that I've noticed. It's possible that there are others.

I think conversion to factor, then numeric may make a great deal of sense. as.numeric(factor("2014-01-01")) gives a result of 1, so that's a good thing. Happy to take a stab at implementing that on this branch.

Don't mean to rock the boat. First pull request ever. I love ChainLadder!

trinostics commented 8 years ago

Yes, of course! The rownames of the resulting matrix (wide triangle) are necessariy character, and as.numeric on those date-strings will not work.

It looks like the error message is generated in .as.LongTriangle. It would be nice to know all the places in ChainLadder that call .as.LongTriangle. In Triangles.R it only gets called by as.data.frame.triangle. IMO, .as.LongTriangle could easily be replaced by Hadley's melt function, unless there's a downstream reason the dimnames of the matrix must be convertible to numeric.

If that code in lines 168-170 were commented out, I wonder what would break. Hopefully there would be a ChainLadder unit test that was programmed to make sure that code was there -- assuming it's necessary.

I would be delighted if you took a stab at playing around with it on your branch. Thanks, Brian!

BTW, what is your use case for rownames being dates? Better labeling?

chiefmurph commented 8 years ago

I volunteer to work on the unit test for this change. I will make a test of the example in your OP, Brian. I created a branch called DateLabelsUnitTest and will push (which github for Desktop calls "Publish", I think) when done.

chiefmurph commented 8 years ago

Finished, and pull request issued. Turns out I was incorrectly using Github Desktop when making changes to code in the ChainLadder repository. All straightened out now, thanks to the step by step instructions in this video: https://www.youtube.com/watch?v=XdhuWDdu-rk&feature=youtu.be