230 extending khiops entries with a lightweight json structure for scenarios

[marcboulle]

Tout est implémenté et testé: cf. commit notes

[lucaurelien]

Meets requirements. Functionally Approved.

[popescu-v]

• A couple of comments on the Python code (see in the review)
• Some comments on the spec (https://github.com/KhiopsML/khiops/wiki/Feature-V11:-Extension-du-pilotage-de-Khiops-via-les-sc%C3%A9narios):
  • Regarding the "quoted-printable" (QuoPri) format:
    • to my understanding, QuoPri is optimized for readability and was designed for data blobs where there are relatively few non-printable characters (e.g. in e-mails), while Base64 is designed for being more compact when there are lots of such non-printable characters (e.g. in non-textual files such as raw data, images etc)
    • hence, I think that the QuoPri alternative vs Base64 could also be discussed in light of the kinds of data that we would need to keep as bytes, in practice: if Base64 is used for bytes when we need to be "agnostic" about the encoding of the data (such as when we mix several encodings), then it seems that we cannot make any assumption about the ratio of non-printable characters with respect to the payload of the value in question, right? If this is true, then perhaps we could take out QuoPri from the spec?
  • regarding the decision of streaming the reading of the scenario file, as opposed to the JSON file which is read in-memory at once: at first sight, I would imagine the JSON file as being "bigger" (as it can contain rather big arrays for LOOPs), while the scenario file could be quite compact, at least in the "template" form, if used for a specific "task". Was this decision made to support iterative instantiation of a scenario template from potentially several JSON files, applied in a sequence on the scenario template?

[marcboulle]

Concernant le choix d'encodage Base64, comme indiqué, c'est essentiellement parce qu'il s'agit d'un encodage plus populaire.
L'alternative QuoPri n'est ici qu'évoquée comme un choix alternatif: cela ne fait plus partie de la spec, et ne sera pas documenté dans la documentation utilisateur.

Concernant le choix de streamer ou non le json et le scénario:

• il faut avoir des limites
  • pour éviter les erreur utilisateur (paramétrage par des fichiers énormes erronés, faisant potentiellement planter l'outil)
  • pour éviter de gérer de la mémoire massive, au détriment de la mémoire nécessaire pour les traitements de data mining
• chargement du json en mémoire
  • on limite la taille des json à 1 Mb, ce qui parait énorme vis-a-vis des besoins, mais reste négligeable par rapport à la gestion de la mémoire pour les traitements de data mining
  • il est nécessaire d'avoir toute la structure json en mémoire, car chaque clé peut être utilisée de façon aléatoire à toute ligne d'un scénario
  • le parsing d'un json est facile quand on charge tout; ce serait nettement plus complexe à gérer en stream
• chargement des scénarios en stream
  • ce n'était pas vraiment indispensable: on aurait pu se limiter à une taille max
    • mais quelle taille max: toute transformation (par -O) d'un scénario avec paramétrage json résulte en un scénario plus gros (donc dépassant potentiellement alors sa taille max ...)
  • contrairement au json (dictionnaire de paramètres), un scenario est une suite séquentielle d'instructions, ce qui se prête naturellement à une gestion en stream
    • seule exception: le nombre de lignes (non vides) d'une structure de contrôle (IF ou LOOP), que l'on charge intégralement en mémoire
    • mais dans ce cas, on a limité ce nombre de ligne à 1000

[popescu-v]

Concernant le choix d'encodage Base64, comme indiqué, c'est essentiellement parce qu'il s'agit d'un encodage plus populaire. L'alternative QuoPri n'est ici qu'évoquée comme un choix alternatif: cela ne fait plus partie de la spec, et ne sera pas documenté dans la documentation utilisateur.

Concernant le choix de streamer ou non le json et le scénario:

* il faut avoir des limites
  
  * pour éviter les erreur utilisateur (paramétrage par des fichiers énormes erronés, faisant potentiellement planter l'outil)
  * pour éviter de gérer de la mémoire massive, au détriment de la mémoire nécessaire pour les traitements de data mining

* chargement du json en mémoire
  
  * on limite la taille des json à 1 Mb, ce qui parait énorme vis-a-vis des besoins, mais reste négligeable par rapport à la gestion de la mémoire pour les traitements de data mining
  * il est nécessaire d'avoir toute la structure json en mémoire, car chaque clé peut être utilisée de façon aléatoire à toute ligne d'un scénario
  * le parsing d'un json est facile quand on charge tout; ce serait nettement plus complexe à gérer en stream

* chargement des scénarios en stream
  
  * ce n'était pas vraiment indispensable: on aurait pu se limiter à une taille max
    
    * mais quelle taille max: toute transformation (par -O) d'un scénario avec paramétrage json résulte en un scénario plus gros (donc dépassant potentiellement alors sa taille max ...)
  * contrairement au json (dictionnaire de paramètres), un scenario est une suite séquentielle d'instructions, ce qui se prête naturellement à une gestion en stream
    
    * seule exception: le nombre de lignes (non vides) d'une structure de contrôle (IF ou LOOP), que l'on charge intégralement en mémoire
    * mais dans ce cas, on a limité ce nombre de ligne à 1000

Okay, I get the argument that, unlike the JSON format, scenario template structure (except the LOOPs) warrants a correct streamed reading.

[popescu-v]

LGTM:

• from a functional standpoint.
• the Python code.

Update the spec with:

• deletion of the QuoPri format mention
• statement of the limits for:
  • the size of the JSON file (1 MB)
  • the number of lines of a LOOP structure (1000).