fix(ml,backend): corrige la revue, le typage du drapeau CSV et la portée du biais
`load_from_csv` gardait un `astype(bool)` sur `is_working_hours`, joué avant `_typer` : une case vide du CSV arrivait en `NaN` et en ressortait `True`, soit une heure ouvrée inventée. Le chemin base était corrigé, pas celui-ci, et rien ne le couvrait. La ligne disparaît, et `_typer` ramène désormais les colonnes de `FLAG_COLUMNS` à `float64` quel que soit le contenu lu : sans cela le dtype dépendait de l'écriture du fichier (`0`/`1` contre `True`/`False`) et de la présence d'un trou, et l'égalité de schéma entre les deux chargeurs que promet ML-START n'était vraie que par accident du jeu de test. `Seuils.seuil_biais` valait `0` et `_verdict` exigeait `> 0` : la règle était inerte partout, CLI et DAG compris, et aucun test ne l'exerçait. Elle reste désactivée par défaut, parce qu'un seuil en kWh ne se transpose pas d'un bureau de 10 kWh à une usine de 1 000 kWh et qu'aucune valeur n'a été calibrée sur la vraie série, mais `--bias-threshold` la rend atteignable et l'ADR 0011 porte l'arbitrage. Trois tests couvrent le chemin : inerte par défaut, dérive au-delà du seuil réglé, et priorité de la MAE sur le biais. Deux lignes de doc devenues fausses au passage : la signature de `load_recent_from_database` dans ML-START, qui omettait `until` devenu obligatoire, et la ligne `bias` de 20-backend, qui laissait croire que la métrique décide du verdict. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
f21a843fc2
commit
81c8a67af3
@@ -65,6 +65,15 @@ def parse_args(argv: list[str] | None = None) -> argparse.Namespace:
|
|||||||
default=defauts.min_observations,
|
default=defauts.min_observations,
|
||||||
help="En deçà, le verdict est `indetermine` plutôt qu'un chiffre trompeur.",
|
help="En deçà, le verdict est `indetermine` plutôt qu'un chiffre trompeur.",
|
||||||
)
|
)
|
||||||
|
parser.add_argument(
|
||||||
|
"--bias-threshold",
|
||||||
|
type=float,
|
||||||
|
default=defauts.seuil_biais,
|
||||||
|
help=(
|
||||||
|
"Biais absolu en kWh au-delà duquel le verdict bascule en dérive. "
|
||||||
|
"Zéro, le défaut, laisse le biais informatif : voir l'ADR 0011."
|
||||||
|
),
|
||||||
|
)
|
||||||
parser.add_argument(
|
parser.add_argument(
|
||||||
"--fail-on-drift",
|
"--fail-on-drift",
|
||||||
action="store_true",
|
action="store_true",
|
||||||
@@ -78,6 +87,7 @@ def seuils_depuis(args: argparse.Namespace) -> Seuils:
|
|||||||
fenetre=timedelta(hours=args.window_hours),
|
fenetre=timedelta(hours=args.window_hours),
|
||||||
grace=timedelta(hours=args.grace_hours),
|
grace=timedelta(hours=args.grace_hours),
|
||||||
min_observations=args.min_observations,
|
min_observations=args.min_observations,
|
||||||
|
seuil_biais=args.bias_threshold,
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -41,6 +41,8 @@ class Seuils:
|
|||||||
min_observations: int = 24
|
min_observations: int = 24
|
||||||
ratio_derive: float = 1.25
|
ratio_derive: float = 1.25
|
||||||
mae_plancher: float = 0.0
|
mae_plancher: float = 0.0
|
||||||
|
# Un biais se compte en kWh, donc ne se transpose pas d'un site à l'autre : zéro le désactive,
|
||||||
|
# sans cesser de le mesurer. Réglé par `--bias-threshold`, arbitrage dans l'ADR 0011.
|
||||||
seuil_biais: float = 0.0
|
seuil_biais: float = 0.0
|
||||||
seuil_couverture: float = 0.8
|
seuil_couverture: float = 0.8
|
||||||
|
|
||||||
|
|||||||
@@ -218,3 +218,54 @@ async def test_drift_compares_the_recent_window_to_the_reference_one(
|
|||||||
rapports = await service(depot, min_observations=10, mae_plancher=1.0).evaluate(now=INSTANT)
|
rapports = await service(depot, min_observations=10, mae_plancher=1.0).evaluate(now=INSTANT)
|
||||||
|
|
||||||
assert next(r for r in rapports if r.site_id is None).status == attendu
|
assert next(r for r in rapports if r.site_id is None).status == attendu
|
||||||
|
|
||||||
|
|
||||||
|
async def test_drift_leaves_the_bias_out_of_the_verdict_by_default() -> None:
|
||||||
|
# Le modèle surestime de 3 kWh à chaque heure, et le verdict reste `stable` : le biais est
|
||||||
|
# mesuré et servi, il ne juge pas tant que `--bias-threshold` n'a pas été réglé (ADR 0011).
|
||||||
|
depot = FauxDepot(
|
||||||
|
recentes=paires(nombre=30, prevu=13.0, reel=10.0),
|
||||||
|
anciennes=paires(nombre=30, prevu=13.0, reel=10.0),
|
||||||
|
comptages=[ComptageStatut(site_id="SITE001", status="available", nombre=30)],
|
||||||
|
)
|
||||||
|
|
||||||
|
rapports = await service(depot, min_observations=10).evaluate(now=INSTANT)
|
||||||
|
|
||||||
|
global_ = next(rapport for rapport in rapports if rapport.site_id is None)
|
||||||
|
assert global_.status == STATUT_STABLE
|
||||||
|
assert global_.bias == 3.0
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize(
|
||||||
|
("prevu", "attendu"),
|
||||||
|
[(13.0, STATUT_DERIVE), (11.0, STATUT_STABLE)],
|
||||||
|
ids=["biais_au_dela", "biais_sous_le_seuil"],
|
||||||
|
)
|
||||||
|
async def test_drift_reports_derive_on_the_bias_once_a_threshold_is_set(
|
||||||
|
prevu: float, attendu: str
|
||||||
|
) -> None:
|
||||||
|
# MAE récente et MAE de référence sont égales : seul le biais peut faire basculer le verdict.
|
||||||
|
depot = FauxDepot(
|
||||||
|
recentes=paires(nombre=30, prevu=prevu, reel=10.0),
|
||||||
|
anciennes=paires(nombre=30, prevu=prevu, reel=10.0),
|
||||||
|
comptages=[ComptageStatut(site_id="SITE001", status="available", nombre=30)],
|
||||||
|
)
|
||||||
|
|
||||||
|
rapports = await service(depot, min_observations=10, seuil_biais=2.0).evaluate(now=INSTANT)
|
||||||
|
|
||||||
|
global_ = next(rapport for rapport in rapports if rapport.site_id is None)
|
||||||
|
assert global_.status == attendu
|
||||||
|
|
||||||
|
|
||||||
|
async def test_drift_prefers_the_mae_reason_when_both_the_mae_and_the_bias_exceed() -> None:
|
||||||
|
depot = FauxDepot(
|
||||||
|
recentes=paires(nombre=30, prevu=20.0, reel=10.0),
|
||||||
|
anciennes=paires(nombre=30, prevu=11.0, reel=10.0),
|
||||||
|
comptages=[ComptageStatut(site_id="SITE001", status="available", nombre=30)],
|
||||||
|
)
|
||||||
|
|
||||||
|
rapports = await service(depot, min_observations=10, seuil_biais=2.0).evaluate(now=INSTANT)
|
||||||
|
|
||||||
|
global_ = next(rapport for rapport in rapports if rapport.site_id is None)
|
||||||
|
assert global_.status == STATUT_DERIVE
|
||||||
|
assert "MAE" in (global_.reason or "")
|
||||||
|
|||||||
@@ -106,3 +106,11 @@ def test_main_exits_zero_when_drift_is_detected_without_the_flag(
|
|||||||
|
|
||||||
assert code == 0
|
assert code == 0
|
||||||
assert capsys.readouterr().out != ""
|
assert capsys.readouterr().out != ""
|
||||||
|
|
||||||
|
|
||||||
|
def test_parse_args_leaves_the_bias_threshold_disabled_by_default() -> None:
|
||||||
|
assert cli.parse_args([]).bias_threshold == 0.0
|
||||||
|
|
||||||
|
|
||||||
|
def test_seuils_depuis_carries_the_bias_threshold() -> None:
|
||||||
|
assert cli.seuils_depuis(cli.parse_args(["--bias-threshold", "2.5"])).seuil_biais == 2.5
|
||||||
|
|||||||
+1
-1
@@ -25,7 +25,7 @@ Le choix du modèle est dans l'ADR 0005. Ce document ne les répète pas.
|
|||||||
|---|---|---|
|
|---|---|---|
|
||||||
| `load_from_csv(path)` | `ml/data/all_sites_combined.csv` | Chemin de démarrage, tant que la base n'est pas peuplée |
|
| `load_from_csv(path)` | `ml/data/all_sites_combined.csv` | Chemin de démarrage, tant que la base n'est pas peuplée |
|
||||||
| `load_from_database(connection)` | `reading` joint à `site`, **historique complet** | Entraînement |
|
| `load_from_database(connection)` | `reading` joint à `site`, **historique complet** | Entraînement |
|
||||||
| `load_recent_from_database(connection, since=…)` | `reading` joint à `site`, **borné par `since`** | Scoring |
|
| `load_recent_from_database(connection, since=…, until=…)` | `reading` joint à `site`, **borné des deux côtés** | Scoring |
|
||||||
|
|
||||||
L'égalité des schémas n'est pas un confort : c'est ce qui permet de valider tout le pipeline sur
|
L'égalité des schémas n'est pas un confort : c'est ce qui permet de valider tout le pipeline sur
|
||||||
CSV, sans base joignable, et d'obtenir le même comportement une fois la base peuplée. Une
|
CSV, sans base joignable, et d'obtenir le même comportement une fois la base peuplée. Une
|
||||||
|
|||||||
@@ -97,6 +97,13 @@ modèle change n'est pas une dérive, c'est une régression de réentraînement.
|
|||||||
l'identique.
|
l'identique.
|
||||||
- La CLI sort en code non nul sous `--fail-on-drift` seulement. Par défaut, constater une dérive
|
- La CLI sort en code non nul sous `--fail-on-drift` seulement. Par défaut, constater une dérive
|
||||||
n'est pas un échec d'exécution.
|
n'est pas un échec d'exécution.
|
||||||
|
- **Le biais ne fait pas basculer le verdict par défaut** : `Seuils.seuil_biais` vaut `0`, ce qui
|
||||||
|
désactive la règle. Le plafond de MAE se dérive de la fenêtre de référence, donc il vaut pour
|
||||||
|
n'importe quel site ; un seuil de biais, lui, s'exprime en kWh et ne se transpose pas d'un
|
||||||
|
bureau de 10 kWh à une usine de 1 000 kWh. En déclarer un sans l'avoir calibré sur la vraie
|
||||||
|
série ferait rougir la tâche sans rien prouver. Le `bias` signé reste calculé, stocké et servi
|
||||||
|
par `GET /api/v1/monitoring/drift` : il se lit, il ne juge pas encore. `--bias-threshold`
|
||||||
|
l'active site par site quand une valeur aura été mesurée.
|
||||||
|
|
||||||
## Effet de bord assumé sur le pipeline
|
## Effet de bord assumé sur le pipeline
|
||||||
|
|
||||||
|
|||||||
@@ -235,7 +235,7 @@ n'ajoute rien.
|
|||||||
| Métrique | Ce qu'elle dit |
|
| Métrique | Ce qu'elle dit |
|
||||||
|---|---|
|
|---|---|
|
||||||
| `mae` | Erreur moyenne en kWh, la métrique même qu'optimise LightGBM |
|
| `mae` | Erreur moyenne en kWh, la métrique même qu'optimise LightGBM |
|
||||||
| `bias` | Erreur moyenne **signée** : c'est elle qui distingue un modèle plus bruyant d'un modèle qui se trompe systématiquement du même côté |
|
| `bias` | Erreur moyenne **signée** : c'est elle qui distingue un modèle plus bruyant d'un modèle qui se trompe systématiquement du même côté. Lue et servie, elle ne fait basculer le verdict que sous `--bias-threshold`, faute d'un seuil en kWh transposable d'un site à l'autre ([ADR 0011](../adr/0011-surveillance-de-derive-dans-le-backend.md)) |
|
||||||
| `mape` | Comparable entre sites de tailles différentes, hors réalisés nuls |
|
| `mape` | Comparable entre sites de tailles différentes, hors réalisés nuls |
|
||||||
| `coverage_ratio` | Part des prévisions disponibles qui ont trouvé leur réalisé : mesure le pipeline, pas le modèle |
|
| `coverage_ratio` | Part des prévisions disponibles qui ont trouvé leur réalisé : mesure le pipeline, pas le modèle |
|
||||||
| `insufficient_data_ratio` | Part des sites privés d'historique suffisant |
|
| `insufficient_data_ratio` | Part des sites privés d'historique suffisant |
|
||||||
|
|||||||
@@ -43,8 +43,8 @@ NUMERIC_COLUMNS = [
|
|||||||
"capacity_kw",
|
"capacity_kw",
|
||||||
]
|
]
|
||||||
|
|
||||||
# Piege : `reading.is_working_hours` est nullable et entre dans les features. Une seule lecture a
|
# Piege : `is_working_hours` est nullable et entre dans les features. Toujours `float64`, jamais
|
||||||
# NULL rend la colonne `object`, que LightGBM refuse ("pandas dtypes must be int, float or bool").
|
# `bool` : `astype(bool)` ferait un `True` d'une absence, et les deux chargeurs divergeraient.
|
||||||
FLAG_COLUMNS = ["is_working_hours"]
|
FLAG_COLUMNS = ["is_working_hours"]
|
||||||
|
|
||||||
_READING_QUERY = text(
|
_READING_QUERY = text(
|
||||||
@@ -113,10 +113,15 @@ def load_recent_from_database(
|
|||||||
|
|
||||||
|
|
||||||
def load_from_csv(csv_path: Path) -> pd.DataFrame:
|
def load_from_csv(csv_path: Path) -> pd.DataFrame:
|
||||||
"""Lit le jeu de donnees CSV historique (chemin de demarrage, hors base)."""
|
"""Lit le jeu de donnees CSV historique (chemin de demarrage, hors base).
|
||||||
|
|
||||||
|
`is_working_hours` passe par `_typer` comme le chemin base, et non par un `astype(bool)` : le
|
||||||
|
fichier livre porte cette colonne en `0`/`1`, donc une case vide arrive en `NaN` et `astype`
|
||||||
|
la rendrait `True` sans rien signaler. Les deux chargeurs rendent ainsi le meme schema, ce que
|
||||||
|
`docs/ML-START.md` promet.
|
||||||
|
"""
|
||||||
frame = pd.read_csv(csv_path, parse_dates=["timestamp"])
|
frame = pd.read_csv(csv_path, parse_dates=["timestamp"])
|
||||||
frame["capacity_kw"] = float("nan")
|
frame["capacity_kw"] = float("nan")
|
||||||
frame["is_working_hours"] = frame["is_working_hours"].astype(bool)
|
|
||||||
|
|
||||||
return _typer(frame[OUTPUT_COLUMNS])
|
return _typer(frame[OUTPUT_COLUMNS])
|
||||||
|
|
||||||
@@ -131,12 +136,18 @@ def _typer(frame: pd.DataFrame) -> pd.DataFrame:
|
|||||||
n'importe quelle autre colonne mesuree entierement absente sur une fenetre de scoring, pas
|
n'importe quelle autre colonne mesuree entierement absente sur une fenetre de scoring, pas
|
||||||
seulement `capacity_kw`.
|
seulement `capacity_kw`.
|
||||||
|
|
||||||
|
Les colonnes de `FLAG_COLUMNS` sont en outre ramenees a `float64` : ce sont des drapeaux
|
||||||
|
nullables, et c'est le seul dtype qui survive a l'absence sans inventer de valeur. Sans cela,
|
||||||
|
le meme chargeur rendrait `bool`, `int64` ou `float64` selon le contenu de la fenetre lue.
|
||||||
|
|
||||||
Piege additionnel : `NUMERIC_COLUMNS` inclut `consumption_kwh`, la cible du modele, pas
|
Piege additionnel : `NUMERIC_COLUMNS` inclut `consumption_kwh`, la cible du modele, pas
|
||||||
seulement des variables explicatives. Une valeur non numerique y devient donc silencieusement
|
seulement des variables explicatives. Une valeur non numerique y devient donc silencieusement
|
||||||
`NaN` aussi bien a l'entrainement (ou `train.py` l'exclura ensuite via son `dropna`) qu'au
|
`NaN` aussi bien a l'entrainement (ou `train.py` l'exclura ensuite via son `dropna`) qu'au
|
||||||
scoring -- ce n'est pas un effet de bord limite aux colonnes mesurees.
|
scoring -- ce n'est pas un effet de bord limite aux colonnes mesurees.
|
||||||
"""
|
"""
|
||||||
typee = frame.copy()
|
typee = frame.copy()
|
||||||
for colonne in (*NUMERIC_COLUMNS, *FLAG_COLUMNS):
|
for colonne in NUMERIC_COLUMNS:
|
||||||
typee[colonne] = pd.to_numeric(typee[colonne], errors="coerce")
|
typee[colonne] = pd.to_numeric(typee[colonne], errors="coerce")
|
||||||
|
for colonne in FLAG_COLUMNS:
|
||||||
|
typee[colonne] = pd.to_numeric(typee[colonne], errors="coerce").astype("float64")
|
||||||
return typee
|
return typee
|
||||||
|
|||||||
@@ -1,6 +1,7 @@
|
|||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
|
|
||||||
import pandas as pd
|
import pandas as pd
|
||||||
|
import pytest
|
||||||
|
|
||||||
from enervision_ml.data import NUMERIC_COLUMNS, load_from_csv
|
from enervision_ml.data import NUMERIC_COLUMNS, load_from_csv
|
||||||
|
|
||||||
@@ -53,3 +54,44 @@ def test_load_from_csv_always_types_capacity_kw_as_float(tmp_path: Path) -> None
|
|||||||
|
|
||||||
assert frame["capacity_kw"].dtype == "float64"
|
assert frame["capacity_kw"].dtype == "float64"
|
||||||
assert pd.isna(frame["capacity_kw"].iloc[0])
|
assert pd.isna(frame["capacity_kw"].iloc[0])
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize("present", ["1", "True"], ids=["entier", "booleen_textuel"])
|
||||||
|
def test_load_from_csv_keeps_a_missing_is_working_hours_as_nan(
|
||||||
|
tmp_path: Path, present: str
|
||||||
|
) -> None:
|
||||||
|
# Une case vide vaut "on ne sait pas", que LightGBM sait traiter. La rendre `True` inventerait
|
||||||
|
# une heure ouvree, et le modele apprendrait sur une valeur que personne n'a mesuree.
|
||||||
|
csv_path = write_csv(
|
||||||
|
tmp_path,
|
||||||
|
f"SITE001,2026-01-01T00:00:00,10.5,15.0,50.0,0.0,{present},office",
|
||||||
|
"SITE001,2026-01-01T01:00:00,11.5,15.2,50.5,0.0,,office",
|
||||||
|
)
|
||||||
|
|
||||||
|
frame = load_from_csv(csv_path)
|
||||||
|
|
||||||
|
assert frame["is_working_hours"].iloc[0] == 1
|
||||||
|
assert pd.isna(frame["is_working_hours"].iloc[1])
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize(
|
||||||
|
"valeurs",
|
||||||
|
[("1", "0"), ("True", "False")],
|
||||||
|
ids=["entier", "booleen_textuel"],
|
||||||
|
)
|
||||||
|
def test_load_from_csv_always_types_is_working_hours_as_float(
|
||||||
|
tmp_path: Path, valeurs: tuple[str, str]
|
||||||
|
) -> None:
|
||||||
|
# Le dtype ne doit pas dependre de l'ecriture du fichier ni de la presence d'un trou : c'est
|
||||||
|
# ce qui rend comparable le schema des deux chargeurs, cf. `test_data_integration.py`.
|
||||||
|
present, absent = valeurs
|
||||||
|
csv_path = write_csv(
|
||||||
|
tmp_path,
|
||||||
|
f"SITE001,2026-01-01T00:00:00,10.5,15.0,50.0,0.0,{present},office",
|
||||||
|
f"SITE001,2026-01-01T01:00:00,11.5,15.2,50.5,0.0,{absent},office",
|
||||||
|
)
|
||||||
|
|
||||||
|
frame = load_from_csv(csv_path)
|
||||||
|
|
||||||
|
assert frame["is_working_hours"].dtype == "float64"
|
||||||
|
assert list(frame["is_working_hours"]) == [1.0, 0.0]
|
||||||
|
|||||||
@@ -142,6 +142,21 @@ def test_load_recent_from_database_types_a_null_is_working_hours_as_float64(
|
|||||||
assert list(frame["is_working_hours"].isna()) == [True, False]
|
assert list(frame["is_working_hours"].isna()) == [True, False]
|
||||||
|
|
||||||
|
|
||||||
|
def test_load_recent_from_database_types_is_working_hours_as_float64_even_without_a_null(
|
||||||
|
connexion_ml: Connection,
|
||||||
|
) -> None:
|
||||||
|
# Sans cette garantie, le dtype dependrait du contenu de la fenetre lue : `bool` ici, `float64`
|
||||||
|
# des qu'une seule lecture est a NULL, et le schema des deux chargeurs cesserait d'etre egal.
|
||||||
|
site_id = insere_site(connexion_ml)
|
||||||
|
insere_lectures(connexion_ml, site_id, heures=2, fin=ANCRAGE)
|
||||||
|
|
||||||
|
frame = load_recent_from_database(
|
||||||
|
connexion_ml, since=ANCRAGE - timedelta(hours=2), until=ANCRAGE
|
||||||
|
)
|
||||||
|
|
||||||
|
assert frame["is_working_hours"].dtype == "float64"
|
||||||
|
|
||||||
|
|
||||||
def test_both_loaders_produce_the_same_columns_in_the_same_order(
|
def test_both_loaders_produce_the_same_columns_in_the_same_order(
|
||||||
connexion_ml: Connection, tmp_path: Path
|
connexion_ml: Connection, tmp_path: Path
|
||||||
) -> None:
|
) -> None:
|
||||||
|
|||||||
Reference in New Issue
Block a user