openfisca / openfisca-core

OpenFisca core engine. See other repositories for countries-specific code & data.
https://openfisca.org
GNU Affero General Public License v3.0
171 stars 75 forks source link

Asof parameters #978

Closed benjello closed 1 month ago

benjello commented 3 years ago

New features

benjello commented 3 years ago

@sandcha @guillett @maukoquiroga @eraviart : I know this PR is extremely ugly and I intent to improve it. But I definitively need your advice on how to do that.

benjello commented 3 years ago

@maukoquiroga @sandcha @guillett : I would love to hear about you on this WIP PR ...

benjello commented 3 years ago

@maukoquiroga : you went through a large refactoring and I am quite lost to rebase my branch on master ... Could you rebase it for me? Thanks.

sandcha commented 3 years ago

Does this PR answer this issue: https://github.com/openfisca/openfisca-core/issues/720 ? 🙂

benjello commented 3 years ago

Does this PR answer this issue: #720 ?

No, it is a completely different topic.

sandcha commented 2 years ago

Rebasing the branch as discussed with @benjello.

coveralls commented 2 years ago

Coverage Status

Coverage increased (+0.1%) to 79.005% when pulling d215247fe0d53d6fbda888677e34963e3203fa22 on asof-parameters into 601a397fd02d6c56b751cadfbda0f3e5c911934f on master.

benjello commented 2 years ago

@nikhilwoodruff @eraviart @sandcha @MattiSG :

I need your input to imporve this PR. In particular for the prefix of the generation (ne_apres_YYYY_MM-DD, ne_avant_YYYY_MM-DD). Do after_YYYY_MM-DD, before_YYYY_MM-DD would be more generic ?

nikhilwoodruff commented 2 years ago

This looks nice to me. Of course my preference would definitely be before/after but feel free to outvote this.

sandcha commented 2 years ago

Thank you for this PR and for the examples!

Use cases

Thanks to the parameters_date_indexing/trimtp_rg.yml, I understand that what we call asof here could improve the modelization of some use cases (and that it has nothing to do with reforms 😅). Possible use cases:

Asof is a new feature because...

So far, when we filtered parameters by date, it was the simulation period or, more precisely, the date on which we observed the applicable law. Here, if I'm right, we have use cases where at the same time, we observe different applicable law options that depend on a date specific to the entity. So, we need to access a parameter by a double index: the usual "simulation period" and a new index, a "date" index.

Consequently, it looks great to have some mechanism to manage these situations!

Questions & issues

But I have some doubts about the implementation:

@benjello @nikhilwoodruff What do you think about these implementation questions? 🤔 cc @maukoquiroga @MattiSG

To summarize what I'm saying here: such a parameter would be great 🙌 , keys to parse like after_YYYY_MM_DD open multiple issues, any other structured format where we would not have keys to parse to extract multiple information (😰) would work for me 😊

sandcha commented 2 years ago

One solution might be to see the asof parameter as a scale that evolves by date: the thresholds values would be dates as they are naturally ordered and could be filtered with the usual .calc(information from the entity) method.

Here is a first example from trimtp_rg.yml; it would be changed as follows:

- nombre_trimestres_cibles_par_generation:
-   description: Nombre de trimestres cibles par génération
-   before_1934_01_01:
-     description: Avant 1934
-     values:
-       1983-01-01:
-         value: 150.0
-   after_1934_01_01:
-     description: '1934-01-01'
-     values:
-       1994-01-01:
-         value: 151.0
-       1983-01-01:
-         value: null

+ number_targeted_quarters_per_generation:
+   description: The number of targeted contribution quarters per generation
+   metadata:
+     threshold_unit: date
+     amount_unit: quarters
+ brackets:
+   - amount: 
+       1983-01-01:
+         value: 150.0
+     1994-01-01:
+         value: null
+     threshold: 
+       1983-01-01:
+         value: 0001-01-01 # from python/openfisca default date to next threshold
+   - amount: 
+       1994-01-01:
+          value: 151.0
+     threshold: 
+       1983-01-01:
+         value: 1934-01-01 # born after January 1934

This would also mean that we extend the allowed parameters types to include dates. Would it work for your use case?

benjello commented 2 years ago

@sandcha @nikhilwoodruff : I think we should go for a more general multiindexed array parameters. What do you think of something like the following:

quarters_by_generation:
  description:  Quarters needed to get full retirement by generation
  index:
    generation:
      description: date of birth
      type: date
      selection: asof # could be exact by default or any other  
    # period is the implicit 0-axis
    # We can have more than one more axis.
  values:
    null:
      1983-01-01:
        value: 150.0
    1934-01-01:
      1994-01-01:
        value: 151.0
      1983-01-01:
        value: null

cc @eraviart @guillett @maukoquiroga

nikhilwoodruff commented 2 years ago

@sandcha @nikhilwoodruff : I think we should go for a more general multiindexed array parameters. What do you think of something like the following:

quarters_by_generation:
  description:  Quarters needed to get full retirement by generation
  index:
    generation:
      description: date of birth
      type: date
      selection: asof # could be exact by default or any other  
    # period is the implicit 0-axis
    # We can have more than one more axis.
  values:
    null:
      1983-01-01:
        value: 150.0
    1934-01-01:
      1994-01-01:
        value: 151.0
      1983-01-01:
        value: null

cc @eraviart @guillett @maukoquiroga

I think that scheme would work great for us, @benjello - I like that it's more streamlined. I'm actually just re-implementing a part of the UK system that I think this feature would probably simplify a lot: the Child Tax Credit has a limit on the number of children eligible (2), but when testing eligibility for each child, we need to check against the child limit at the date of their birth, rather than at the current date. Am I right in thinking that with this PR, we'd just need to do something like the below parameter and formula?

description: Child limit
values: 
  null:
    null: .inf
  2017-04-06:
    2017-04-06: 2
def formula(person, period, parameters):
  birth_date = person("birth_date", period)
  child_index = person("child_index", period)
  meets_limit = child_index < parameters(period).child_limit[birth_date]
  return meets_limit
benjello commented 1 month ago

Implemented in #1212