From aa338dd152ddd59e9ff4dbe86be117ed944a5445 Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Tue, 14 Jul 2026 16:11:36 +0000 Subject: [PATCH 1/2] Map PasHub walls_construction_type labels to SAP10 codes `from_site_notes` copied the raw `walls_construction_type` survey label onto the building part; the calculator's `_int_or_none` read it as `None`, so every wall fell back to the age-band + thickness U-value and lost the solid/cavity/timber/system-built distinction. Add `_pashub_wall_construction_int` (mirroring `_pashub_party_wall_ construction_int`) mapping the four surveyed labels to the WALL_* codes `u_wall` consumes, strict-raising `UnmappedPasHubLabel` on an unknown label and passing a blank label through as the empty-string "no lodging" sentinel (the field is `Union[int, str]`). On the Guinness GMCA cohort this trims SAP MAE 3.22 -> 3.11 and lifts within-1.0 12.9% -> 14.9%. Closes #1560 Co-Authored-By: Claude Opus 4.8 (1M context) --- datatypes/epc/domain/mapper.py | 38 +++++++++++++++- .../epc/domain/tests/test_from_site_notes.py | 45 +++++++++++++++++-- 2 files changed, 78 insertions(+), 5 deletions(-) diff --git a/datatypes/epc/domain/mapper.py b/datatypes/epc/domain/mapper.py index 5c785343f..39c429255 100644 --- a/datatypes/epc/domain/mapper.py +++ b/datatypes/epc/domain/mapper.py @@ -5936,7 +5936,9 @@ def _map_main_building_part( return SapBuildingPart( identifier=BuildingPartIdentifier.MAIN, construction_age_band=_extract_age_band(main.age_range), - wall_construction=main.walls_construction_type, + wall_construction=_pashub_wall_construction_int( + main.walls_construction_type + ), wall_insulation_type=main.walls_insulation_type, wall_thickness_measured=main.wall_thickness_mm is not None, party_wall_construction=_pashub_party_wall_construction_int( @@ -5962,7 +5964,9 @@ def _map_extension_building_part( return SapBuildingPart( identifier=BuildingPartIdentifier.extension(ext_c.id), construction_age_band=_extract_age_band(ext_c.age_range), - wall_construction=ext_c.walls_construction_type, + wall_construction=_pashub_wall_construction_int( + ext_c.walls_construction_type + ), wall_insulation_type=ext_c.walls_insulation_type, wall_thickness_measured=ext_c.wall_thickness_mm is not None, party_wall_construction=_pashub_party_wall_construction_int( @@ -7126,6 +7130,36 @@ def _pashub_party_wall_construction_int(label: Optional[str]) -> Optional[int]: return _PASHUB_PARTY_WALL_CONSTRUCTION_TO_SAP10[key] +# PasHub surveyed `walls_construction_type` label → SAP10 `wall_construction` +# code (the WALL_* constants in domain.sap10_ml.rdsap_uvalues) that `u_wall` +# consumes. Same target code-space as the GOV.UK-API (`_api_wall_construction_ +# code`) and Elmhurst (`_ELMHURST_WALL_CODE_TO_SAP10`) siblings — PasHub just +# lodges the full human label. +_PASHUB_WALL_CONSTRUCTION_TO_SAP10: Dict[str, int] = { + "Solid brick": 3, # WALL_SOLID_BRICK + "Cavity": 4, # WALL_CAVITY + "Timber frame": 5, # WALL_TIMBER_FRAME + "System Build (i.e Any Other)": 6, # WALL_SYSTEM_BUILT +} + + +def _pashub_wall_construction_int(label: Optional[str]) -> Union[int, str]: + """Resolve a PasHub surveyed `walls_construction_type` label to its SAP10 + `wall_construction` code at the mapper boundary. A blank label is "no lodging" + and passes through as the empty string (the `SapBuildingPart.wall_construction` + field is `Union[int, str]`, and the calculator's `_int_or_none` resolves a + non-numeric string to None → the age-band/thickness U-value fallback); a + non-empty label the lookup does not cover strict-raises `UnmappedPasHubLabel` + so the coverage gap is fixed here rather than silently dropping the + construction distinction downstream (#1560).""" + if label is None or not label.strip(): + return "" + key = label.strip() + if key not in _PASHUB_WALL_CONSTRUCTION_TO_SAP10: + raise UnmappedPasHubLabel("wall construction", label) + return _PASHUB_WALL_CONSTRUCTION_TO_SAP10[key] + + def _resolve_elmhurst_underfloor_subtype( main_floor: ElmhurstFloorDetails, main_age_band: str, diff --git a/datatypes/epc/domain/tests/test_from_site_notes.py b/datatypes/epc/domain/tests/test_from_site_notes.py index 843c2a96f..81bd72e29 100644 --- a/datatypes/epc/domain/tests/test_from_site_notes.py +++ b/datatypes/epc/domain/tests/test_from_site_notes.py @@ -74,6 +74,45 @@ class TestPartyWallConstructionCoding: EpcPropertyDataMapper.from_site_notes(survey) +class TestWallConstructionCoding: + """`from_site_notes` must int-code the PAS Hub `walls_construction_type` label + to the SAP10 `wall_construction` code `u_wall` consumes (WALL_* constants), + not copy the raw label. A raw label reads as `None`, so the wall U-value falls + back to age band + thickness only and loses the solid/cavity/timber/system + distinction (Guinness cohort, issue #1560). + """ + + @pytest.mark.parametrize( + "label, expected_code", + [ + ("Solid brick", 3), # WALL_SOLID_BRICK + ("Cavity", 4), # WALL_CAVITY + ("Timber frame", 5), # WALL_TIMBER_FRAME + ("System Build (i.e Any Other)", 6), # WALL_SYSTEM_BUILT + ("", ""), # no lodging — passes through (calc nulls it via _int_or_none) + ], + ) + def test_label_maps_to_sap_code( + self, label: str, expected_code: "int | str" + ) -> None: + data = load("pashub_rdsap_site_notes_example1.json") + data["building_construction"]["main_building"][ + "walls_construction_type" + ] = label + survey = from_dict(PasHubRdSapSiteNotes, data) + result = EpcPropertyDataMapper.from_site_notes(survey) + assert result.sap_building_parts[0].wall_construction == expected_code + + def test_unknown_label_strict_raises(self) -> None: + data = load("pashub_rdsap_site_notes_example1.json") + data["building_construction"]["main_building"][ + "walls_construction_type" + ] = "Woven Basket" + survey = from_dict(PasHubRdSapSiteNotes, data) + with pytest.raises(UnmappedPasHubLabel): + EpcPropertyDataMapper.from_site_notes(survey) + + class TestFromSiteNotesExample1: """ Fixture: pashub_rdsap_site_notes_example1.json @@ -259,8 +298,8 @@ class TestFromSiteNotesExample1: assert result.sap_building_parts[0].construction_age_band == "I" def test_wall_construction(self, result: EpcPropertyData) -> None: - # main_building.walls_construction_type: "Cavity" - assert result.sap_building_parts[0].wall_construction == "Cavity" + # main_building.walls_construction_type: "Cavity" → SAP10 code 4 (WALL_CAVITY) + assert result.sap_building_parts[0].wall_construction == 4 def test_wall_insulation_type(self, result: EpcPropertyData) -> None: # main_building.walls_insulation_type: "As built" @@ -509,7 +548,7 @@ class TestFromSiteNotesExample1: SapBuildingPart( identifier=BuildingPartIdentifier.MAIN, construction_age_band="I", - wall_construction="Cavity", + wall_construction=4, # "Cavity" → SAP10 code 4 (WALL_CAVITY) wall_insulation_type="As built", wall_thickness_measured=True, party_wall_construction=4, # "Cavity Masonry, Unfilled" → SAP10 code 4 (U=0.5) From ed15bd13f7b039fecdfa680ba2f8ba5e85da6f33 Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Tue, 14 Jul 2026 16:19:55 +0000 Subject: [PATCH 2/2] Map PasHub walls_insulation_type labels to SAP10 codes `from_site_notes` copied the raw `walls_insulation_type` survey label onto the building part; the calculator's `_int_or_none` read it as `None`, so the ~101 filled-cavity walls in the cohort lost their lower-U credit and were over-counted. Add `_pashub_wall_insulation_type_int` (mirroring the wall-construction helper and the Elmhurst `_ELMHURST_WALL_INSULATION_TO_SAP10` sibling) mapping the surveyed labels to the SAP10 wall-insulation codes `u_wall` consumes ("As built" -> 4 assumed/default, "Filled Cavity" -> 2, "External" -> 1), strict-raising `UnmappedPasHubLabel` on an unknown label. Stacked on #1560, this is the fabric fix that removes the cohort's systematic SAP bias: mean signed -0.79 -> -0.04, MAE 3.11 -> 2.79, within-0.5 8.5% -> 11.4%. Closes #1561 Co-Authored-By: Claude Opus 4.8 (1M context) --- datatypes/epc/domain/mapper.py | 35 ++++++++++++++- .../epc/domain/tests/test_from_site_notes.py | 44 +++++++++++++++++-- 2 files changed, 74 insertions(+), 5 deletions(-) diff --git a/datatypes/epc/domain/mapper.py b/datatypes/epc/domain/mapper.py index 39c429255..3ccd88fe4 100644 --- a/datatypes/epc/domain/mapper.py +++ b/datatypes/epc/domain/mapper.py @@ -5939,7 +5939,9 @@ def _map_main_building_part( wall_construction=_pashub_wall_construction_int( main.walls_construction_type ), - wall_insulation_type=main.walls_insulation_type, + wall_insulation_type=_pashub_wall_insulation_type_int( + main.walls_insulation_type + ), wall_thickness_measured=main.wall_thickness_mm is not None, party_wall_construction=_pashub_party_wall_construction_int( main.party_wall_construction_type @@ -5967,7 +5969,9 @@ def _map_extension_building_part( wall_construction=_pashub_wall_construction_int( ext_c.walls_construction_type ), - wall_insulation_type=ext_c.walls_insulation_type, + wall_insulation_type=_pashub_wall_insulation_type_int( + ext_c.walls_insulation_type + ), wall_thickness_measured=ext_c.wall_thickness_mm is not None, party_wall_construction=_pashub_party_wall_construction_int( ext_c.party_wall_construction_type @@ -7160,6 +7164,33 @@ def _pashub_wall_construction_int(label: Optional[str]) -> Union[int, str]: return _PASHUB_WALL_CONSTRUCTION_TO_SAP10[key] +# PasHub surveyed `walls_insulation_type` label → SAP10 wall-insulation code that +# `u_wall` consumes. Same target code-space as the Elmhurst sibling +# (`_ELMHURST_WALL_INSULATION_TO_SAP10`) — PasHub just lodges the full human +# label. "As built" is the assumed/default cascade code (Elmhurst "A"=4). +_PASHUB_WALL_INSULATION_TYPE_TO_SAP10: Dict[str, int] = { + "As built": 4, # as built / assumed (default cascade) + "Filled Cavity": 2, # WALL_INSULATION_FILLED_CAVITY + "External": 1, # external wall insulation +} + + +def _pashub_wall_insulation_type_int(label: Optional[str]) -> Union[int, str]: + """Resolve a PasHub surveyed `walls_insulation_type` label to its SAP10 + wall-insulation code at the mapper boundary. A blank label is "no lodging" and + passes through as the empty string (the field is `Union[int, str]`; the + calculator's `_int_or_none` resolves a non-numeric string to None); a non-empty + label the lookup does not cover strict-raises `UnmappedPasHubLabel` so the + coverage gap is fixed here rather than silently dropping the filled-cavity + U-value credit downstream (#1561).""" + if label is None or not label.strip(): + return "" + key = label.strip() + if key not in _PASHUB_WALL_INSULATION_TYPE_TO_SAP10: + raise UnmappedPasHubLabel("wall insulation type", label) + return _PASHUB_WALL_INSULATION_TYPE_TO_SAP10[key] + + def _resolve_elmhurst_underfloor_subtype( main_floor: ElmhurstFloorDetails, main_age_band: str, diff --git a/datatypes/epc/domain/tests/test_from_site_notes.py b/datatypes/epc/domain/tests/test_from_site_notes.py index 81bd72e29..e579079a2 100644 --- a/datatypes/epc/domain/tests/test_from_site_notes.py +++ b/datatypes/epc/domain/tests/test_from_site_notes.py @@ -113,6 +113,44 @@ class TestWallConstructionCoding: EpcPropertyDataMapper.from_site_notes(survey) +class TestWallInsulationTypeCoding: + """`from_site_notes` must int-code the PAS Hub `walls_insulation_type` label + to the SAP10 wall-insulation code `u_wall` consumes (matching the Elmhurst + `_ELMHURST_WALL_INSULATION_TO_SAP10` sibling), not copy the raw label. A raw + label reads as `None`, so a filled cavity loses its lower-U credit and the + wall is over-counted (Guinness cohort, issue #1561). + """ + + @pytest.mark.parametrize( + "label, expected_code", + [ + ("As built", 4), # as built / assumed (default cascade) + ("Filled Cavity", 2), # WALL_INSULATION_FILLED_CAVITY + ("External", 1), # external wall insulation + ("", ""), # no lodging — passes through (calc nulls it via _int_or_none) + ], + ) + def test_label_maps_to_sap_code( + self, label: str, expected_code: "int | str" + ) -> None: + data = load("pashub_rdsap_site_notes_example1.json") + data["building_construction"]["main_building"][ + "walls_insulation_type" + ] = label + survey = from_dict(PasHubRdSapSiteNotes, data) + result = EpcPropertyDataMapper.from_site_notes(survey) + assert result.sap_building_parts[0].wall_insulation_type == expected_code + + def test_unknown_label_strict_raises(self) -> None: + data = load("pashub_rdsap_site_notes_example1.json") + data["building_construction"]["main_building"][ + "walls_insulation_type" + ] = "Aerogel Blanket" + survey = from_dict(PasHubRdSapSiteNotes, data) + with pytest.raises(UnmappedPasHubLabel): + EpcPropertyDataMapper.from_site_notes(survey) + + class TestFromSiteNotesExample1: """ Fixture: pashub_rdsap_site_notes_example1.json @@ -302,8 +340,8 @@ class TestFromSiteNotesExample1: assert result.sap_building_parts[0].wall_construction == 4 def test_wall_insulation_type(self, result: EpcPropertyData) -> None: - # main_building.walls_insulation_type: "As built" - assert result.sap_building_parts[0].wall_insulation_type == "As built" + # main_building.walls_insulation_type: "As built" → SAP10 code 4 (as built/assumed) + assert result.sap_building_parts[0].wall_insulation_type == 4 def test_wall_thickness_measured(self, result: EpcPropertyData) -> None: # main_building.wall_thickness_mm: 280 → thickness was measured @@ -549,7 +587,7 @@ class TestFromSiteNotesExample1: identifier=BuildingPartIdentifier.MAIN, construction_age_band="I", wall_construction=4, # "Cavity" → SAP10 code 4 (WALL_CAVITY) - wall_insulation_type="As built", + wall_insulation_type=4, # "As built" → SAP10 code 4 (as built/assumed) wall_thickness_measured=True, party_wall_construction=4, # "Cavity Masonry, Unfilled" → SAP10 code 4 (U=0.5) sap_floor_dimensions=[