Skip to content

Keep min count of PDCs instead of none #14

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

Merged
merged 5 commits into from
Nov 16, 2024

Conversation

guiohm
Copy link
Contributor

@guiohm guiohm commented Oct 4, 2024

C'est à discuter. Je comprends qu'on préfère ne rien proposer plutôt qu'une valeur incorrecte.

Mon argument est de proposer une valeur par défaut conservatrice probablement juste plutôt que rien du tout.

Cependant l'idéal serait de fournir une liste de champs dont les valeurs sont "déduites" et de les signaler comme telles dans Osmose. (je peux m'occuper de ça avec les modifications correspondantes du côté Osmose)

@nlehuby
Copy link
Member

nlehuby commented Oct 5, 2024

hum 🤔 je ne sais pas trop...

J'avoue préférer ne rien mettre que de risquer de mettre une valeur fausse, surtout pour les attributs qui se relèvent facilement sur le terrain comme c'est le cas pour capacity qu'on remplit avec nbre_pdc.
Ajouter des infos dans OSM est généralement une opération simple, alors que challenger les données et corriger des erreurs nécessite plus d'expérience, donc je pense qu'on risque de créer plein d'erreurs potentielles.

Tu as d'autres champs en tête où on pourrait envisager un traitement de ce genre ?

@guiohm
Copy link
Contributor Author

guiohm commented Oct 5, 2024

Oui il y a les champs que je propose dans cette PR #15.
Notamment la puissance pour T2 :
Lorsqu'un PDC indique 50 kw avec 2 prises Chademo et T2, on sait que T2 ne peut pas dépasser 43 kw, donc les 50 kw concernent Chademo, mais le chargeur ne pourrait supporter le T2 qu'à 22 kw, donc la déduction pour T2 à 43 kw pourrait être fausse.
Ceci dit, c'est un problème courant de voir que la puissance affichée n'est pas vraiment celle disponible (exemple de chargeurs 150 kw qui brident à 70 lorsque 2 véhicules sont connectés).

@guiohm
Copy link
Contributor Author

guiohm commented Oct 5, 2024

En fait l'idée simple serait d'afficher pour ces champs dans Osmose <valeur> (déduit: à confirmer).

@nlehuby
Copy link
Member

nlehuby commented Oct 6, 2024

En fait l'idée simple serait d'afficher pour ces champs dans Osmose <valeur> (déduit: à confirmer).

Je ne suis pas sure de comprendre pas ce que tu veux faire : tu proposes d'ajouter une mention dans l'erreur remontée par Osmose avec la liste de tous les tags qui ont été déduits et sont peut-être faux ?

par exemple, pour cette erreur :
image

le commentaire deviendrait :
Car charging station not integrated

IZIVIA | MARSEILLE 07 - KENNEDY, 102 CORNICHE PDT JF KENNEDY 13007 MARSEILLE, Conditions accès aux bornes dépendantes des accords et formules de votre EMSP. Attention, les tags suivants ont été calculés et sont peut-être faux : capacity, socket:output:chademo et socket:output:type2_combo

C'est ça ?

À noter qu'il y a déjà une mention proche (affichée dans le panneau à droite de l'écran) :
image

Mais elle est générique (c'est la même pour tous les éléments remontée par l'analyse) et franchement, je pense que personne ne lit ce truc...

@guiohm
Copy link
Contributor Author

guiohm commented Oct 6, 2024

Et bien en fait j'ai notamment en tête ce cas là : https://osmose.openstreetmap.fr/en/issue/37223e0a-a4ff-65a1-f874-f17d358322c5
Dans lequel il n'est du coup carrément pas proposé de tag capacity.

Et effectivement j'avais zappé le message générique. J'utilise JOSM qui renvoie directement sur l'URL ci-dessus, et je scrolle tout en bas pour voir les infos. Et j'aimerai ajouter d'une manière ou d'une autre quels sont les champs "devinés" ou calculés qui ne sont pas directement ceux d'openData. ça faciliterait la vérification.

Alors ça peut être :

  • générer un message non générique qui liste ces champs
  • suffixer la valeur du champ par !GUESSED mais peut-être plus dangereux si quelqu'un importe et oublie d'enlever le suffixe
  • utiliser une autre colonne pour ces infos (j'ai pas encore vu le code, je ne sais pas si c'est envisageable)

J'ai également en tête d'autres champs comme par exemple dans certains cas on pourrait récupérer une station avec une ref:EU:EVSE invalide à confirmer (mais je dois analyser un peu plus d'abord).

@Marc-marc-marc
Copy link
Contributor

Marc-marc-marc commented Oct 6, 2024

Mon avis en tant qu'utilisateur de la donnée :

  • le nombre d'emplacement est un critère éliminatoire (dans ABRP par ex, par défaut on rejette des stations ayant moins de 4 charges simultanés lors des longs trajets). donc avoir l'info "4 ou +" ou ne pas avoir l'info, cela fait une grande différence.
    suggestion capacité=2+ (idée inspirée des horaires d'ouverture) pourrait dire clairement que c'est un minimum même si on ne sait pas la valeur exacte à cause d'une incohérence. Mais cela veux dire que SC et plein d'autres outils vont devoir gérer cette incertitude pour proposer de la rafiner (et espérer que pleins de contributeurs ne supprime pas le +purement et simplement... mais cela pourrait être un moyen pour osmose de savoir qu'il y a incertitude et ne pas proposer de remplacer capacité=3 par 2 si c'est 2+- une alternative est fixme=incohérence opendata sur capacity (généré ici ou au niveau osmose)
  • à l'inverse les suppositions sur la puissance max par prise me semble hasardeuse (sauf pour rejeter les valeurs erronées). Lorsqu'un PDC indique 50 kw, la meilleur chose à faire est charging_station:output=50 kW. Supposer que la prise type 2 fait 22 kW alors qu'il y a du 43 kW (très rare c'est vrai), c'est la roulette probabiliste.

@nlehuby
Copy link
Member

nlehuby commented Oct 8, 2024

J'utilise JOSM qui renvoie directement sur l'URL ci-dessus, et je scrolle tout en bas pour voir les infos.

tu sais que tu peux cliquer sur le lien fix-josm (présent dans la poup-up dans ma capture précédente par ex) et avoir un nouvel objet créé dans JOSM avec directement tous les tags proposés sans avoir à faire de copié collé ?

suffixer la valeur du champ par !GUESSED

L'option d'ajouter !GUESSED ou autre chose dans le tag est absolument à éviter. Je veux bien prendre les paris que si on fait ça, ça finit tel quel dans OSM.

@nlehuby
Copy link
Member

nlehuby commented Oct 8, 2024

Dans l'absolu, je pense qu'on devrait essayer de ne proposer que des trucs officiels ou dont on est surs.
L'utilisateur d'Osmose aura peut-être un regard critique sur la position et sur les noms, mais n'ira probablement pas plus loin. C'est d'ailleurs un des intérêts d'Osmose : pas besoin d'être un spécialiste des bornes de recharge, de savoir quel est le tag pour une prise CHAdeMO ni même ce qu'est un identifiant d'itinérance, de connaitre le schéma open data, etc
D'ailleurs, je me suis posée plusieurs la question de ne pas proposer des tags du tout et d'avoir une analyse qui dit juste "y a surement une borne ici" parce que les objets qu'on a dans le fichier correspondent trop mal aux objets qu'on veut dans OSM.

Donc bof pour essayer de deviner la capacité ou la puissance pour mettre ça dans Osmose derrière.

Par contre, ça ne me dérange pas qu'on fasse des savants calculs dans ce repo et qu'on génère des champs (_guessed au lieu de _grouped par exemple) qui pourraient être utilisés par des utilisateurs plus experts qui voudraient ajouter ces attributs en connaissance de cause.

@guiohm
Copy link
Contributor Author

guiohm commented Oct 9, 2024

tu sais que tu peux cliquer sur le lien fix-josm (présent dans la poup-up dans ma capture précédente par ex) et avoir un nouvel objet créé dans JOSM avec directement tous les tags proposés sans avoir à faire de copié collé ?

Oui je fais ça. D'où l'idée d'avoir plus de valeurs pré-remplies.

L'option d'ajouter !GUESSED ou autre chose dans le tag est absolument à éviter. Je veux bien prendre les paris que si on fait ça, ça finit tel quel dans OSM.

Humm, je ne vais pas parier là dessus :) fort probable hélas.

Par contre, ça ne me dérange pas qu'on fasse des savants calculs dans ce repo et qu'on génère des champs (_guessed au lieu de _grouped par exemple) qui pourraient être utilisés par des utilisateurs plus experts qui voudraient ajouter ces attributs en connaissance de cause.

Donc je propose de reverter ce commit => on ne propose pas de nb_pdc/capacity si les infos ne sont pas cohérentes.

Et d'ajouter un nouveau champ nbre_pdc_unsure ou nbre_pdc_tainted ou nbre_pdc_dirty ou nbre_pdc_guessed ? Ou mieux... J'ai la sensation que '_guessed' n'est pas approprié dans ce cas puisque ça vient quand même des données "officielles".

@guiohm
Copy link
Contributor Author

guiohm commented Oct 9, 2024

J'ai opté pour nbre_pdc_unsure dans mon dernier commit. Changeable évidemment :)

@Marc-marc-marc
Copy link
Contributor

c'est le min en fonction du nombre de prise ? de ref ?

@guiohm
Copy link
Contributor Author

guiohm commented Oct 11, 2024

Dans le fichier d'entrée, Pour un station_id, on a plusieurs pdc (lignes), et on a également la colonne nbre_pdc.
Il arrive que ces 2 valeurs ne correspondent pas, donc si on a 4 lignes et nbre_pdc = 2.
En sortie on aura :

  • nbre_pdc = 0 (valeur utilisée dans Osmose)
  • nb_prises_grouped = 4
  • [nouveau champ]nbre_pdc_unsure = 2 => min entre 2 et 4

@Marc-marc-marc
Copy link
Contributor

nbre_pdc = 0 (valeur utilisée dans Osmose)

pq 0 ? parce que incohérence ? ce serrait mieux de rien mettre dans ce cas
tu as un exemple oü on a des images ou des infos sur d'autres apps ? histoire de voir si ce ne serrait pas une confusion entre nombre de prise et nombre de place de chargement (une borne tristandard par exemple a 3 prises et pourrait avoir 3 ref mais qu'une place de chargement)

@guiohm
Copy link
Contributor Author

guiohm commented Oct 12, 2024

Actuellement, c'est mis à 0 parce qu'incohérence (c'est ce que je voudrais changer).
Ne rien mettre n'est pas possible car ce champ est requis (et doit être un entier) dans le schema IRVE. En revanche c'est à faire du côté Osmose oui (si nbre_pdc > 0 on utilise ce champ).

Il y a probablement plusieurs confusions possibles. Notamment dû à la hiérarchie station > borne > point de charge qui n'est pas correctement représentée dans les données data.gouv.
reference: https://afirev.fr/fr/definition-des-termes-de-la-mobilite-electrique/

Dans le schema IRVE, nbre_pdc est décrit comme "Le nombre de points de recharge sur la station."

Mais il n'est pas clair (pour moi en tout cas) si une ligne dans le csv issu de data.gouv correspond à une borne ou un point de charge. J'ai l'impression que c'est variable. Et j'ai l'impression que certains opérateurs peuvent interpreter nbre_pdc comme "Le nombre de point de recharge sur cette borne" et non pas pour la station.

Je n'ai pas d'exemple dont je sois sûr (i.e d'une station que je connais ave cette erreur), mais on peut regarder celui-ci (au hasard) :

id_station_itinerance|id_pdc_itinerance|coordonneesXY          |nbre_pdc|datagouv_organization_or_owner|puissance_nominale|prise_type_ef|prise_type_2|prise_type_combo_ccs|prise_type_chademo|prise_type_autre|gratuit|date_mise_en_service|date_maj  |cable_t2_attache|nom_station           |adresse_station                        |id_pdc_local  |
---------------------+-----------------+-----------------------+--------+------------------------------+------------------+-------------+------------+--------------------+------------------+----------------+-------+--------------------+----------+----------------+----------------------+---------------------------------------+--------------+
FRG10P43268A         |ETOADE000201     |[4.1161033, 45.1437823]|       1|e-totem                       |              22.0|true         |true        |false               |false             |false           |true   |2020-12-04          |2024-08-21|                |Intermarché Yssingeaux|195 Route de Retournac 43200 YSSINGEAUX|ETOADE000201  |
FRG10P43268A         |FRG10E43268A11   |[4.1161033, 45.1437823]|       2|e-totem                       |              22.0|false        |true        |false               |false             |false           |false  |2020-12-09          |2024-08-21|                |Intermarché Yssingeaux|195 Route de Retournac 43200 YSSINGEAUX|FRG10E43268A11|
FRG10P43268A         |FRG10E43268A12   |[4.1161033, 45.1437823]|       2|e-totem                       |              22.0|true         |true        |false               |false             |false           |false  |2020-12-09          |2024-08-21|                |Intermarché Yssingeaux|195 Route de Retournac 43200 YSSINGEAUX|FRG10E43268A12|

Ici quand on compare le nb de lignes et nbre_pdc ça ne colle pas, donc on met 0 en sortie, ce qui est à mon sens un peu radical.
Mon avis est que 0 est faux dans tous les cas, la valeur minimum entre les 2 (ici 2) est une bonne valeur par défaut (soit juste, soit sous-estimée). Dans cet exemple, probablement sous-estimée.
Donc proposer une valeur par défaut à la création dans Osmose me parait une bonne amélioration.

Une fois le nouveau champ dispo, je pourrai proposer une PR coté Osmose (qui pour le moment ne fait que proposer nbr_pdc).

Au final cette PR ne résout pas le problème de fond, mais améliore ce cas de figure (qui concerne plus de 9000 stations).

@nlehuby
Copy link
Member

nlehuby commented Oct 13, 2024

Dans le fichier d'entrée, Pour un station_id, on a plusieurs pdc (lignes), et on a également la colonne nbre_pdc. Il arrive que ces 2 valeurs ne correspondent pas, donc si on a 4 lignes et nbre_pdc = 2. En sortie on aura :

* `nbre_pdc = 0` (valeur utilisée dans Osmose)

* `nb_prises_grouped = 4`

* [nouveau champ]`nbre_pdc_unsure = 2` => min entre 2 et 4

Euh non, attention, il ne fait jamais mettre nbre_pdc à 0, même en cas d'incohérence car ça créairait un tag capacity=0 dans OSM, ce qui n'aurait pas de sens.
Il faut le mettre à None, afin qu'il ne soit pas proposé par Osmose.

@nlehuby
Copy link
Member

nlehuby commented Oct 13, 2024

J'ai l'impression qu'à moins de connaitre la configuration en bornes de la station (et donc de déjà savoir quelle est le bon tag capacity à utiliser) c'est impossible de faire qqch d'utile avec ces info contradictoires ... Mais bon, je n'ai pas de contrindication à ajouter un attribut nbre_pdc_unsure non utilisé par Osmose.

@guiohm
Copy link
Contributor Author

guiohm commented Oct 13, 2024

Euh non, attention, il ne fait jamais mettre nbre_pdc à 0, même en cas d'incohérence car ça créairait un tag capacity=0 dans OSM, ce qui n'aurait pas de sens.
Il faut le mettre à None, afin qu'il ne soit pas proposé par Osmose.

Oups, désolé. Dans ma tête c'était 0, mais c'est bien None dans le code.

@nlehuby nlehuby merged commit 1ab43d8 into Jungle-Bus:master Nov 16, 2024
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