From 30b1728337265f9ca7e9d8d581ae63f2dff5d5fa Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Wed, 15 Jul 2026 13:52:13 +0000 Subject: [PATCH] =?UTF-8?q?PasHub=20extraction:=20thread=20showers/baths,?= =?UTF-8?q?=20normalize=20floor=20label,=20code=20no-water-heating=20?= =?UTF-8?q?=F0=9F=9F=A9?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three silent-drop extraction fixes the gov-API/Elmhurst mappers already apply but `from_site_notes` uniquely omitted — each let a favorable default reach the SAP-10.2 calculator and over-rate the dwelling. Same calculator, so the whole gap was extraction fidelity (gov-API corpus 78.6% within-0.5 vs pashub 23.9%). #1598 — electric-shower + bath counts: `_map_sap_heating` built `shower_outlets` but never set `electric_shower_count`/`mixer_shower_count`/`number_baths`, so the cascade defaulted to 1 gas mixer / 0 electric, billing electric-shower hot water off the cheap main system. 100/205 fixtures lodge "Electric Shower". #1599 — floor label: pashub lodges "Ground Floor" (capital F); the calculator's RdSAP §5(12) suspended-timber floor-infiltration gate is case-sensitive on "Ground floor", so the 0.2-ACH term was silently dropped. Normalized at the mapper boundary. 98/205 suspended-timber. #1600 — water-heating "None": the RdSAP 10 §10.7 "no water heating system" signal was discarded and HW inherited the gas combi (58 Hackle 67.7 vs verified 59). Coded to WHC 999 so the existing electric-immersion + Table 28/29 default fires (also folds in #1594: blank cylinder size is no cylinder, not a phantom one). Guinness GMCA 205 cohort: within-0.5 23.9% -> 50.2% (103/205), MAE 1.410 -> 0.650. Ratchets tightened to 0.50 / 0.65. Verified vs Khalim's ground truth (16 Stillwater 78.2 -> 75.7 vs 76). #1601 (upper-floor +0.25 m) held — it empirically over-corrects this cohort (internal-vs-external dimension question, under investigation). pyright: 0 new errors. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../documents_parser/tests/test_end_to_end.py | 6 +- .../tests/test_pashub_sap_accuracy.py | 18 ++- datatypes/epc/domain/mapper.py | 88 ++++++++++++-- .../epc/domain/tests/test_from_site_notes.py | 115 +++++++++++++++++- 4 files changed, 211 insertions(+), 16 deletions(-) diff --git a/backend/documents_parser/tests/test_end_to_end.py b/backend/documents_parser/tests/test_end_to_end.py index 92544ee82..d3c489306 100644 --- a/backend/documents_parser/tests/test_end_to_end.py +++ b/backend/documents_parser/tests/test_end_to_end.py @@ -69,6 +69,10 @@ class TestPdfToEpcPropertyData: shower_outlet_type="Non-Electric Shower" ), ), + # water_use: one 'Non-Electric Shower' (mixer), one bath (#1598) + number_baths=1, + electric_shower_count=0, + mixer_shower_count=1, ), sap_windows=[ SapWindow( @@ -213,7 +217,7 @@ class TestPdfToEpcPropertyData: wall_thickness_mm=310, roof_insulation_location="Joists", roof_insulation_thickness=100, - floor_type="Ground Floor", + floor_type="Ground floor", # normalized (#1599) floor_construction_type="Solid", floor_insulation_type_str="As Built", floor_u_value_known=False, diff --git a/backend/documents_parser/tests/test_pashub_sap_accuracy.py b/backend/documents_parser/tests/test_pashub_sap_accuracy.py index f1b8037ce..6463e0c25 100644 --- a/backend/documents_parser/tests/test_pashub_sap_accuracy.py +++ b/backend/documents_parser/tests/test_pashub_sap_accuracy.py @@ -88,7 +88,16 @@ _MANIFEST_PATH = _FIXTURES_DIR / "manifest.json" # observed 20.0% (41/205). # Ratcheted 0.19 -> 0.23 by the window-orientation fix (see the MAE note # below): observed 23.9% (49/205) — the campaign's largest within-0.5 move. -_MIN_WITHIN_HALF: float = 0.23 +# Ratcheted 0.23 -> 0.50 by the #1596 extraction bundle (#1598 electric-shower +# + bath counts never threaded into `sap_heating`; #1599 floor label "Ground +# Floor" normalized to the calculator's "Ground floor" so the §5(12) suspended- +# timber infiltration term fires; #1600 water-heating "None" coded to the RdSAP +# §10.7 WHC 999 default instead of inheriting the gas combi). Observed 50.2% +# (103/205) — the campaign's largest move. All three are silent-drop fixes the +# gov-API/Elmhurst paths already apply; verified against Khalim's ground truth +# (16 Stillwater 78.2 -> 75.7 vs 76; 58 Hackle 67.7 -> ~58 vs 59). #1601 (upper- +# floor +0.25 m) is HELD — it empirically over-corrects this cohort. +_MIN_WITHIN_HALF: float = 0.50 # Ratcheted 2.71 -> 2.64 by #1590 bug 2 (roof "Insulation At: None" -> explicit # zero thickness): observed MAE 2.632 — the two ~7-SAP roof over-raters # (507665533138 / 507670893756) correcting is exactly 14/205 ≈ 0.07 of MAE. @@ -142,7 +151,12 @@ _MIN_WITHIN_HALF: float = 0.23 # RdSAP corpus). 16 Bingley Close reverts +3.2 -> +4.3, so observed MAE 1.405 -> # 1.4103; re-tighten when the water-only branch is fixed correctly under #1592 # (the cylinder-insulation MAPPER fix is retained, so the revert is partial). -_MAX_SAP_MAE: float = 1.411 +# Ratcheted 1.411 -> 0.65 by the #1596 extraction bundle (#1598 shower/bath + +# #1599 floor-label + #1600 water-heating-None — see the within-0.5 note above): +# observed MAE 0.6499. The three silent-drop fixes the gov-API/Elmhurst paths +# already apply; the electric-shower drop (100/205 fixtures) was the dominant +# lever. #1601 (upper-floor +0.25 m) held — it over-corrects this cohort. +_MAX_SAP_MAE: float = 0.65 _KNOWN_GAP_REASON = ( "pashub `from_site_notes` mapper does not int-code a main-heating " diff --git a/datatypes/epc/domain/mapper.py b/datatypes/epc/domain/mapper.py index d914bec2e..0bdda601b 100644 --- a/datatypes/epc/domain/mapper.py +++ b/datatypes/epc/domain/mapper.py @@ -383,7 +383,9 @@ class EpcPropertyDataMapper: ), sap_building_parts=sap_building_parts, solar_water_heating=renewables.solar_hot_water, - has_hot_water_cylinder=heating.water_heating.cylinder_size != "No Cylinder", + has_hot_water_cylinder=_pashub_cylinder_present( + heating.water_heating.cylinder_size + ), has_fixed_air_conditioning=ventilation.has_fixed_air_conditioning, wet_rooms_count=0, # no equivalent in site notes extensions_count=general.number_of_extensions, @@ -5948,6 +5950,12 @@ def _extract_age_band(age_range: str) -> str: def _map_floor_dimensions(floors: List[FloorMeasurement]) -> List[SapFloorDimension]: + # NOTE (#1601, HELD): the gov-API/Elmhurst mappers add a 0.25 m joist-void to + # UPPER-storey heights. Adding it here is spec-plausible but EMPIRICALLY + # over-corrects the PAS Hub cohort (within-0.5 50%→18%, moves 16 Stillwater + # away from its verified 76) once the shower/floor fixes land — pashub's + # surveyed height likely already behaves as a storey height. Held pending + # ground truth on PAS Hub's height semantics; left as the raw surveyed value. return [ SapFloorDimension( room_height_m=floor.height_m, @@ -6083,6 +6091,22 @@ def _pashub_room_in_roof( ) +# PAS Hub lodges the ground-floor type as "Ground Floor" (capital F). The +# calculator's suspended-timber floor-infiltration gate (RdSAP 10 §5(12), +# `_has_suspended_timber_floor_per_spec`) compares against the canonical +# "Ground floor" (lowercase f) that the gov-API path emits, so the capital-F +# label silently dropped the 0.2-ACH infiltration term for every PAS Hub +# suspended-timber floor. Normalize to the canonical spelling at the mapper +# boundary; any other floor-type label passes through unchanged (#1599). +_CANONICAL_GROUND_FLOOR_TYPE: Final[str] = "Ground floor" + + +def _pashub_floor_type(label: str) -> str: + if label.strip().lower() == _CANONICAL_GROUND_FLOOR_TYPE.lower(): + return _CANONICAL_GROUND_FLOOR_TYPE + return label + + def _map_main_building_part( construction: BuildingConstruction, measurements: BuildingMeasurements, @@ -6118,7 +6142,7 @@ def _map_main_building_part( roof_construction_type=roof.construction_type or None, roof_insulation_location=roof_location, roof_insulation_thickness=roof_thickness, - floor_type=floor.floor_type, + floor_type=_pashub_floor_type(floor.floor_type), floor_construction_type=floor.floor_construction, floor_insulation_type_str=floor.floor_insulation_type, floor_u_value_known=floor.floor_u_value_known, @@ -6179,6 +6203,12 @@ def _map_sap_window(window: Window) -> SapWindow: ) +# PAS Hub lodges a shower outlet's type as this exact label (capital S) for an +# electric shower; anything else ("Non-Electric Shower") is a mixer draw. The +# Elmhurst sibling uses the lowercase "Electric shower" spelling. +_PASHUB_ELECTRIC_SHOWER_LABEL: Final[str] = "Electric Shower" + + def _map_sap_heating( heating: HeatingAndHotWater, ventilation: Ventilation, water_use: WaterUse ) -> SapHeating: @@ -6197,6 +6227,21 @@ def _map_sap_heating( else None ) + # SAP 10.2 Appendix J routes electric showers via §J line 64a (their own + # kWh stream) and treats mixer showers as drawing from the HW system, so + # the cascade needs the actual per-type counts. Leaving them None made it + # default to 1 gas mixer / 0 electric, billing electric-shower hot water + # off the cheap main system and over-rating SAP (issue #1598). Tally the + # surveyed outlet list by the PAS Hub label ("Electric Shower" / "Non- + # Electric Shower"), mirroring the gov-API `_count_shower_outlets_by_type` + # and Elmhurst paths; a zero-shower lodging resolves to (0, 0). + electric_shower_count = sum( + 1 for s in water_use.showers if s.outlet_type == _PASHUB_ELECTRIC_SHOWER_LABEL + ) + mixer_shower_count = sum( + 1 for s in water_use.showers if s.outlet_type != _PASHUB_ELECTRIC_SHOWER_LABEL + ) + _ELECTRIC_SYSTEM_TYPES = {"electric storage heaters", "electric underfloor heating"} _raw_fuel = main.fuel.split(", ")[0] if main.fuel else "" fuel_type = ( @@ -6210,13 +6255,17 @@ def _map_sap_heating( ) # Water heating is only typed for cylinder dwellings; a combi leaves the WHC - # None so HW inherits the main system (issue #1565). - cylinder_present = heating.water_heating.cylinder_size != "No Cylinder" - water_heating_code = ( - _pashub_water_heating_code(heating.water_heating.system) - if cylinder_present - else None - ) + # None so HW inherits the main system (issue #1565). A `Water Heating Type: + # None` lodgement is the RdSAP 10 §10.7 "no water heating system" signal and + # must NOT inherit the (cheap) main system — code it WHC 999 so the + # calculator applies its electric-immersion + Table 28/29 defaults (#1600). + cylinder_present = _pashub_cylinder_present(heating.water_heating.cylinder_size) + if heating.water_heating.type.strip() == _PASHUB_NO_WATER_HEATING_TYPE: + water_heating_code: Optional[int] = _PASHUB_WHC_NO_WATER_HEATING_SYSTEM + elif cylinder_present: + water_heating_code = _pashub_water_heating_code(heating.water_heating.system) + else: + water_heating_code = None water_heating_fuel = ( _PASHUB_STANDARD_ELECTRICITY_FUEL if water_heating_code == _PASHUB_WHC_ELECTRIC_IMMERSION @@ -6265,6 +6314,9 @@ def _map_sap_heating( heating.secondary_heating.secondary_system ), shower_outlets=shower_outlets, + number_baths=water_use.number_of_baths, + electric_shower_count=electric_shower_count, + mixer_shower_count=mixer_shower_count, cylinder_size=( _pashub_cylinder_size_code( heating.water_heating.cylinder_size, @@ -7826,6 +7878,24 @@ _PASHUB_WATER_HEATING_SYSTEM_TO_SAP10: Dict[str, int] = { } _PASHUB_WHC_ELECTRIC_IMMERSION: Final[int] = 903 _PASHUB_STANDARD_ELECTRICITY_FUEL: Final[int] = 30 # Table 32 standard electricity +# RdSAP 10 §10.7 (PDF p.55) "No water heating system" — WHC 999. When PAS Hub +# lodges `Water Heating Type: None` (no system identified), the calculator's +# `_apply_rdsap_no_water_heating_system_default` substitutes electric immersion +# + the Table 28/29 cylinder defaults, matching the gov-API/Elmhurst path +# (mirrors `cert_to_inputs._WHC_NO_WATER_HEATING_SYSTEM`; leaf-module convention +# — the mapper keeps a local copy rather than importing the private constant). +_PASHUB_WHC_NO_WATER_HEATING_SYSTEM: Final[int] = 999 +# PAS Hub label for the "no water heating system" lodgement (Water Heating Type). +_PASHUB_NO_WATER_HEATING_TYPE: Final[str] = "None" + + +def _pashub_cylinder_present(cylinder_size: Optional[str]) -> bool: + """A PAS Hub dwelling has a hot-water cylinder iff a non-blank cylinder size + is lodged that is not "No Cylinder". A blank/None size is NOT a cylinder — a + combi lodging no "Cylinder Size:" line was previously billed a phantom + cylinder (storage + primary loss). "No Access" stays present — the cylinder + exists, it just wasn't measured (issue #1594).""" + return bool(cylinder_size) and cylinder_size != "No Cylinder" def _pashub_water_heating_code(system_label: Optional[str]) -> Optional[int]: diff --git a/datatypes/epc/domain/tests/test_from_site_notes.py b/datatypes/epc/domain/tests/test_from_site_notes.py index cd0d35de7..7534f7f1c 100644 --- a/datatypes/epc/domain/tests/test_from_site_notes.py +++ b/datatypes/epc/domain/tests/test_from_site_notes.py @@ -1109,6 +1109,106 @@ class TestWaterHeatingCoding: ) +class TestNoWaterHeatingSystemAndCylinderPresence: + """A PAS Hub `Water Heating Type: None` lodgement is the RdSAP 10 §10.7 + "no water heating system" signal → WHC 999 (the calculator then applies its + electric-immersion + Table 28/29 defaults), NOT the gas-combi inheritance + (issue #1600 / 58 Hackle St). Separately, `has_hot_water_cylinder` must be + False for a blank/None cylinder size (a combi lodging no "Cylinder Size:" + line was billed a phantom cylinder), while "No Access" stays present + (issue #1594). + """ + + def _survey( + self, *, wh_type: str = "Regular", system: str = "From main heating 1", + cylinder_size: str = "No Cylinder", + ) -> PasHubRdSapSiteNotes: + data = load("pashub_rdsap_site_notes_example1.json") + wh = data["heating_and_hot_water"]["water_heating"] + wh["type"] = wh_type + wh["system"] = system + wh["cylinder_size"] = cylinder_size + return from_dict(PasHubRdSapSiteNotes, data) + + def test_water_heating_type_none_maps_whc_999(self) -> None: + # 58 Hackle: Water Heating Type "None", blank system + cylinder. + result = EpcPropertyDataMapper.from_site_notes( + self._survey(wh_type="None", system="", cylinder_size="") + ) + assert result.sap_heating.water_heating_code == 999 + + def test_blank_cylinder_size_has_no_cylinder(self) -> None: + result = EpcPropertyDataMapper.from_site_notes( + self._survey(wh_type="None", system="", cylinder_size="") + ) + assert result.has_hot_water_cylinder is False + + def test_explicit_no_cylinder_has_no_cylinder(self) -> None: + result = EpcPropertyDataMapper.from_site_notes( + self._survey(cylinder_size="No Cylinder") + ) + assert result.has_hot_water_cylinder is False + + def test_real_cylinder_size_is_present(self) -> None: + result = EpcPropertyDataMapper.from_site_notes( + self._survey(cylinder_size="Normal (90-130 litres)") + ) + assert result.has_hot_water_cylinder is True + + def test_no_access_cylinder_is_present(self) -> None: + result = EpcPropertyDataMapper.from_site_notes( + self._survey(cylinder_size="No Access") + ) + assert result.has_hot_water_cylinder is True + + +class TestShowerAndBathCounts: + """`from_site_notes` must thread the surveyed shower outlets and bath count + into `sap_heating.electric_shower_count` / `mixer_shower_count` / + `number_baths`. The calculator reads those counts (SAP 10.2 Appendix J + routes electric showers to their own kWh stream); leaving them None made + the cascade default to 1 gas mixer / 0 electric, billing electric-shower + hot water off the cheap main system and over-rating SAP (issue #1598). + Mirrors the gov-API (`_count_shower_outlets_by_type`) and Elmhurst paths. + example1 lodges one 'Non-Electric Shower' and one bath. + """ + + def _survey( + self, showers: list[str], baths: int = 1 + ) -> PasHubRdSapSiteNotes: + data = load("pashub_rdsap_site_notes_example1.json") + data["water_use"]["showers"] = [ + {"id": i + 1, "outlet_type": t} for i, t in enumerate(showers) + ] + data["water_use"]["number_of_baths"] = baths + return from_dict(PasHubRdSapSiteNotes, data) + + def test_electric_shower_counted(self) -> None: + result = EpcPropertyDataMapper.from_site_notes( + self._survey(["Electric Shower"]) + ) + assert result.sap_heating.electric_shower_count == 1 + assert result.sap_heating.mixer_shower_count == 0 + + def test_non_electric_shower_is_mixer(self) -> None: + result = EpcPropertyDataMapper.from_site_notes( + self._survey(["Non-Electric Shower"]) + ) + assert result.sap_heating.electric_shower_count == 0 + assert result.sap_heating.mixer_shower_count == 1 + + def test_no_showers_counts_zero(self) -> None: + result = EpcPropertyDataMapper.from_site_notes(self._survey([])) + assert result.sap_heating.electric_shower_count == 0 + assert result.sap_heating.mixer_shower_count == 0 + + def test_number_baths_threaded(self) -> None: + result = EpcPropertyDataMapper.from_site_notes( + self._survey(["Electric Shower"], baths=2) + ) + assert result.sap_heating.number_baths == 2 + + class TestMainHeatingEfficiencySource: """`from_site_notes` must carry the surveyed PCDB `product_id` onto `main_heating_index_number`, so the calculator reads the actual appliance's @@ -1360,7 +1460,7 @@ class TestFromSiteNotesExample1: ) def test_floor_height(self, result: EpcPropertyData) -> None: - # floors[0].height_m: 2.37 + # floors[0].height_m: 2.37 (Floor 1, upper) assert ( result.sap_building_parts[0].sap_floor_dimensions[0].room_height_m == 2.37 ) @@ -1513,6 +1613,10 @@ class TestFromSiteNotesExample1: shower_outlet_type="Non-Electric Shower" ), ), + # water_use: one 'Non-Electric Shower' (mixer), one bath (#1598) + number_baths=1, + electric_shower_count=0, + mixer_shower_count=1, ), # Windows sap_windows=[ @@ -1610,7 +1714,7 @@ class TestFromSiteNotesExample1: roof_construction_type="Pitched roof (Slates or tiles), Access to loft", roof_insulation_location="Joists", roof_insulation_thickness=100, - floor_type="Ground Floor", + floor_type="Ground floor", # normalized from "Ground Floor" (#1599) floor_construction_type="Suspended, not timber", floor_insulation_type_str="As Built", floor_u_value_known=False, @@ -1735,8 +1839,11 @@ class TestFromSiteNotesFloorConstruction: return EpcPropertyDataMapper.from_site_notes(survey) def test_floor_type(self, result: EpcPropertyData) -> None: - # building_construction.floor.floor_type: "Ground Floor" - assert result.sap_building_parts[0].floor_type == "Ground Floor" + # building_construction.floor.floor_type: "Ground Floor" (capital F) is + # normalized to the calculator's canonical "Ground floor" so the RdSAP + # §5(12) suspended-timber floor-infiltration gate (case-sensitive on + # "Ground floor") fires instead of dropping to 0 ACH (issue #1599). + assert result.sap_building_parts[0].floor_type == "Ground floor" def test_floor_construction_type(self, result: EpcPropertyData) -> None: # building_construction.floor.floor_construction: "Suspended, not timber"