publicodes / tools

Set of utility functions to write tooling for Publicodes models
https://publicodes.github.io/tools/
MIT License
8 stars 0 forks source link

pkg: upgrade publicodes to 1.0.0 (NGC-454) #34

Closed Clemog closed 8 months ago

Clemog commented 9 months ago

[!IMPORTANT] Pour l'instant toutes les règles dont un de leurs parents contient un mécanisme contexte ne sont pas optimisées. On passe donc de 962 règles à 1147 dans la version optimisée. Des améliorations pourraient être fait pour optimiser les règles enfants qui le peuvent mais pour l'instant je préfère opter pour la solution simple et qui assure que les résultats du calcul sont fiables (cf. le commentaire dans constantFolding.ts).

Clemog commented 9 months ago

@EmileRolley, j'ai l'impression qu'il y a un peu de taf au niveau de l'optim, je crois que l'AST a changé non?

Clemog commented 9 months ago

Je vois qu'il y un souci avec le fait qu'on ait plus accès aux variables traversées dans l'évaluation.. EDIT: c'est en fait une option désormais donc je pense que c'est facilement corrigeable pour le moment :)

Clemog commented 9 months ago

Note : L'import ne fonctionne plus car nom a disparu des mécanismes (cf https://github.com/publicodes/publicodes/pull/420) -> J'ai corrigé normalemnt

Clemog commented 9 months ago

J'ai essayé d'avancer au mieux !

Je penses qu'il y a un petit soucis avec les variables traversées, j'ai parfois cette erreur :

image

on en rediscute, j'ai l'impression que l'optim fonctionne pas encore sur le modèle NGC avec publiocdes v1 :)

Clemog commented 9 months ago

Pour l'instant toutes les règles dont un de leurs parents contient un mécanisme contexte ne sont pas optimisées. On passe donc de 962 règles à 1147 dans la version optimisée. Des améliorations pourraient être fait pour optimiser les règles enfants qui le peuvent mais pour l'instant je préfère opter pour la solution simple et qui assure que les résultats du calcul sont fiables (cf. le commentaire dans constantFolding.ts).

C'est pas à cause de la disparition de recalculNode dans l'AST ? On peut pas faire en sorte que ça reste ?

EmileRolley commented 9 months ago

Pour l'instant toutes les règles dont un de leurs parents contient un mécanisme contexte ne sont pas optimisées. On passe donc de 962 règles à 1147 dans la version optimisée. Des améliorations pourraient être fait pour optimiser les règles enfants qui le peuvent mais pour l'instant je préfère opter pour la solution simple et qui assure que les résultats du calcul sont fiables (cf. le commentaire dans constantFolding.ts).

C'est pas à cause de la disparition de recalculNode dans l'AST ? On peut pas faire en sorte que ça reste ?

Ce n'est pas du à la migration de publicodes. C'est juste que j'étais partis du principe que le recalcule (maintenant contexte) impactait une seule règle. C'est le cas dans le modèle de NGC où les contexte sont uniquement utilisés avec un valeur: <rule_name>. Or, cela n'est pas correct :

  1. le nouveau contexte s'applique à toutes les sous-étapes (sous-règles) nécessaires au calcul de la règle référencée avec le mécanisme valeur
  2. on peut également appliquer un nouveau contexte à une formule plus complexe impliquant plusieurs règles.

Le fait que l'on n'ait actuellement pas eu de problème avec l'optim revient d'un jeu de circonstances propres au modèle actuel de NGC mais n'est pas fiable pour un modèle arbitraire.

EmileRolley commented 8 months ago

closed in favor of #35