openfisca-core icon indicating copy to clipboard operation
openfisca-core copied to clipboard

Add feature - interpolate on a SingleAmountTaxScale

Open RamParameswaran opened this issue 4 years ago • 8 comments

New features

  • Introduce ability to interpolate between values in a SingleAmountTaxScale

  • Currently we can return the nearest value (floor or ceiling) between two thresholds, based on an 'input_value' using SingleAmountTaxScale.calc(input_value)

  • This PR allows calculating the linear-interpolated value between two thresholds using SingleAmountTaxScale.calc(input_value, interpolate=True).

    • Behind the scenes it uses the numpy.interp() function
    • I've passed through the right= and left= keyword arguments for user flexibility

Use case example:

# An example: 
thresholds = [0,2,4,6,8,10]    # x-values
values = [0,10,30,60,100,50]   # y-values

# We want to find y values for x=3, x=7, and x=9, using piecewise linear interpolation
y(3) ==> 20
y(7) ==> 80
y(9) ==> 75

RamParameswaran avatar Apr 13 '21 05:04 RamParameswaran

@RamParameswaran : it looks like the interpolation procedure you are applying is converting a SingleAmountTaxScale to a LinearAverageRateTaxScale. Could you confirm ? If it is the cas why about using a converter.

benjello avatar Apr 13 '21 08:04 benjello

@RamParameswaran : it looks like the interpolation procedure you are applying is converting a SingleAmountTaxScale to a LinearAverageRateTaxScale. Could you confirm ? If it is the cas why about using a converter.

@benjello I think the interpolation in this PR is different from a LinearAverageRateTaxScale because it specifically deals with amounts, not rates.

In my use case, we want to specify the parameters yaml as [threshold, amount] pairs. We probably could use a LinearAverageRateTaxScale, but we'd have to transform the parameter data so much, that it would not be easily traceable back to the legislation. I think this PR is a much cleaner approach.

# An example: 
thresholds = [0,2,4,6,8,10]    # x-values
amounts = [0,10,30,60,100,50]   # y-values

# We want to find y values for x=3, x=7, and x=9, using piecewise linear interpolation
y(3) ==> 20
y(7) ==> 80
y(9) ==> 75

RamParameswaran avatar Apr 13 '21 13:04 RamParameswaran

@RamParameswaran : yes you will have to compute the rates but I think that my suggestion is cleaner since in the end what you get is actually a LinearAverageRateTaxScale.

benjello avatar Apr 13 '21 13:04 benjello

Thanks @benjello ! And @RamParameswaran thanks for this contribution. One question: why not, instead of adding another responsibility to the SingleAmountTaxScale, create another amount scale responsible for just this use case?

bonjourmauko avatar Apr 13 '21 14:04 bonjourmauko

So for example we would have:

class LinearAverageAmountTaxScale(AmountTaxScaleLike):
   pass

Then we will be able to rely on polymorphism —multiple dispatch— to decide which scale to instantiate depending on the requested simulation parameters.

bonjourmauko avatar Apr 13 '21 14:04 bonjourmauko

@maukoquiroga : you will add another object that behaves like an existing one ... But I won't die on this hill and I can live with @RamParameswaran solution but I would definitely like to have a converter to enrich the TaxScale ecosystem.

benjello avatar Apr 13 '21 15:04 benjello

you will add another object that behaves like an existing one ...

Agreed - for this reason, I do not think creating another TaxScale is appropriate.

@benjello I see your point about it effectively becoming a LinearAverageRateTaxScale. I'll try my hand at creating a converter tomorrow, and follow up :+1:

RamParameswaran avatar Apr 14 '21 11:04 RamParameswaran

Hi @benjello and @maukoquiroga

I've written a converter function (see branch: 'add-converter-SingAmountTaxScale'). I'm not sure how useful it is, or indeed what it really means to convert a SingleAmountTaxScale to a LinearAverageRateTaxScale. What are the use-cases? :confused: Would you mind doing a sanity-check @benjello?

I think that conceptualising what it means to convert a SingleAmountTaxScale into a LinearAverageRateTaxScale requires quite a lot of mental abstraction. I think it is unreasonable to expect users to do this, just to perform simple arithmetic like linear interpolation between two thresholds. But I appreciate that this comment is rather philosophical :sweat_smile: - keen to hear your thoughts!

For the above reasons, I am still in favour of the approach of this current PR (998).

Thanks! :smile:

RamParameswaran avatar Apr 15 '21 05:04 RamParameswaran