From e136e937d6f7657ad3f5c66a5d0f2e77ef92271c Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Wed, 17 Jun 2026 00:48:50 +0000 Subject: [PATCH] =?UTF-8?q?fix(heat-transmission):=20match=20roof=20descri?= =?UTF-8?q?ption=20per=20part=20by=20kind=20(RdSAP=2010=20=C2=A75.11)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The deduplicated `epc.roofs[]` list cannot be indexed 1:1 against the building parts (190/329 multi-part certs have len(roofs) != len(parts)), so every part's `u_roof` consumed a SINGLE join of all roof descriptions. That leaked one part's insulation state onto another: a "Flat, no insulation" extension dragged a "Pitched, insulated (assumed)" main roof to the uninsulated 2.30, ~3x over-stating its heat loss. 3-part certs systematically under-rated (56% within-0.5, mean -0.79 SAP). Partition the non-RR roof descriptions into flat vs pitched/sloping and match each part to its own kind (`_main_roof_descriptions_by_kind`), falling back to the global join when a part's kind has no matching entry. Corpus cert 100010129331: roof 110.5 -> 31.3 W/K, +13.10 -> -0.05 SAP. RdSAP-21.0.1 within-0.5 68.8% -> 69.5% (MAE 0.888 -> 0.859; PE 13.9 -> 13.6); 3-part cohort 56% -> 61%. Floors/ceilings ratcheted. Pinned in test_heat_transmission (by_kind split + mixed-roof no-contamination). Co-Authored-By: Claude Opus 4.8 (1M context) --- .../worksheet/heat_transmission.py | 42 ++++++++++- .../worksheet/test_heat_transmission.py | 75 +++++++++++++++++++ .../epc_client/test_sap_accuracy_corpus.py | 14 +++- 3 files changed, 127 insertions(+), 4 deletions(-) diff --git a/domain/sap10_calculator/worksheet/heat_transmission.py b/domain/sap10_calculator/worksheet/heat_transmission.py index 4d2971d8..d12b7da9 100644 --- a/domain/sap10_calculator/worksheet/heat_transmission.py +++ b/domain/sap10_calculator/worksheet/heat_transmission.py @@ -400,6 +400,32 @@ def _joined_main_roof_descriptions(roofs: list[Any]) -> Optional[str]: return " | ".join(parts) +def _main_roof_descriptions_by_kind( + roofs: list[Any], +) -> tuple[Optional[str], Optional[str]]: + """Partition the non-RR roof descriptions into ``(pitched, flat)`` joins. + + The deduplicated ``epc.roofs[]`` list cannot be indexed 1:1 against the + building parts (190/329 multi-part certs have len(roofs) != len(parts)), + so each part's ``u_roof`` historically consumed the SINGLE join of every + roof description. That leaks one part's insulation state onto another: a + "Flat, no insulation" extension dragged a "Pitched, insulated (assumed)" + main roof to the uninsulated 2.30, ~3x over-stating its heat loss (cert + 100010129331: roof 110.5 -> ~28 W/K, +13 SAP). Splitting by flat vs + pitched/sloping lets each part match its own kind; the global join + (`_joined_main_roof_descriptions`) stays the fallback when a part's kind + has no matching entry. "Roof room(s)" entries are dropped (they carry + their own §3.9/§3.10 shell cascade).""" + pitched: list[str] = [] + flat: list[str] = [] + for e in roofs: + d = getattr(e, "description", "") + if not d or "roof room" in d.lower(): + continue + (flat if "flat" in d.lower() else pitched).append(d) + return (" | ".join(pitched) or None, " | ".join(flat) or None) + + def _part_geometry(part: SapBuildingPart) -> dict[str, float]: if not part.sap_floor_dimensions: # A part with no floor dimensions has no derivable RR shell or @@ -617,6 +643,9 @@ def heat_transmission_from_cert( country = Country.from_code(epc.country_code) roof_description = _joined_main_roof_descriptions(epc.roofs) + pitched_roof_description, flat_roof_description = ( + _main_roof_descriptions_by_kind(epc.roofs) + ) wall_description = _joined_descriptions(epc.walls) floor_description = _joined_descriptions(epc.floors) @@ -888,8 +917,19 @@ def heat_transmission_from_cert( roof_thickness_explicitly_zero = ( isinstance(raw_roof_thickness, int) and raw_roof_thickness == 0 ) + # RdSAP 10 §5.11 — match THIS part's roof to its own kind's lodged + # description (flat vs pitched/sloping) rather than the global join, + # so a flat "no insulation" part does not drag a pitched insulated + # part to the uninsulated 2.30. Fall back to the global join when the + # part's kind has no matching `epc.roofs[]` entry. + part_roof_is_flat = "flat" in (part.roof_construction_type or "").lower() + matched_roof_description = ( + flat_roof_description if part_roof_is_flat else pitched_roof_description + ) + if matched_roof_description is None: + matched_roof_description = roof_description effective_roof_description = ( - None if roof_thickness_explicitly_zero else roof_description + None if roof_thickness_explicitly_zero else matched_roof_description ) # RdSAP 10 §5.11 Table 18 page 45: column (3) "Flat roof" applies # when the per-bp roof construction lodges as a flat roof and the diff --git a/tests/domain/sap10_calculator/worksheet/test_heat_transmission.py b/tests/domain/sap10_calculator/worksheet/test_heat_transmission.py index d1ed0c92..d841746e 100644 --- a/tests/domain/sap10_calculator/worksheet/test_heat_transmission.py +++ b/tests/domain/sap10_calculator/worksheet/test_heat_transmission.py @@ -38,6 +38,7 @@ from domain.sap10_calculator.worksheet.heat_transmission import ( from domain.sap10_calculator.worksheet.heat_transmission import ( _alt_wall_w_per_k, # pyright: ignore[reportPrivateUsage] _joined_main_roof_descriptions, # pyright: ignore[reportPrivateUsage] + _main_roof_descriptions_by_kind, # pyright: ignore[reportPrivateUsage] _part_geometry, # pyright: ignore[reportPrivateUsage] _round_half_up, # pyright: ignore[reportPrivateUsage] _window_bp_index, # pyright: ignore[reportPrivateUsage] @@ -82,6 +83,80 @@ def test_joined_main_roof_descriptions_keeps_pure_rr_fallback() -> None: assert result == "Roof room(s), no insulation (assumed)" +def test_main_roof_descriptions_by_kind_splits_flat_from_pitched() -> None: + # Arrange — a cert with a pitched insulated main roof + a flat + # uninsulated extension. The deduplicated epc.roofs[] cannot be indexed + # 1:1 against the parts, so each part must match its own KIND's + # description: the flat part's "no insulation" must not leak onto the + # pitched part (which would force the whole pitched roof to U=2.30). + roofs = [ + _Desc("Pitched, insulated (assumed)"), + _Desc("Flat, no insulation"), + _Desc("Roof room(s), no insulation (assumed)"), + ] + + # Act + pitched, flat = _main_roof_descriptions_by_kind(roofs) + + # Assert — RR dropped; flat and pitched kept apart. + assert pitched == "Pitched, insulated (assumed)" + assert flat == "Flat, no insulation" + + +def test_mixed_flat_pitched_roof_does_not_contaminate_pitched_u_value() -> None: + # Arrange — 2-part dwelling: a 100 m² pitched insulated-assumed main + # roof (U=0.40) + a 2 m² flat uninsulated extension (U=2.30). Before the + # per-kind split, the joined "Pitched, insulated (assumed) | Flat, no + # insulation" description leaked the flat's "no insulation" onto the + # pitched part, billing the WHOLE roof at 2.30 (100×2.30 + 2×2.30 = + # 234.6 W/K). Correct: 100×0.40 + 2×2.30 = 44.6 W/K. Mirrors corpus + # cert 100010129331 (roof 110.5 -> 31.3 W/K, +13 -> 0 SAP). + main = make_building_part( + identifier=BuildingPartIdentifier.MAIN, + construction_age_band="C", + roof_construction=4, + floor_dimensions=[ + make_floor_dimension( + total_floor_area_m2=100.0, room_height_m=2.5, + party_wall_length_m=0.0, heat_loss_perimeter_m=40.0, floor=0, + ), + ], + ) + ext = make_building_part( + identifier=BuildingPartIdentifier.EXTENSION_1, + construction_age_band="C", + roof_construction=5, + floor_dimensions=[ + make_floor_dimension( + total_floor_area_m2=2.0, room_height_m=2.5, + party_wall_length_m=0.0, heat_loss_perimeter_m=6.0, floor=0, + ), + ], + ) + ext.roof_construction_type = "Flat" + epc = make_minimal_sap10_epc( + total_floor_area_m2=102.0, + country_code="ENG", + sap_building_parts=[main, ext], + ) + epc.roofs = [ + EnergyElement( + description="Pitched, insulated (assumed)", + energy_efficiency_rating=4, environmental_efficiency_rating=4, + ), + EnergyElement( + description="Flat, no insulation", + energy_efficiency_rating=1, environmental_efficiency_rating=1, + ), + ] + + # Act + result = heat_transmission_from_cert(epc) + + # Assert — pitched main billed at its insulated U, not the flat's 2.30. + assert abs(result.roof_w_per_k - 44.6) <= 2.0 + + def test_part_geometry_floorless_part_honours_full_key_contract() -> None: # Arrange — a building part lodged with NO sap_floor_dimensions (e.g. # a party-wall-only or RR-only extension; observed on 5 certs in a diff --git a/tests/infrastructure/epc_client/test_sap_accuracy_corpus.py b/tests/infrastructure/epc_client/test_sap_accuracy_corpus.py index 8944b576..9aed0f34 100644 --- a/tests/infrastructure/epc_client/test_sap_accuracy_corpus.py +++ b/tests/infrastructure/epc_client/test_sap_accuracy_corpus.py @@ -111,10 +111,18 @@ _CORPUS = Path( # the wall_insulation_type=3 under-rate cluster (cert 100052159386 -26.2 -> -4.1 # SAP, walls 300 -> 55 W/K). within-0.5 68.6% -> 68.8% (MAE 0.942 -> 0.888; # PE MAE 14.3 -> 13.9; CO2 MAE 0.27 -> 0.26). Unit-pinned in test_rdsap_uvalues. -_MIN_WITHIN_HALF_SAP = 0.685 -_MAX_SAP_MAE = 0.89 +# PER-PART ROOF DESCRIPTION (RdSAP 10 §5.11): the deduplicated epc.roofs[] list +# was joined into ONE description fed to EVERY building part's u_roof, so a flat +# "no insulation" extension dragged a pitched "insulated (assumed)" main roof to +# the uninsulated 2.30 (3-part certs systematically under-rated: 56% within, +# -0.79 mean). Matching each part to its own kind (flat vs pitched) fixed cert +# 100010129331 (roof 110.5 -> 31.3 W/K, +13.1 -> -0.05 SAP). within-0.5 +# 68.8% -> 69.5% (MAE 0.888 -> 0.859; PE 13.9 -> 13.6); 3-part cohort 56% -> +# 61%. Pinned in test_heat_transmission (by_kind split + no-contamination). +_MIN_WITHIN_HALF_SAP = 0.69 +_MAX_SAP_MAE = 0.86 _MAX_CO2_MAE_TONNES = 0.30 # t CO2 / yr vs co2_emissions_current -_MAX_PE_PER_M2_MAE = 14.5 # kWh / m2 / yr vs energy_consumption_current +_MAX_PE_PER_M2_MAE = 14.0 # kWh / m2 / yr vs energy_consumption_current def _load_corpus() -> list[dict[str, Any]]: