Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adapte France à la modification de la gestion des spirales #1268

Merged
merged 10 commits into from
Mar 29, 2019

Conversation

sandcha
Copy link
Contributor

@sandcha sandcha commented Feb 4, 2019

Fixes #1211
Fixes #1283
Connected to openfisca/openfisca-core#817

  • Correction d'un crash affectant les inférences du système socio-fiscal.
  • Périodes concernées : toutes.
  • Zones impactées : RSA, revenu disponible, cotisations, réforme net/brut
  • Détails :
    • Adapte France à la modification de la gestion des spirales
    • Pour plus d'informations, voir le descriptif de la PR

Les modifications de Core dans openfisca/openfisca-core#817 concernent la détection et la gestion des cas où le calcul d'une variable à une période implique de calculer préalablement la même variable à une autre période ("spirales").

Adaptation des tests du RSA

Les versions antérieures étaient affectées par un bug (#1283, openfisca/openfisca-core#749) se manifestant notamment pour la variable rsa, fixée par erreur à 0 lorsqu'on calculait préalablement revenu_disponible dans la même simulation, alors que sa "vraie" valeur (constatée en calculant rsa en premier) était différente.

La correction de ce bug entraîne la réintégration du RSA non nul dans le revenu disponible, et implique de forcer à 0 les valeurs du RSA incriminées. Cela concerne

  • tests/formulas/aides_logement_fiabilisation.yaml
  • tests/formulas/revenu_disponible.yaml

Réforme de net à brut

La réforme "de net à brut" ne fonctionne que partiellement avec ces modifications, le montant du salaire brut à partir du net est calculé correctement, mais il semble que les résultats intermédiaires sont désormais impliqués dans une spirale et par conséquent retirés du cache. Ces vérifications sont pour l'instant mises en commentaires.

Mode de recouvrement des cotisations

Le test cas_types_ui.yaml échoue car le calcul des cotisations selon le mode de recouvrement par défaut ("mensuel anticipé") exige de régler max_spiral_loops = 2 dans Core, mais cela dégrade légèrement les performances (temps d'exécution) alors que max_spiral_loops = 1 présente une amélioration. On bascule donc sur le mode mensuel_strict par défaut.

Ces deux derniers cas seront corrigés dans de futures PRs.

Tests de performance

master

Premier test revenu disponible
2.602721 s
Second test revenu disponible
2.312436 s
3e test revenu disponible
2.339221 s
Premier test ciblé spirale
1.209179 s
Second test ciblé spirale
1.118555 s
3e test ciblé spirale
1.162432 s

simplify-cycle-detection-redux

Premier test revenu disponible
2.566360 s
Second test revenu disponible
2.303230 s
3e test revenu disponible
2.196354 s
Premier test ciblé spirale
0.681062 s
Second test ciblé spirale
0.732799 s
3e test ciblé spirale
0.688142 s


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

  • Modifient l'API publique d'OpenFisca France (par exemple renommage ou suppression de variables).
  • Corrigent ou améliorent un calcul déjà existant.

Quelques conseils à prendre en compte :

@sandcha
Copy link
Contributor Author

sandcha commented Feb 4, 2019

Premier bilan concernant les performances de cette branche et sa mère en Core.

Contexte
Avec @fpagnoux, nous avons cherché à évaluer s'il y avait une différence nette de temps d'exécution d'un calcul de variable avec spirale sur la branche master et la branche simplify-cycle-detection-redux qui ici, cherche à résoudre le problème de spirale détecté sur France.

Méthode
Nous avons mesuré le temps de simulation.calculate(...) sur France par lots de 10 simulations avec le script measure_spiral_performances.py

Résultats
Ces temps n'indiquent pas d'écart de performances net suite à l'utilisation de la branche simplify-cycle-detection-redux traitant des spirales.

Voici les traces obtenues sachant que :

  • Les 3 premiers calculs de temps d'exécution font appel à revenu_disponible parce qu'il impose une profondeur de calcul supérieure à la moyenne.
  • Les 3 suivants exécutent le test python remonté par mes-aides dans la branche test-cycle indiquée dans ce commentaire.

Sur branches Core et France simplify-cycle-detection-redux

Premier test revenu disponible
3.140392 s

Second test revenu disponible
2.664585 s

3e test revenu disponible
2.635230 s

Premier test ciblé spirale
1.410129 s

Second test ciblé spirale
1.464613 s

3e test ciblé spirale
1.393975 s

Sur branches master de Core 25.2.9 et France 34.5.0

Premier test revenu disponible
3.166132 s

Second test revenu disponible
2.551172 s

3e test revenu disponible
2.225182 s

Premier test ciblé spirale
1.425713 s

Second test ciblé spirale
1.119020 s

3e test ciblé spirale
1.449040 s

@Morendil Morendil force-pushed the simplify-cycle-detection-redux branch 3 times, most recently from 4308d88 to eb0fab4 Compare March 19, 2019 23:27
@Morendil Morendil changed the title [WIP] Adapte à la détection de spirale Adapte France à la modification de la gestion des spirales Mar 20, 2019
@Morendil Morendil force-pushed the simplify-cycle-detection-redux branch from 6846cd1 to 50cf0d5 Compare March 20, 2019 16:48
@bfabre01
Copy link

@Morendil @sandcha : savez-vous environ quand cette PR pourra être mergée ?

@bfabre01
Copy link

Hello @Morendil et @sandcha , savez-vous quand est-ce que cette PR pourra être mergée, pour qu'on l'importe au CASD ? Merci.

@sandcha
Copy link
Contributor Author

sandcha commented Mar 27, 2019

On réintervient dessus cet après-midi. Etat des lieux à venir en fin de journée.

@Morendil
Copy link
Contributor

@bfabre01 Pas eu le temps de boucler ça aujourd'hui, on s'y met demain matin.

@bfabre01
Copy link

@sandcha @Morendil Ok ça marche, merci !

@Morendil Morendil force-pushed the simplify-cycle-detection-redux branch from 50cf0d5 to e99cb07 Compare March 28, 2019 12:14
@Morendil Morendil force-pushed the simplify-cycle-detection-redux branch 2 times, most recently from dbe3098 to 2fab638 Compare March 29, 2019 09:29
@Morendil Morendil force-pushed the simplify-cycle-detection-redux branch from 2fab638 to fad173d Compare March 29, 2019 09:40
@Morendil Morendil merged commit b26b59f into master Mar 29, 2019
@Morendil Morendil deleted the simplify-cycle-detection-redux branch March 29, 2019 10:12
@bfabre01
Copy link

@Morendil @sandcha : super, merci beaucoup !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug dans calcul RSA Revenir en alignement avec Mes Aides
3 participants