Corrige les erreurs de calcul sur les allègements #862

Morendil · 2017-12-14T21:07:45Z

Correction d'un bug.
Périodes concernées : toutes.
Zones impactées : prelevements_obligatoires/prelevements_sociaux/cotisations_sociales/allegements.
Détails :
- Corrige les erreurs de calculs constatés sur la réduction générale ("Fillon").
- (Fixes Comportement anormal du revenu_super_brut sur Décembre 2017 #844)

Ces changements (effacez les lignes ne correspondant pas à votre cas) :

Corrigent ou améliorent un calcul déjà existant.

Morendil · 2017-12-14T21:21:20Z

J'ai ajouté un test pour mettre en évidence le bug et le comportement attendu (tel que je le comprends) sur coefficient_proratisation, et implémenté le comportement attendu.

Cela ne corrige pas encore le test fonctionnel tel qu'il était initialement rédigé à cause d'un écart de 7 centimes, qu'il faudrait peut-être essayer d'expliquer.

En augmentant la marge d'erreur à 10 centimes les tests de décembre deviennent passants, mais il reste encore 3 échecs sur les tests de novembre. Il est possible qu'il reste d'autres bugs dans le calcul.

Morendil · 2017-12-15T06:54:45Z

Après quelques tâtonnements j'arrive à un état stable, au prix d'une modification du test pour le cas d'un salarié embauché pour le seul mois de novembre dans le cas d'un recouvrement anticipé avec régularisation en fin de période.

Mon interprétation de "anticipé" m'amène à affecter l'allègement à novembre et pas décembre, et la régularisation est alors de 0. Mais je me suis peut-être trompé. Il serait utile de documenter par des références précises le fonctionnement des modes de recouvrement, je n'ai trouvé des informations en ligne que sur le mode progressif.

Le noeud du problème était bien la variable coefficient_proratisation et son interaction avec les dates de début et de fin de contrat. Dans la formulation initiale des tests on ne spécifiait pas la date de fin de contrat, ce qui conduisait à supposer que sur la période de la simulation (2017) le salarié avait été employé pendant 2 mois… mais payé sur 1 seul. Fort logiquement cela revenait à le payer moitié moins et faussait donc la comparaison entre le smic et le salaire.

Autre problème, coefficient_proratisation ne voyait pas d'inconvénient à renvoyer des valeurs négatives si la date de fin était antérieure à la date de début, ce qui arrivait comme conséquence de l'évaluation de contrat_de_travail_debut (ou fin) à une période antérieure à la période (le mois) de versement du salaire. J'ai donc ajouté un plancher à 0.

Je suis perplexe quant au fait qu'une variable comme contrat_de_travail_debut soit sensible à la période pour laquelle elle est évaluée. Dans mon esprit on devrait avoir pour certaines variables quelque chose comme
definition_period = ALWAYS
indiquant que sa valeur est invariante dans le temps. Je n'irai pas jusque là dans l'implémentation, je passe le relais à l'équipe Core.

Anna-Livia · 2017-12-15T14:40:00Z

@Morendil On a definition_period = ETERNITY

Morendil · 2017-12-15T14:43:01Z

@Anna-Livia Oh yeah! Bon ça c'est la bonne nouvelle :) Du coup je reformule ce qui me rend perplexe, pourquoi contrat_de_travail_debut n'est pas en definition_period = ETERNITY - peut-être que la réponse est "on n'y a pas encore pensé" et ça serait OK (je ne juge pas), mais peut-être aussi y a-t-il une raison que je ne connais pas encore…

benjello · 2017-12-15T15:23:36Z

Je ne suis pas sûr que l'on veuille mettre definition_period = ETERNITY pour contrat_de_travail_debut. En effet imaginons que quelqu'un enchaîne plusieurs emplois, contrat_de_travail_debut un mois donné est la valeur du début de son contrat du travail pour le mois en question.

Anna-Livia · 2018-01-04T15:25:04Z

@benjello Le seul contrat_de_travail_debut qui est intéressant dans OpenFisca est celui du contrat de travail en cours, non ? ( Il me semble qu'un scenario décrit une situation à un instant T).
ETERNITY n'est pas un soucis pour moi vu que contrat_de_travail_debut sera pour toujours la date de début de ce contrat de travail d'un instant T.

benjello · 2018-01-04T15:52:47Z

@Anna-Livia : openfisca a été étendu spécifiquement pour gérer plusieurs périodes (retraite, recouvrement des allègements de charge etc) sinon toutes les variables seraient à ETERNITY.

benjello · 2018-01-04T15:53:57Z

Je pense que l'on peut se corriger pour l'usage de @Morendil avec un set_input_variable bien senti.

Morendil · 2018-01-04T16:23:47Z

Il me semble que dans une situation où se succèdent (ou se chevauchent) deux contrats de travail, la valeur de la variable "date de début du contrat" sera fonction non pas de la période à laquelle on évalue cette variable, mais de la finalité pour laquelle on l'évalue.

Pour le dire autrement, la formule de la variable coefficient_proratisation a besoin de connaitre le contrat auquel on l'applique. Un cas de test tout simple: un·e salarié·e qui cumule deux temps partiels sur le même mois, l'un de 1/3 temps et l'autre de 2/3 temps. Je ne saurais pas comment effectuer un calcul correct pour les deux contrats en l'état actuel de l'API des formules. D'un autre côté je ne sais pas non plus exprimer ce cas de test pour l'instant…

Que fait set_input_variable ?

benjello · 2018-01-04T16:44:23Z

@Morendil : on ne gère pas plusieurs contrats de travail en même temps mais on peut gérer une situation ou un individu à plusieurs contrats de travail qui se succèdent.

Anna-Livia · 2018-01-04T16:53:59Z

@benjello on a un test avec un exemple de plusieurs contrats de travails les uns après les autres ? J'ai du mal à voir comment ça se goupille.
Je rejoins aussi @Morendil sur la question du set_input. Comment est-ce que ça pourrait être utilisé ?

benjello · 2018-01-04T17:01:14Z

@Anna-Livia @Morendil : pour l'usage considéré modifier le set_input pour qu'il mette la même date de début du contrat de travail pour tous les mois de l'année. Ou une extension qui converti en period en ETERNITY

Pour plusieurs contrat de travail successifs: spécifier chaque moi la date de début et la date de fin du contrat de travail

Anna-Livia · 2018-01-04T17:06:14Z

Merci @benjello pour ces explications 🙌

benjello · 2018-01-04T17:09:16Z

@Anna-Livia : à tester though, j'expose comment je vois les choses, je ne garantis pas que c'est bien codé ;-)

Morendil · 2018-01-04T18:03:55Z

On a me semble-t-il le même problème dans le cas d'un salarié qui ferait une semaine d'un même mois chez un employeur X et les trois semaines restantes chez un employeur Y: ces deux contrats consécutifs sont "simultanés" du point de vue d'une variable déclarant definition_period = MONTH, comme coefficient_proratisation.

benjello · 2018-01-04T21:15:55Z

@Morendil : oui on ne peut pas descendre plus fin que le mois.

Anna-Livia · 2018-02-15T11:56:24Z

...nce/model/prelevements_obligatoires/prelevements_sociaux/cotisations_sociales/allegements.py

@@ -52,8 +52,8 @@ def formula(self, simulation, period):
        #   forfait_heures_annee
        #   forfait_jours_annee ]

-        contrat_de_travail_debut = simulation.calculate('contrat_de_travail_debut', period)
-        contrat_de_travail_fin = simulation.calculate('contrat_de_travail_fin', period)
+        contrat_de_travail_debut = simulation.calculate('contrat_de_travail_debut', '2099-01')


Il semblerait que les tests passent sans cette modification. Est-ce que tu reproduit @Morendil ?

Anna-Livia · 2018-02-15T11:56:31Z

...nce/model/prelevements_obligatoires/prelevements_sociaux/cotisations_sociales/allegements.py

-        contrat_de_travail_debut = simulation.calculate('contrat_de_travail_debut', period)
-        contrat_de_travail_fin = simulation.calculate('contrat_de_travail_fin', period)
+        contrat_de_travail_debut = simulation.calculate('contrat_de_travail_debut', '2099-01')
+        contrat_de_travail_fin = simulation.calculate('contrat_de_travail_fin', '2099-01')


Il semblerait que les tests passent sans cette modification. Est-ce que tu reproduit @Morendil ?

benjello · 2018-02-21T18:15:31Z

...nce/model/prelevements_obligatoires/prelevements_sociaux/cotisations_sociales/allegements.py


    # Montant de l'allegment
-    return (ratio_smic_salaire < law.plafond_en_nombre_de_smic) * law.reduction * assiette
+    return (assiette < law.plafond_en_nombre_de_smic * smic_proratise) * law.reduction * assiette


benjello · 2018-02-21T18:16:13Z

tests/test_coefficient.py

+def test_coefficient_proratisation_only_contract_periods_wide():
+  tax_benefit_system = FranceTaxBenefitSystem()
+  scenario = tax_benefit_system.new_scenario()
+  scenario.init_single_entity(period='2017', # wide: we simulate for the year


indentation: 4 espaces ;-)

Morendil · 2018-02-21T20:17:09Z

@benjello Merci et done !

Une remarque meta: je vais me permettre de passer outre ton "request changes" pour cette PR à ce stade là, en usant du "Dismiss review", pour les raisons suivantes:

@Anna-Livia est en "Approve" après de multiples relectures
l'erreur de whitespace (sauf erreur de ma part, je suis novice en Python) est totalement transparente sur le plan fonctionnel et donc le problème n'est pas bloquant
d'autres fichiers Python de la codebase souffrent d'erreurs de whitespace (un très petit nombre, j'en conviens) on peut donc s'attendre à une PR corrective pour traiter ce sujet en un seul tir groupé
une fois que la mise à jour du Changelog + setup.py est faite, c'est pénible de devoir re-résoudre les conflits si une PR a été passée entre-temps, et pour moi c'est un frein considérable à pousser moi-même mes PR au bout (au lieu de me défausser sur l'équipe Core)
@Anna-Livia m'a signalé que les PR de l'IPP sont assez urgentes et risquent de passer devant si je tarde, et j'ai zéro bande passante avant lundi, ce qui fait que j'ai promis de merger celle-ci ce soir sur mon temps libre; or je ne peux pas le faire sans "dismiss" ta review.

A titre personnel j'use du "request changes" avec la plus grande parcimonie pour beta.gouv.fr et seulement quand j'ai l'impression que la PR concernée dégraderait le code. Je préfère encourager chaque contributeur à faire preuve de responsabilité quant à la prise en charge de la qualité du code poussé. Le plus souvent je donne un "Approve" et des suggestions d'amélioration, plutôt que risquer de devenir le bottleneck pour la publication d'une PR.

Je cite cet usage personnel à titre informatif et on peut bien entendu choisir d'adopter une norme différente pour OF-Fr.

See comments on PR.

benjello · 2018-02-21T21:10:39Z

@Morendil: tu as bien fait. Généralement, on me reproche d'être trop laxiste ... J'ai peut-être surréagi

Morendil mentioned this pull request Dec 14, 2017

Comportement anormal du revenu_super_brut sur Décembre 2017 #844

Closed

Morendil requested a review from Anna-Livia December 15, 2017 12:57

Morendil assigned benjello and fpagnoux Dec 15, 2017

Morendil requested a review from benjello December 15, 2017 15:01

Morendil unassigned benjello and fpagnoux Dec 15, 2017

Morendil requested a review from fpagnoux December 15, 2017 15:01

Anna-Livia mentioned this pull request Jan 12, 2018

Les Enums sont définis comme en Python 3 #846

Merged

bonjourmauko added the flow:contribution-backlog label Jan 30, 2018

Anna-Livia requested changes Feb 15, 2018

View reviewed changes

Morendil force-pushed the fillon branch from bd91195 to 606a665 Compare February 21, 2018 07:29

Anna-Livia approved these changes Feb 21, 2018

View reviewed changes

benjello previously requested changes Feb 21, 2018

View reviewed changes

Anna-Livia and others added 16 commits February 21, 2018 21:05

Add failing tests

d0552bc

Add failing test for coefficient_proratisation

8124859

Clarify test

1a6d5bd

Make coefficient test pass

8ef9969

Fix test to avoid weird period handling

389bd37

Some test tweaks

1287fcf

Fix coefficient test

b842e96

Fix evaluation date & test period

e26b958

Fix YAML contract end date correcting off-by-1-day

43de39a

Fix timing for allegement in november/anticipe case

91acf12

Distinguish narrow and wide cases

fc3af5e

Use request_last_or_next for contrat de travail

0990ff9

Avoid division by zero by not computing a ratio

21b03a3

Update Changelog and bump version

5a5704c

Répare des enums dans les tests

5e2c44f

Fix whitespace

3e4e58c

Morendil force-pushed the fillon branch from 65d83a5 to 3e4e58c Compare February 21, 2018 20:07

Morendil merged commit 087fc3a into master Feb 21, 2018

Morendil deleted the fillon branch February 21, 2018 21:23

MattiSG removed the flow:contribution-backlog label Feb 21, 2018

Morendil mentioned this pull request Oct 8, 2018

Corriger le calcul de l'allègement Fillon #395

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Corrige les erreurs de calcul sur les allègements #862

Corrige les erreurs de calcul sur les allègements #862

Morendil commented Dec 14, 2017 •

edited

Loading

Morendil commented Dec 14, 2017

Morendil commented Dec 15, 2017

Anna-Livia commented Dec 15, 2017

Morendil commented Dec 15, 2017

benjello commented Dec 15, 2017 •

edited

Loading

Anna-Livia commented Jan 4, 2018 •

edited

Loading

benjello commented Jan 4, 2018

benjello commented Jan 4, 2018

Morendil commented Jan 4, 2018 •

edited

Loading

benjello commented Jan 4, 2018

Anna-Livia commented Jan 4, 2018

benjello commented Jan 4, 2018 •

edited

Loading

Anna-Livia commented Jan 4, 2018

benjello commented Jan 4, 2018

Morendil commented Jan 4, 2018

benjello commented Jan 4, 2018

Anna-Livia Feb 15, 2018

Anna-Livia Feb 15, 2018

benjello Feb 21, 2018

benjello Feb 21, 2018

Morendil commented Feb 21, 2018

benjello commented Feb 21, 2018

Corrige les erreurs de calcul sur les allègements #862

Corrige les erreurs de calcul sur les allègements #862

Conversation

Morendil commented Dec 14, 2017 • edited Loading

Morendil commented Dec 14, 2017

Morendil commented Dec 15, 2017

Anna-Livia commented Dec 15, 2017

Morendil commented Dec 15, 2017

benjello commented Dec 15, 2017 • edited Loading

Anna-Livia commented Jan 4, 2018 • edited Loading

benjello commented Jan 4, 2018

benjello commented Jan 4, 2018

Morendil commented Jan 4, 2018 • edited Loading

benjello commented Jan 4, 2018

Anna-Livia commented Jan 4, 2018

benjello commented Jan 4, 2018 • edited Loading

Anna-Livia commented Jan 4, 2018

benjello commented Jan 4, 2018

Morendil commented Jan 4, 2018

benjello commented Jan 4, 2018

Anna-Livia Feb 15, 2018

Choose a reason for hiding this comment

Anna-Livia Feb 15, 2018

Choose a reason for hiding this comment

benjello Feb 21, 2018

Choose a reason for hiding this comment

benjello Feb 21, 2018

Choose a reason for hiding this comment

Morendil commented Feb 21, 2018

benjello commented Feb 21, 2018

Morendil commented Dec 14, 2017 •

edited

Loading

benjello commented Dec 15, 2017 •

edited

Loading

Anna-Livia commented Jan 4, 2018 •

edited

Loading

Morendil commented Jan 4, 2018 •

edited

Loading

benjello commented Jan 4, 2018 •

edited

Loading