From 6287ee7c8a97f298b35f094b5e84f1a237b0061e Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Wed, 22 Jul 2026 12:48:21 +0000 Subject: [PATCH] =?UTF-8?q?Normalise=20dual-encoded=20main=5Fheating=5Ffra?= =?UTF-8?q?ction=20to=20percent=20at=20the=20mapper=20=F0=9F=9F=A9?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The gov-EPC API lodges a two-main dwelling's main_heating_fraction pair in two incompatible encodings — a decimal fraction summing to ~1.0 ([0.8, 0.2]) or an integer percent summing to ~100 ([80, 20]) — while every consumer divides by 100 unconditionally. A decimal pair therefore collapsed the second main's share to ~1% of its true value, billing almost the whole load to the first system's fuel/efficiency. Normalise once at the mapper via a shared pair-sum discriminator (_normalised_main_heating_fraction): a genuine two-main split summing nearer 1.0 than 100 is scaled to percent; a percent pair and any single main are left exactly as lodged, so the 3 percent-encoded corpus certs do not regress. Wired across the seven from_rdsap_schema_* branches and the SAP-17.1 path, whose old int(fraction) floor silently deleted a decimal-lodged second main (int(0.35)=0). Spec: RdSAP 10 item 7-5 (percent is canonical), p.81. Corpus: within-0.5 80.3% -> 80.6%, MAE 0.584 -> 0.578. Example cert 10070086972 17.59 -> 18.73 (toward lodged 18). Closes #1666. Co-Authored-By: Claude Opus 4.8 (1M context) --- datatypes/epc/domain/mapper.py | 95 ++++++++++++-- .../test_mapper_main_heating_fraction.py | 116 ++++++++++++++++++ 2 files changed, 200 insertions(+), 11 deletions(-) create mode 100644 tests/datatypes/epc/domain/test_mapper_main_heating_fraction.py diff --git a/datatypes/epc/domain/mapper.py b/datatypes/epc/domain/mapper.py index b506995c4..ffa27812d 100644 --- a/datatypes/epc/domain/mapper.py +++ b/datatypes/epc/domain/mapper.py @@ -212,6 +212,35 @@ def _sap_opening_area_m2(width: Any, height: Any) -> float: ) +def _normalised_main_heating_fraction( + raw: Optional[Union[int, float]], + all_fractions: Sequence[Optional[Union[int, float]]], +) -> Optional[int]: + """Return `raw` as a canonical PERCENT (RdSAP 10 item 7-5, spec p.81). + + The gov-EPC API lodges a two-main dwelling's `main_heating_fraction` pair in + two incompatible encodings — a DECIMAL fraction summing to ~1.0 + (`[0.8, 0.2]`) or an integer PERCENT summing to ~100 (`[80, 20]`) — and every + consumer divides by 100 unconditionally. A decimal pair therefore collapses + the second system's share to ~1% of its true value, billing almost the whole + load to the first system's fuel and efficiency (#1666). + + Discriminate on the pair sum: a genuine two-main split summing nearer 1.0 + than 100 is decimal-encoded and scaled to percent; a percent-encoded pair + (sum ~100) and any single main are left exactly as lodged, so no + percent-encoded cert regresses. Returned as an int to match the domain + invariant (`MainHeatingDetail.main_heating_fraction` is a 0-100 percent). + """ + if raw is None: + return None + present = [float(f) for f in all_fractions if f is not None] + if len(present) >= 2: + pair_sum = sum(present) + if abs(pair_sum - 1.0) < abs(pair_sum - 100.0): + return round(float(raw) * 100.0) + return round(float(raw)) + + def _pv_array_field(array: Any, name: str) -> Any: """Read a PV-array field from either a `PhotovoltaicArray` dataclass (21.0.x, where the schema Union parsed the nested list) or a raw dict (older schemas @@ -753,7 +782,13 @@ class EpcPropertyDataMapper: main_heating_number=d.main_heating_number, main_heating_control=d.main_heating_control, main_heating_category=d.main_heating_category, - main_heating_fraction=d.main_heating_fraction, + main_heating_fraction=_normalised_main_heating_fraction( + d.main_heating_fraction, + [ + x.main_heating_fraction + for x in schema.sap_heating.main_heating_details + ], + ), sap_main_heating_code=d.sap_main_heating_code, central_heating_pump_age=d.central_heating_pump_age, main_heating_data_source=d.main_heating_data_source, @@ -1385,7 +1420,13 @@ class EpcPropertyDataMapper: main_heating_number=d.main_heating_number, main_heating_control=d.main_heating_control, main_heating_category=d.main_heating_category, - main_heating_fraction=d.main_heating_fraction, + main_heating_fraction=_normalised_main_heating_fraction( + d.main_heating_fraction, + [ + x.main_heating_fraction + for x in schema.sap_heating.main_heating_details + ], + ), sap_main_heating_code=d.sap_main_heating_code, central_heating_pump_age=None, main_heating_data_source=d.main_heating_data_source, @@ -1573,7 +1614,13 @@ class EpcPropertyDataMapper: main_heating_number=d.main_heating_number, main_heating_control=d.main_heating_control, main_heating_category=d.main_heating_category, - main_heating_fraction=d.main_heating_fraction, + main_heating_fraction=_normalised_main_heating_fraction( + d.main_heating_fraction, + [ + x.main_heating_fraction + for x in schema.sap_heating.main_heating_details + ], + ), sap_main_heating_code=d.sap_main_heating_code, central_heating_pump_age=d.central_heating_pump_age, main_heating_data_source=d.main_heating_data_source, @@ -1799,7 +1846,13 @@ class EpcPropertyDataMapper: main_heating_number=d.main_heating_number, main_heating_control=d.main_heating_control, main_heating_category=d.main_heating_category, - main_heating_fraction=d.main_heating_fraction, + main_heating_fraction=_normalised_main_heating_fraction( + d.main_heating_fraction, + [ + x.main_heating_fraction + for x in schema.sap_heating.main_heating_details + ], + ), sap_main_heating_code=d.sap_main_heating_code, central_heating_pump_age=d.central_heating_pump_age, main_heating_data_source=d.main_heating_data_source, @@ -2052,7 +2105,13 @@ class EpcPropertyDataMapper: main_heating_number=d.main_heating_number, main_heating_control=d.main_heating_control, main_heating_category=d.main_heating_category, - main_heating_fraction=d.main_heating_fraction, + main_heating_fraction=_normalised_main_heating_fraction( + d.main_heating_fraction, + [ + x.main_heating_fraction + for x in schema.sap_heating.main_heating_details + ], + ), sap_main_heating_code=d.sap_main_heating_code, central_heating_pump_age=d.central_heating_pump_age, main_heating_data_source=d.main_heating_data_source, @@ -2285,7 +2344,13 @@ class EpcPropertyDataMapper: boiler_ignition_type=d.boiler_ignition_type, main_heating_control=d.main_heating_control, main_heating_category=d.main_heating_category, - main_heating_fraction=d.main_heating_fraction, + main_heating_fraction=_normalised_main_heating_fraction( + d.main_heating_fraction, + [ + x.main_heating_fraction + for x in schema.sap_heating.main_heating_details + ], + ), sap_main_heating_code=d.sap_main_heating_code, central_heating_pump_age=d.central_heating_pump_age, main_heating_data_source=d.main_heating_data_source, @@ -2634,7 +2699,13 @@ class EpcPropertyDataMapper: boiler_ignition_type=d.boiler_ignition_type, main_heating_control=d.main_heating_control, main_heating_category=d.main_heating_category, - main_heating_fraction=d.main_heating_fraction, + main_heating_fraction=_normalised_main_heating_fraction( + d.main_heating_fraction, + [ + x.main_heating_fraction + for x in schema.sap_heating.main_heating_details + ], + ), sap_main_heating_code=d.sap_main_heating_code, central_heating_pump_age=d.central_heating_pump_age, main_heating_data_source=d.main_heating_data_source, @@ -3323,10 +3394,12 @@ def _sap_17_1_heating(schema: SapSchema17_1) -> SapHeating: main_heating_index_number=d.main_heating_index_number, main_heating_number=d.main_heating_number, main_heating_category=d.main_heating_category, - main_heating_fraction=( - int(d.main_heating_fraction) - if d.main_heating_fraction is not None - else None + # #1666: normalise the encoding (and fix the old `int()` floor + # that silently deleted a decimal-lodged second main, e.g. + # int(0.35) -> 0) via the shared pair-sum discriminator. + main_heating_fraction=_normalised_main_heating_fraction( + d.main_heating_fraction, + [x.main_heating_fraction for x in sh.main_heating_details], ), main_heating_data_source=d.main_heating_data_source, ) diff --git a/tests/datatypes/epc/domain/test_mapper_main_heating_fraction.py b/tests/datatypes/epc/domain/test_mapper_main_heating_fraction.py new file mode 100644 index 000000000..4cb336d91 --- /dev/null +++ b/tests/datatypes/epc/domain/test_mapper_main_heating_fraction.py @@ -0,0 +1,116 @@ +"""#1666 — a two-main dwelling's `main_heating_fraction` pair is lodged by the +gov-EPC API in two incompatible encodings: a DECIMAL fraction summing to ~1.0 +(`[0.8, 0.2]`) or an integer PERCENT summing to ~100 (`[80, 20]`). Every consumer +divides by 100 unconditionally, so a decimal-encoded pair collapses the second +main's share to ~1% of its true value and bills almost the whole load to the +first system's fuel/efficiency. The mapper must normalise both encodings to the +spec-canonical percent (RdSAP 10 item 7-5, spec p.81) so the encoding is +invisible downstream, without regressing the percent-encoded certs. +""" + +from __future__ import annotations + +import json +from copy import deepcopy +from pathlib import Path +from typing import Any, Optional, cast + +from datatypes.epc.domain.mapper import EpcPropertyDataMapper +from domain.sap10_calculator.calculator import Sap10Calculator + +_RDSAP_CORPUS = Path("backend/epc_api/json_samples/RdSAP-Schema-21.0.1/corpus.jsonl") +_SAP_17_1_CORPUS = Path("backend/epc_api/json_samples/SAP-Schema-17.1/corpus.jsonl") + + +def _corpus_cert(corpus: Path, uprn: str) -> dict[str, Any]: + for line in corpus.read_text().splitlines(): + if uprn in line: + cert: dict[str, Any] = json.loads(line) + return cert + raise AssertionError(f"uprn {uprn} not found in {corpus.name}") + + +def _mapped_fractions(payload: dict[str, Any]) -> list[Optional[int]]: + epc = EpcPropertyDataMapper.from_api_response(payload) + assert epc is not None + return [m.main_heating_fraction for m in epc.sap_heating.main_heating_details if m] + + +def _continuous_sap(payload: dict[str, Any]) -> float: + epc = EpcPropertyDataMapper.from_api_response(payload) + assert epc is not None + return Sap10Calculator().calculate(epc).sap_score_continuous + + +def _scaled_fractions(payload: dict[str, Any], factor: float) -> dict[str, Any]: + """Deep-copy `payload`, multiplying every lodged `main_heating_fraction` by + `factor` — used to build a percent-encoded twin of a decimal-encoded cert.""" + twin = deepcopy(payload) + + def walk(node: object) -> None: + if isinstance(node, dict): + node_dict = cast("dict[str, Any]", node) + for key, value in node_dict.items(): + if key == "main_heating_fraction" and value is not None: + node_dict[key] = value * factor + else: + walk(value) + elif isinstance(node, list): + for value in cast("list[Any]", node): + walk(value) + + walk(twin) + return twin + + +def test_decimal_encoded_fraction_pair_is_normalised_to_percent() -> None: + # Arrange — corpus cert 10070086972 (semi-detached, lodged SAP 18) lodges its + # two mains as a DECIMAL pair [0.8, 0.2] summing to 1.0; the /100 consumers + # would otherwise bill the second main at 0.2% of the load, not 20%. + payload = _corpus_cert(_RDSAP_CORPUS, "10070086972") + + # Act + fractions = _mapped_fractions(payload) + + # Assert — scaled to the spec-canonical percent (RdSAP 10 item 7-5, p.81). + assert fractions == [80, 20] + + +def test_percent_encoded_fraction_pair_is_left_unchanged() -> None: + # Arrange — corpus cert 40037847 lodges a PERCENT pair [10, 90] summing to + # ~100; the pair-sum discriminator must leave it exactly as lodged so the 3 + # percent-encoded corpus certs do not regress. + payload = _corpus_cert(_RDSAP_CORPUS, "40037847") + + # Act + fractions = _mapped_fractions(payload) + + # Assert + assert fractions == [10, 90] + + +def test_encoding_is_invisible_to_the_continuous_sap() -> None: + # Arrange — the decimal cert and a PERCENT twin built by scaling its lodged + # fractions x100 ([0.8, 0.2] -> [80, 20]); the encoding must not change SAP. + decimal_payload = _corpus_cert(_RDSAP_CORPUS, "10070086972") + percent_payload = _scaled_fractions(decimal_payload, 100) + + # Act + sap_decimal = _continuous_sap(decimal_payload) + sap_percent = _continuous_sap(percent_payload) + + # Assert — the two encodings must produce the same continuous SAP. + assert abs(sap_decimal - sap_percent) <= 1e-9 + + +def test_sap_17_1_decimal_second_main_is_not_floored_to_zero() -> None: + # Arrange — SAP-Schema-17.1 cert 38334849 lodges two mains as a DECIMAL pair + # [0.5, 0.5]. The old `int(fraction)` floor mapped BOTH to 0, silently + # deleting the split; the shared normaliser must scale to percent instead. + payload = _corpus_cert(_SAP_17_1_CORPUS, "38334849") + + # Act + fractions = _mapped_fractions(payload) + + # Assert + assert fractions == [50, 50]