openfisca / openfisca-france

French tax and benefit system for OpenFisca
https://openfisca.org/fr
256 stars 97 forks source link

Misworking of calculate_divide #614

Closed edarin closed 7 years ago

edarin commented 7 years ago

Following a question from Robert Cauneau, we've noticed that some input variable can't be divided into months.

cf : https://jupyter.openfisca.fr/notebooks/users/robert/Etudes%20de%20variables/Test_Retraite_divided.ipynb.

For example: the retraite_brute
But the problem is that some variables are based upon this one (as csg_deductible_retraite) and that these variables are calculated on a monthly basis. So there are every time put to zero.

Looking at the code, I think that only one line is missing in the definition of retraite_brute in comparison to other variables: set_input = set_input_divide_by_period`.

Ths issue concerns at least : retraite_brute, chomage_brut

But also all monthly variables that you want to add as entry variable in the init_single_entity method.

benjello commented 7 years ago

I agree with you @edarin. We may add set_input_divide_by_period to retraite_brute, chomage_brut. But this may introduce a non-controlled approximation that user do not catch. I know we do that elsewhere but i am not happy with the current situation. I would say go for it but we should open an issue where we should list all the year/month period handling problems systematically since some similar problems arise with cotisations sociales too.

edarin commented 7 years ago

but @fpagnoux thought that it might be a real problem with the function calculate.divide(). At least if I understood him.

By the way why it is a non-controlled approximation ?

benjello commented 7 years ago

Yes, I am not very confortable with all these calculate_divide etc. I think we should only use calculate_add. But again, we should be systematic about these matters. It might be interesting to list the remaining calculate_divide and calculate_add_divide to understand all the use cases. I know have a good idea of the use cases for the cotisations but I do not know well enough the ones on the prestations side.

edarin commented 7 years ago

I did the modif I spoke on my computer (adding the set_input_divide_by_period) and it rocks ! ^^

Actually it's not exactly because of the calculate_divide. This function was not used in the computation.

edarin commented 7 years ago

but there is still a huge question : when you want to give in the scenario.init_single_entity a specific monthly based variable it is automaticlay converted in an annual basis and then you can call it on annual basis without an error assertion (problematic) AND you can't have it on a monthly basis !

For better understanding : https://jupyter.openfisca.fr/notebooks/users/robert/Etudes%20de%20variables/Test_Retraite_divided.ipynb#Monthly-based-variabe-as-input

robert49 commented 7 years ago

May I hope that this problem will be solved soon ? I can not go further in my simulations of basic income for retirees and unemployed.

benjello commented 7 years ago

@robert49, I will take a look at your notebook ASAP.

MattiSG commented 7 years ago

@fpagnoux @cbenz à quel point cette issue est-elle impactée par https://github.com/openfisca/openfisca-core/issues/442, https://github.com/openfisca/openfisca-core/issues/459 et https://github.com/openfisca/openfisca-core/issues/468 ?

fpagnoux commented 7 years ago

Le set_input_divide_by_period a déjà été ajouté j'ai l'impression, donc l'issue est probablement obsolète. Et oui, https://github.com/openfisca/openfisca-core/issues/442 a un impact sur le problème.

Dans les circonstances mentionnées (sans le set_input ajouté depuis):

MattiSG commented 7 years ago

l'issue est probablement obsolète

Je ferme. Ne pas hésiter à rouvrir si le problème est toujours présent.