mirror of
https://github.com/Hestia-Homes/Model.git
synced 2026-07-27 23:35:01 +00:00
Flag a modelling baseline that disagrees with the persisted effective 🟩
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
31edd01999
commit
8c2c19495c
3 changed files with 71 additions and 4 deletions
|
|
@ -57,6 +57,12 @@ from repositories.unit_of_work import UnitOfWork
|
|||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
# How far Modelling's continuous re-score may sit from the persisted Effective
|
||||
# SAP before it counts as a divergence. The persisted figure is the *rounded*
|
||||
# integer, so up to half a point is rounding alone; beyond a full point the two
|
||||
# stages are scoring different pictures (#1655).
|
||||
_MAX_BASELINE_DIVERGENCE: Final[float] = 1.0
|
||||
|
||||
# Best-practice install sequence for the role-3 attribution cascade (ADR-0016):
|
||||
# walls → roof → ventilation → floor, per the legacy `Recommendations` class.
|
||||
# Ventilation sits after the fabric that triggers it so its (negative) marginal
|
||||
|
|
@ -130,6 +136,7 @@ class ModellingOrchestrator:
|
|||
uow.solar, prop.identity.uprn
|
||||
)
|
||||
has_recommendations = False
|
||||
checked_baseline = False
|
||||
for scenario in scenarios:
|
||||
plan = self._plan_for(
|
||||
scorer,
|
||||
|
|
@ -142,6 +149,17 @@ class ModellingOrchestrator:
|
|||
solar_potential=solar_potential,
|
||||
considered_measures=considered_measures,
|
||||
)
|
||||
# The baseline is the same picture for every Scenario, so
|
||||
# check it once per Property — and off the Plan's own
|
||||
# baseline Score, which is already computed, rather than
|
||||
# paying for another whole-dwelling re-score per Property.
|
||||
if not checked_baseline:
|
||||
_check_baseline_coherence(
|
||||
property_id,
|
||||
plan.baseline.sap_continuous,
|
||||
uow.property_baseline.get_for_property(property_id),
|
||||
)
|
||||
checked_baseline = True
|
||||
uow.plan.save(
|
||||
plan,
|
||||
property_id=property_id,
|
||||
|
|
@ -500,7 +518,47 @@ def _check_baseline_coherence(
|
|||
modelled_baseline_sap: float,
|
||||
persisted: Optional[PropertyBaselinePerformance],
|
||||
) -> None:
|
||||
raise NotImplementedError
|
||||
"""Assert that Modelling and the Baseline stage agree on what the Property
|
||||
scores now, and say so loudly when they do not (ADR-0002, #1655).
|
||||
|
||||
Modelling re-scores the Effective EPC rather than reading the persisted
|
||||
Effective Performance, so the two can drift apart. The app treats the
|
||||
persisted figure as the Property's current state and reads a Plan's `post_*`
|
||||
against it, so a drift is not cosmetic: it surfaces to the user as a gain
|
||||
nobody modelled (property 749719 reported "+9.65 SAP" on a plan whose own
|
||||
CO2, bill and consumption savings were all exactly 0.0).
|
||||
|
||||
We flag rather than anchor deliberately. Overwriting the Plan's SAP with the
|
||||
persisted integer would make it disagree with its own CO2 / primary energy /
|
||||
bill, which all come off the same SapResult — trading one incoherence for
|
||||
another, and hiding the drift instead of surfacing it. A divergence here is
|
||||
a real defect upstream (see #1656) and should be investigated, not papered
|
||||
over.
|
||||
|
||||
A missing row is a data gap, not a modelling fault: warn and carry on, so it
|
||||
degrades one Property rather than aborting the batch (ADR-0012 reserves the
|
||||
abort for a load-bearing calculator raise)."""
|
||||
if persisted is None:
|
||||
logger.warning(
|
||||
"no persisted Baseline Performance for property_id=%s; modelling "
|
||||
"against its own re-score (modelled_baseline_sap=%.2f)",
|
||||
property_id,
|
||||
modelled_baseline_sap,
|
||||
)
|
||||
return
|
||||
effective_sap: int = persisted.effective.sap_score
|
||||
if abs(modelled_baseline_sap - effective_sap) <= _MAX_BASELINE_DIVERGENCE:
|
||||
return
|
||||
logger.error(
|
||||
"modelling baseline diverges from persisted Effective Performance for "
|
||||
"property_id=%s: modelled=%.2f effective=%s (%s). Plan post_* figures "
|
||||
"are read against the effective baseline, so this surfaces as an "
|
||||
"unmodelled gain — see #1655/#1656.",
|
||||
property_id,
|
||||
modelled_baseline_sap,
|
||||
effective_sap,
|
||||
persisted.rebaseline_reason,
|
||||
)
|
||||
|
||||
|
||||
def _target_sap(scenario: Scenario) -> Optional[float]:
|
||||
|
|
|
|||
|
|
@ -178,17 +178,24 @@ class FakeSpatialRepo(SpatialRepository):
|
|||
|
||||
|
||||
class FakePropertyBaselineRepo(PropertyBaselineRepository):
|
||||
def __init__(self) -> None:
|
||||
def __init__(
|
||||
self, by_property: Optional[dict[int, PropertyBaselinePerformance]] = None
|
||||
) -> None:
|
||||
self.saved: list[tuple[PropertyBaselinePerformance, int]] = []
|
||||
self._by_property = by_property or {}
|
||||
|
||||
def save(self, baseline: PropertyBaselinePerformance, property_id: int) -> int:
|
||||
self.saved.append((baseline, property_id))
|
||||
self._by_property[property_id] = baseline
|
||||
return len(self.saved)
|
||||
|
||||
def get_for_property(
|
||||
self, property_id: int
|
||||
) -> Optional[PropertyBaselinePerformance]: # pragma: no cover
|
||||
raise NotImplementedError
|
||||
) -> Optional[PropertyBaselinePerformance]:
|
||||
"""None for a Property the Baseline stage never established — the real
|
||||
repo's behaviour, and what the Modelling coherence check treats as a
|
||||
data gap rather than a fault."""
|
||||
return self._by_property.get(property_id)
|
||||
|
||||
|
||||
class FakeScenarioRepository(ScenarioRepository):
|
||||
|
|
|
|||
|
|
@ -43,6 +43,8 @@ def _persisted(effective_sap: int) -> PropertyBaselinePerformance:
|
|||
lodged=None,
|
||||
effective=effective,
|
||||
rebaseline_reason="physical_state_changed",
|
||||
space_heating_kwh=0.0,
|
||||
water_heating_kwh=0.0,
|
||||
)
|
||||
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue