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

Eligibilite rsa jeune #1253

Merged
merged 4 commits into from
Jan 10, 2019
Merged

Eligibilite rsa jeune #1253

merged 4 commits into from
Jan 10, 2019

Conversation

benjello
Copy link
Member

@benjello benjello commented Jan 8, 2019

Merci de contribuer à OpenFisca ! Effacez cette ligne ainsi que, pour chaque ligne ci-dessous, les cas ne correspondant pas à votre contribution :)

  • Correction d'un crash.
  • Périodes concernées : toutes.
  • Zones impactées : prestations/minima_sociaux/rsa.
  • Détails :
    • Gestion de la variable rsa_eligibilite pour la période du RMI qui a été endomagée lors de l'introduction du paramètre rsa_jeune.
    • Problème de type constaté lors de la mutiplication d'un scalaire égal à 1 avec des vecteurs numpy booléens révélé lors d'un usage au CASD (potentiellement dépendant du système d'exploiattion etc)

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

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

Quelques conseils à prendre en compte :

Et surtout, n'hésitez pas à demander de l'aide ! :)

@benjello benjello requested review from a team, guillett and Morendil January 8, 2019 13:05
@benjello
Copy link
Member Author

benjello commented Jan 8, 2019

@guillett @Morendil: j'ai introduit explicitement la gestion d'une période dans une formule et peut-être que cela peut-être fait autrement. Votre avis m'intéresse.

@guillett
Copy link
Member

guillett commented Jan 8, 2019

@benjello pourquoi ne pas créer une nouvelle variable avec false en valeur par défaut et une formule à partir de 2009-06 ?

@benjello
Copy link
Member Author

benjello commented Jan 8, 2019

@guillet, ce n'est pas inélégant mais je mets par défaut un gros coût sur la création de variable car

  • cela rend plus complexe le modèle
  • c'est coûteux en terme de mémoire

Mais comme tu as par ailleurs proposé des solutions (je ne retrouve pas de lien :-( ) pour mieux gérer la mémoire, c'est envisageable, à terme.

@Morendil
Copy link
Contributor

Morendil commented Jan 8, 2019

@benjello Une alternative: moduler le comportement par un paramètre (qui prendrait une valeur équivalente à true jusqu'à juin 2009 et false après).

@benjello
Copy link
Member Author

benjello commented Jan 8, 2019

@Morendil : j'ai failli faire cela mais il y a plusieurs problèmes qui m'ont empêché de le faire:

  • les paramètres ne prennent pas des booléens et je trouvais déjà un peu sale le paramètre rsa_jeune égal à 1 qui nous à conduit à un bug lors de multiplication de types différents
  • le paramètre concernerait à la fois le RMI et le RSA donc pas très lisible dans l'arbre des paramètres

Donc je serais même enclin à virer le paramètre rsa_jeune et être explicite dans la formule ...

@Morendil
Copy link
Contributor

Morendil commented Jan 9, 2019

@benjello Alors je me range à l'avis de @guillett.

@benjello
Copy link
Member Author

benjello commented Jan 9, 2019

@Morendil : même après ma réponse à @guillet visant à éviter de créer de nouvelle variable ?
Plus précisément m'autoriseriez-vous à merger tel quel ?

@Morendil
Copy link
Contributor

OK pour un hotfix suite aux soucis rencontrés sur les usages de l'IPP au CASD, et il est important qu'on prenne l'engagement

  • de revenir là-dessus plus tard (notamment la bascule RSA/RMI est à revoir)
  • de mieux caractériser ce qui a été KO au CASD pour savoir le reproduire en test dans l'intégration continue

@benjello benjello force-pushed the eligibilite-rsa-jeune branch from d8bb361 to 1008583 Compare January 10, 2019 14:23
@benjello benjello merged commit 17b18e5 into master Jan 10, 2019
@benjello benjello deleted the eligibilite-rsa-jeune branch January 10, 2019 15:08
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.

3 participants