casact / chainladder-python

Actuarial reserving in Python
https://chainladder-python.readthedocs.io/en/latest/
Mozilla Public License 2.0
191 stars 71 forks source link

A standard for assumption vectors #127

Open jbogaardt opened 3 years ago

jbogaardt commented 3 years ago

Many assumptions/hypterparameters of Estimators take on a vector-like feel along an axis of a Triangle. However, there is a great deal of inconsistency in implementation across estimators with at least five different approaches being used.

For example:

import chainladder as cl
import numpy as np
raa = cl.load_sample('raa')

# List-style approach
cl.Development(n_periods=[3]*5+[-1]*4).fit(raa)
# Dict-style approach
cl.DevelopmentConstant(
    patterns={k: 3.0**(12/k) for k in range(12, 120, 12)},
    style='cdf').fit(raa) 
# tuple-style approach
cl.TailCurve(fit_period=(3, None)).fit(raa)
# callable approach
cl.DevelopmentConstant(
    patterns=lambda x : {k: 3.0**(12/k) for k in range(12, 120, 12)},
    style='cdf').fit(raa)
#  numpy array approach
bcl, bf, cc = cl.Chainladder(), cl.BornhuetterFerguson(), cl.CapeCod()
estimators = [('bcl', bcl), ('bf', bf), ('cc', cc)]
weights = np.array([[1, 0, 0]] * 4 + [[0, 1, 0]] * 3 + [[0, 0, 1]] * 3)
cl.VotingChainladder(estimators=estimators, weights=weights) 

We need to determine a flexible standard that can can be used by all estimators. Multiple approaches should be fine but should be implemented everywhere so that users can use the style that matches their mental model. We will also need to retain backward compatibility of any deprecated approaches until the next major release.

cbalona commented 3 years ago

I have two thoughts to address this:

  1. Define a "VectorAssumptionHandler" class that implements conversion methods for each of the approaches that converts the approach to a single global approach. For example, if the vector is a callable type, then use an vector_assumption_is_callable method to convert the callable to an array that will be used. One problem I see with this approach is that I don't think the conversion can be standardised across estimators. In these cases we will need to overwrite or augment the methods, and this will lead to fragmented code and bloat; why define the class if we will just keep overwriting its methods?
  2. Enforce that each estimator has a conversion method for the chosen approaches through tests. Something like assert hasattr('vector_assumption_is_callable') or, even better, directly testing each method for each estimator. Write these methods for each estimator.

What I like about 1. is that its easier to manage across all estimators. But, if we cant define a general method to convert each approach for each estimator, then its pointless. Ignoring that desire, I haven't yet thought of a cleaner approach than 2.

jbogaardt commented 3 years ago
  1. One problem I see with this approach is that I don't think the conversion can be standardised across estimators.

I'm trying to think of where this would be a problem, but can't come up with any. As long as the class has access to the vector-like assumption and the Triangle (which it does during fit) then I think it should work. Note, Triangle here can be any Triangle not just X itself. For example X.ldf_

Each estimator would have to specify the axis when invoking the handler, but that should be fine as it always seems to be unambiguous. For VotingChainladder it is the origin for Development it is the development axis.

Perhaps VectorAssumptionHandler shows up near EstimatorBase in the inheritance hierarchy. Within any estimator we can then just use it.

def fit(self, X, y=None, sample_weight=None):
    ...
    coerced_assumption = self.handle_vector(self.any_vector_assumption, triangle=X.link_ratio, axis=3)
    assert type(coerced_assumption) == np.ndarray # an array broadcastable to shape of X.link_ratio
    ...
cbalona commented 3 years ago

So here is one case that I looked at where the treatment would differ for estimators. When an int is provided, Development uses the following conversion method:

def _assign_n_periods_weight_int(self, X, n_periods):
        """ Zeros out weights depending on number of periods desired
            Only works for type(n_periods) == int
        """
        xp = X.get_array_module()
        if n_periods < 1 or n_periods >= X.shape[-2] - 1:
            return X.values * 0 + 1
        else:
            val_offset = {
                "Y": {"Y": 1},
                "Q": {"Y": 4, "Q": 1},
                "M": {"Y": 12, "Q": 3, "M": 1},
            }
            val_date_min = (
                X.valuation[X.valuation <= X.valuation_date]
                .drop_duplicates()
                .sort_values()
            )
            val_date_min = val_date_min[
                -n_periods * val_offset[X.development_grain][X.origin_grain] - 1
            ]
            w = X[X.valuation >= val_date_min]
            return xp.nan_to_num((w / w).values) * X.nan_triangle

But this is quite different to what VotingChainladder would use, in fact, an int should not be provided for it. Is this just an issue for the case of an int? I.e. would a callable be equally handled for all estimators (allowing for axis and passing X in)?

jbogaardt commented 3 years ago

Ah I see. I was thinking the class would "handle" the coercion of the assumption to a numpy array only, but what each estimator does with that numpy array is up to them. In this way, Development would be able to take the following as inputs:

# These are currently supported
n_periods=5
n_periods = [5]*10
# These are not supported...yet
n_periods = lambda : 5
n_periods = {i: 5 for i in range(12, 120, 12)}

In each case, the assumption would resolve to:

np.array([[[[5,5,5,5,5,5,5,5,5,5]]]])

I was thinking Development would still need to retain _assign_n_periods_weight_int, and it would need to be refactored to support the numpy array. But it could also lose any type checking logic like _assign_n_periods_weight_list which is a private method to standardize the n_periods input to a list.

cbalona commented 3 years ago

In that case I think MethodBase should just have a private function that coerces the inputs to an array, and then each estimator can do with the array what it needs to after having been refactored to handle the array.

jbogaardt commented 3 years ago

makes sense