From dee72bbdfa34a83f00ab0307a789a73a55d26f9c Mon Sep 17 00:00:00 2001 From: Jun-te Kim Date: Fri, 3 Jul 2026 09:12:19 +0000 Subject: [PATCH] =?UTF-8?q?fix(rdsap):=20=C2=A75=20(12)=20floor=20infiltra?= =?UTF-8?q?tion=20falls=20back=20to=20floors[].description?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit _has_suspended_timber_floor_per_spec only read the Main bp's per-part floor_construction_type lodgement; the gov-API mapper frequently leaves that None even when the global epc.floors[].description carries an explicit "Suspended, ..." observation. _main_floor_u_value already fell back to this joined description for the U-value calc — the infiltration rule did not, so a genuinely suspended-timber floor silently entered (12)=0 instead of 0.2 unsealed. Extracted the shared _effective_floor_description helper and added a tri-state resolver (explicit lodgement -> description, excluding "not timber" -> Table 19 footnote-1 age-A/B default) so both paths agree. Cert 100061275133 (Elmhurst-validated build, PR #1439 handoff): engine 77.24 -> 76.33, now an exact match to lodged (76). Corpus gauge 77.7% -> 77.8% within-0.5, MAE 0.637 -> 0.636; floors ratcheted. Co-Authored-By: Claude Fable 5 --- .../sap10_calculator/rdsap/cert_to_inputs.py | 97 +++++++++++++++---- .../rdsap/test_cert_to_inputs.py | 74 ++++++++++++++ .../test_real_cert_sap_accuracy.py | 34 ++++--- .../epc_client/test_sap_accuracy_corpus.py | 15 ++- 4 files changed, 185 insertions(+), 35 deletions(-) diff --git a/domain/sap10_calculator/rdsap/cert_to_inputs.py b/domain/sap10_calculator/rdsap/cert_to_inputs.py index 27451560e..5d87f80cd 100644 --- a/domain/sap10_calculator/rdsap/cert_to_inputs.py +++ b/domain/sap10_calculator/rdsap/cert_to_inputs.py @@ -5321,6 +5321,35 @@ _AGE_BANDS_A_TO_E: Final[frozenset[str]] = frozenset({"A", "B", "C", "D", "E"}) _SUSPENDED_TIMBER_FLOOR_TYPE: Final[str] = "Suspended timber" _GROUND_FLOOR_TYPE: Final[str] = "Ground floor" _FLOOR_U_SEALED_THRESHOLD: Final[float] = 0.5 +# Table 19 footnote (1): age bands whose default floor_construction is +# suspended timber when the construction is unknown. Mirrors +# `_SUSPENDED_TIMBER_DEFAULT_BANDS` in rdsap_uvalues.py — duplicated +# locally rather than cross-imported (private symbol) per this module's +# existing convention (see `_main_floor_u_value`). +_TABLE_19_FOOTNOTE_1_SUSPENDED_TIMBER_BANDS: Final[frozenset[str]] = frozenset({"A", "B"}) + + +def _effective_floor_description( + epc: EpcPropertyData, main: SapBuildingPart, +) -> Optional[str]: + """Mirror `heat_transmission_section_from_cert`'s + `effective_floor_description` rule: the per-bp `floor_construction_type` + lodgement ("Suspended timber" / "Solid") takes precedence over the + global `epc.floors[].description` since it's the explicit per-part + Elmhurst Summary §3/§9 lodgement. Falls back to the joined global + description when the per-bp field is unlodged (the gov-API mapper + often leaves it `None`). Inlined (vs importing from + heat_transmission) to keep cert_to_inputs free of cross-module + private symbol imports. + """ + if main.floor_construction_type: + return main.floor_construction_type + descs = [ + d for d in + (getattr(f, "description", None) for f in (epc.floors or [])) + if d + ] + return " | ".join(descs) if descs else None def _main_floor_u_value(epc: EpcPropertyData) -> Optional[float]: @@ -5357,21 +5386,6 @@ def _main_floor_u_value(epc: EpcPropertyData) -> Optional[float]: int(raw_floor_ins) if isinstance(raw_floor_ins, (int, float)) else (0 if raw_floor_ins == "NI" else None) ) - # Mirror heat_transmission's `effective_floor_description`: the per-bp - # `floor_construction_type` takes precedence over a joined - # `epc.floors[].description` since the per-part lodgement is the - # explicit Elmhurst Summary §3/§9 surface. Inline the join (vs - # importing from heat_transmission) to keep cert_to_inputs free of - # cross-module private symbol imports. - if main.floor_construction_type: - effective_floor_description = main.floor_construction_type - else: - descs = [ - d for d in - (getattr(f, "description", None) for f in (epc.floors or [])) - if d - ] - effective_floor_description = " | ".join(descs) if descs else None return u_floor( country=Country.from_code(epc.country_code) if epc.country_code else None, age_band=main.construction_age_band, @@ -5380,10 +5394,49 @@ def _main_floor_u_value(epc: EpcPropertyData) -> Optional[float]: area_m2=ground_fd.total_floor_area_m2, perimeter_m=ground_fd.heat_loss_perimeter_m, wall_thickness_mm=main.wall_thickness_mm, - description=effective_floor_description, + description=_effective_floor_description(epc, main), ) +def _floor_construction_is_suspended_timber( + epc: EpcPropertyData, main: SapBuildingPart, +) -> bool: + """RdSAP 10 Table 19 footnote (1) construction resolution for the §5 + (12) floor-infiltration rule. Tri-state, mirroring `u_floor`'s + `_floor_is_suspended_from_description` / `_SUSPENDED_TIMBER_DEFAULT_ + BANDS` cascade (rdsap_uvalues.py) so the (12) infiltration rule and + the floor U-value calc agree on which floors are suspended timber: + + 1. An explicit construction lodgement (`_effective_floor_description` + — per-bp `floor_construction_type` else the joined + `epc.floors[].description`) decides: "Suspended, ..." is timber + UNLESS it's "Suspended, not timber" (suspended concrete etc. has + no floorboard-gap infiltration); "Solid, ..." is not suspended. + 2. No description signal AND no lodged `floor_construction` code at + all → Table 19 footnote (1): unknown construction defaults to + suspended timber only for age bands A/B. + """ + desc = _effective_floor_description(epc, main) + if desc is not None: + d = desc.strip().lower() + if d.startswith("suspended"): + return "not timber" not in d + if d.startswith("solid"): + return False + ground_fd = next( + (fd for fd in main.sap_floor_dimensions if fd.floor == 0), + main.sap_floor_dimensions[0] if main.sap_floor_dimensions else None, + ) + construction_known = ( + ground_fd is not None + and _int_or_none(ground_fd.floor_construction) is not None + ) + if construction_known: + return False + age = (main.construction_age_band or "").strip().upper() + return age in _TABLE_19_FOOTNOTE_1_SUSPENDED_TIMBER_BANDS + + def _has_suspended_timber_floor_per_spec( epc: EpcPropertyData, ) -> tuple[bool, bool]: @@ -5409,9 +5462,13 @@ def _has_suspended_timber_floor_per_spec( infiltration 0.2. The rule only applies when the Main bp's lowest floor is a - "Ground floor" with "Suspended timber" construction. All other - combinations fall through to `(False, False)` and the cascade - enters 0 for (12). + "Ground floor" with "Suspended timber" construction. Construction is + resolved the same tri-state way `u_floor` resolves it (RdSAP 10 + Table 19 footnote 1): the explicit per-bp `floor_construction_type` / + joined `epc.floors[].description` lodgement, else — when neither + lodges a construction signal — the footnote-1 age-band default + (suspended timber for bands A/B only). All other combinations fall + through to `(False, False)` and the cascade enters 0 for (12). """ if not epc.sap_building_parts: return False, False @@ -5421,7 +5478,7 @@ def _has_suspended_timber_floor_per_spec( return True, False if main.floor_type != _GROUND_FLOOR_TYPE: return False, False - if main.floor_construction_type != _SUSPENDED_TIMBER_FLOOR_TYPE: + if not _floor_construction_is_suspended_timber(epc, main): return False, False age = (main.construction_age_band or "").strip().upper() if age in _AGE_BANDS_F_TO_M: diff --git a/tests/domain/sap10_calculator/rdsap/test_cert_to_inputs.py b/tests/domain/sap10_calculator/rdsap/test_cert_to_inputs.py index d6a3c1cdc..e387c73c9 100644 --- a/tests/domain/sap10_calculator/rdsap/test_cert_to_inputs.py +++ b/tests/domain/sap10_calculator/rdsap/test_cert_to_inputs.py @@ -27,6 +27,7 @@ from datatypes.epc.surveys.elmhurst_site_notes import ( ) from datatypes.epc.domain.epc_property_data import ( + EnergyElement, EpcPropertyData, MainHeatingDetail, PhotovoltaicArray, @@ -1891,6 +1892,79 @@ def test_main_floor_u_value_routes_suspended_timber_via_floor_construction_type( assert sealed is False +def test_has_suspended_timber_floor_per_spec_falls_back_to_floors_description() -> None: + """RdSAP 10 §5 (12) reads the Main bp's `floor_construction_type` + lodgement, but the gov-API mapper often leaves that field `None` + while the global `epc.floors[].description` still carries the + assessor's "Suspended, ..." observation (cert 100061275133: Main bp + `floor_construction_type=None`, `epc.floors[0].description= + "Suspended, no insulation (assumed)"`). `_main_floor_u_value` already + falls back to this joined description (`effective_floor_description`); + the (12) infiltration rule must apply the same fallback so a + suspended-timber ground floor doesn't silently enter (12)=0.""" + from dataclasses import replace + # Arrange + main = replace( + make_building_part( + construction_age_band="B", + floor_dimensions=[make_floor_dimension(floor=0)], + ), + floor_type="Ground floor", + floor_construction_type=None, + ) + epc = make_minimal_sap10_epc(sap_building_parts=[main]) + epc.floors = [ + EnergyElement( + description="Suspended, no insulation (assumed)", + energy_efficiency_rating=0, + environmental_efficiency_rating=0, + ), + ] + + # Act + has_susp, sealed = _has_suspended_timber_floor_per_spec(epc) + + # Assert — description signal routes to the suspended-timber branch; + # band B is A-E with no supplied U-value and no retrofit insulation + # signal, so rule (b) applies "unsealed" → (12) = 0.2. + assert has_susp is True + assert sealed is False + + +def test_has_suspended_timber_floor_per_spec_excludes_suspended_not_timber() -> None: + """"Suspended, not timber" (e.g. suspended concrete) starts with the + same "Suspended" prefix as timber floors but must NOT trigger the + §5 (12) rule — the rule is specifically about air infiltration + through timber floorboard gaps (see the API mapper's + `_API_FLOOR_CONSTRUCTION_TO_STR` comment: "only 'Suspended timber' + triggers the §5 (12)... adjustment").""" + from dataclasses import replace + # Arrange + main = replace( + make_building_part( + construction_age_band="B", + floor_dimensions=[make_floor_dimension(floor=0)], + ), + floor_type="Ground floor", + floor_construction_type=None, + ) + epc = make_minimal_sap10_epc(sap_building_parts=[main]) + epc.floors = [ + EnergyElement( + description="Suspended, not timber", + energy_efficiency_rating=0, + environmental_efficiency_rating=0, + ), + ] + + # Act + has_susp, sealed = _has_suspended_timber_floor_per_spec(epc) + + # Assert + assert has_susp is False + assert sealed is False + + def test_rdsap_extract_fans_default_per_table_5() -> None: # Arrange — RdSAP 10 §4.1 Table 5 (PDF p.28) "Extract fans" default # when the lodged number is unknown. The Summary §12.0 "No. of diff --git a/tests/domain/sap10_calculator/test_real_cert_sap_accuracy.py b/tests/domain/sap10_calculator/test_real_cert_sap_accuracy.py index e57859941..e1877c598 100644 --- a/tests/domain/sap10_calculator/test_real_cert_sap_accuracy.py +++ b/tests/domain/sap10_calculator/test_real_cert_sap_accuracy.py @@ -867,23 +867,31 @@ _EXPECTATIONS: Final[tuple[RealCertExpectation, ...]] = ( # 2-storey SEMI-DETACHED HOUSE, band B, SOLID BRICK 400 mm with EXTERNAL # insulation, pitched 300 mm loft, SUSPENDED uninsulated ground floor, # mains-gas COMBI, double glazing (glazed_type 13, 2022+), TFA 78. - # Lodged 76 / engine 77 (77.24). Built in accredited Elmhurst RdSAP10 - # (evidence saved: elmhurst_summary.pdf / elmhurst_worksheet.pdf): the - # checkable FABRIC matches the engine closely — ROOF (300 mm loft, U 0.14) - # and WINDOWS (15.32 m² × eff U 1.3258 = 20.3106 W/K) match EXACTLY; the - # externally-insulated WALL is U 0.29 (engine) vs 0.30 (Elmhurst) — which - # VALIDATES the slice-5 external-insulation default (100 mm added over solid - # brick, `_WALL_ADDED_INSULATION_UNKNOWN_THICKNESS_MM`); the suspended - # ground floor is U 0.68 (engine) vs 0.70 (Elmhurst). (Elmhurst''s overall - # worksheet SAP 67 is NOT comparable — the reused-assessment combi entered - # as a "Standard Combi" carries a ~50 kWh/month Table-3a keep-hot loss the - # lodged assessment did not, a build-side heating artifact, not an engine - # gap.) Engine 77 sits within 1.24 of lodged; PINNED to the observed 77. + # Lodged 76 / engine 76 (76.33, was 77.24 before the §5 (12) fix below). + # Built in accredited Elmhurst RdSAP10 (evidence saved: elmhurst_summary.pdf + # / elmhurst_worksheet.pdf): the checkable FABRIC matches the engine + # closely — ROOF (300 mm loft, U 0.14) and WINDOWS (15.32 m² × eff U + # 1.3258 = 20.3106 W/K) match EXACTLY; the externally-insulated WALL is + # U 0.29 (engine) vs 0.30 (Elmhurst) — which VALIDATES the slice-5 + # external-insulation default (100 mm added over solid brick, + # `_WALL_ADDED_INSULATION_UNKNOWN_THICKNESS_MM`); the suspended ground + # floor is U 0.68 (engine) vs 0.70 (Elmhurst). (Elmhurst''s overall + # worksheet SAP is NOT comparable — the reused-assessment combi entered + # as a "Standard Combi" carries a Table-3a keep-hot loss the lodged + # assessment did not, a build-side heating artifact, not an engine gap.) + # §5 (12) fix: the Main bp lodges no `floor_construction_type` (gov-API + # leaves it None), but `epc.floors[0].description` = "Suspended, no + # insulation (assumed)" — `_has_suspended_timber_floor_per_spec` was + # only reading the per-bp field and silently entered (12)=0 for a + # genuinely suspended-timber floor. Now falls back to the joined + # description (mirroring `_main_floor_u_value`'s existing fallback), + # applying (12)=0.2 unsealed; engine converges EXACTLY on lodged (76). + # PINNED to the observed exact match. RealCertExpectation( schema="RdSAP-Schema-21.0.1", sample="uprn_100061275133", cert_num="uprn-100061275133", - sap_score=77, + sap_score=76, ), # UPRN 100051051866 (corpus; no cert number lodged). RdSAP-21.0.1 native — # DETACHED BUNGALOW, band I, CAVITY wall as-built insulated, pitched diff --git a/tests/infrastructure/epc_client/test_sap_accuracy_corpus.py b/tests/infrastructure/epc_client/test_sap_accuracy_corpus.py index 345f426e5..ff14c1fa7 100644 --- a/tests/infrastructure/epc_client/test_sap_accuracy_corpus.py +++ b/tests/infrastructure/epc_client/test_sap_accuracy_corpus.py @@ -237,7 +237,9 @@ _CORPUS = Path( # 69.51 -> 74.34 = lodged 74, VALIDATED against the accredited Elmhurst # worksheet ("insulated flat roof" U 0.5). within 77.3% -> 77.7%, PE 3.1 -> # 2.97, CO2 0.074 -> 0.072. -_MIN_WITHIN_HALF_SAP = 0.777 +# 77.7% -> 77.8% via the §5 (12) suspended-timber floor-infiltration +# fallback (see the `_MAX_SAP_MAE` note below for the mechanism). +_MIN_WITHIN_HALF_SAP = 0.778 # 0.793 -> 0.789 via the §12 Unknown-meter + dual-electric-immersion off-peak # trigger (RdSAP 10 PDF p.62): Apartment 241 (main 691 + 903 dual immersion) # -5.38 -> -1.05. Worksheet-validated on "simulated case 48" (Elmhurst SAP 57, @@ -323,7 +325,16 @@ _MIN_WITHIN_HALF_SAP = 0.777 # (U~1.4) -> ~0.29, PE +59.6 -> ~0, SAP 64.35 -> 71.74 (lodged 71). Scoped to # brick/stone (the only §5.8 R-value path); within-0.5 held at 77.0%, MAE # 0.658 -> 0.647, PE 3.2 -> 3.1. Unit-pinned in test_rdsap_uvalues. -_MAX_SAP_MAE = 0.641 +# 77.7% -> 77.8% (MAE 0.637 -> 0.636) via the §5 (12) suspended-timber +# floor-infiltration fallback: `_has_suspended_timber_floor_per_spec` +# only read the Main bp's per-part `floor_construction_type` lodgement, +# which the gov-API mapper frequently leaves `None` even when the global +# `epc.floors[].description` carries an explicit "Suspended, ..." +# observation (`_main_floor_u_value` already fell back to this joined +# description for the U-value calc — the infiltration rule did not). +# Cert 100061275133 (Elmhurst-validated build): SAP 77.24 -> 76.33, now +# an EXACT match to lodged (76). Unit-pinned in test_cert_to_inputs. +_MAX_SAP_MAE = 0.637 _MAX_CO2_MAE_TONNES = 0.072 # t CO2 / yr vs co2_emissions_current _MAX_PE_PER_M2_MAE = 3.0 # kWh / m2 / yr vs energy_consumption_current