From f14f76daf89051bb4b8dfd183f6d9bfbd8e2bdf6 Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Mon, 18 May 2026 22:22:04 +0000 Subject: [PATCH] docs: pin spec-aligned secondary-heating fraction per Appendix A MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An attempted slice (S-B30, not committed) hypothesised that `main_heating_fraction=1` on the cert meant "no secondary heating" and overrode Table 11's 10% default. Probe at 300 certs penalised it: SAP MAE 4.69 → 4.85, SAP bias 0.98 → 1.61. The hypothesis was wrong and I should have read the spec before coding. SAP 10.2 Appendix A1 (p. 43) defines `main_heating_fraction` as the allocation between TWO main heating systems when both exist; not as the main-vs-secondary fraction. 99% of corpus certs have =1, meaning "single main, 100% allocation". SAP 10.2 Appendix A4(d) (p. 45) is explicit: "If any fixed secondary heater has been identified, the calculation proceeds with the identified secondary heater" and "Table 11 gives the fraction of the heating that is assumed to be supplied by the secondary system" — no override based on main_heating_fraction. Adds: - Regression test pinning the spec behaviour (test_main_heating_fraction_does_not_override_table11_secondary_default) - Regression test for the already-spec-aligned fallback path - _secondary_fraction docstring explaining why main_heating_fraction is NOT consulted (with reference to the failed attempt) - secondary_heating_type kwarg on make_sap_heating (test-only, was missing — needed to construct the regression fixture) Probe at 300 certs unchanged from prior baseline: SAP MAE 4.69, bias 0.98 PE MAE 43.32, bias 37.69 The hand-trace finding that cert 9036-0827 over-predicts cost remains real, but the secondary-heating fraction is per-spec. The residual ~£33 gap on that cert is most likely missing PCDB efficiency lookup (cert has main_heating_data_source=1 and index_number=10241 — PCDB data — and we fall back to category-default 0.80 vs typical PCDB- listed condensing-boiler 0.90+). Deferred to Session C per ADR-0009. Co-Authored-By: Claude Opus 4.7 --- .../domain/src/domain/ml/tests/_fixtures.py | 2 + .../src/domain/sap/rdsap/cert_to_inputs.py | 12 +++- .../sap/rdsap/tests/test_cert_to_inputs.py | 71 +++++++++++++++++++ 3 files changed, 84 insertions(+), 1 deletion(-) diff --git a/packages/domain/src/domain/ml/tests/_fixtures.py b/packages/domain/src/domain/ml/tests/_fixtures.py index 0d139839..37fccde4 100644 --- a/packages/domain/src/domain/ml/tests/_fixtures.py +++ b/packages/domain/src/domain/ml/tests/_fixtures.py @@ -85,6 +85,7 @@ def make_sap_heating( cylinder_size: Optional[Union[int, str]] = None, cylinder_insulation_thickness_mm: Optional[int] = None, secondary_fuel_type: Optional[int] = None, + secondary_heating_type: Optional[int] = None, ) -> SapHeating: """Build a SapHeating with SAP10 API defaults.""" return SapHeating( @@ -98,6 +99,7 @@ def make_sap_heating( cylinder_size=cylinder_size, cylinder_insulation_thickness_mm=cylinder_insulation_thickness_mm, secondary_fuel_type=secondary_fuel_type, + secondary_heating_type=secondary_heating_type, ) diff --git a/packages/domain/src/domain/sap/rdsap/cert_to_inputs.py b/packages/domain/src/domain/sap/rdsap/cert_to_inputs.py index d5bc6383..5b0b63d7 100644 --- a/packages/domain/src/domain/sap/rdsap/cert_to_inputs.py +++ b/packages/domain/src/domain/sap/rdsap/cert_to_inputs.py @@ -533,7 +533,17 @@ def _secondary_fraction( when (a) the cert has a secondary system lodged OR (b) the main heating code is in the §A.2.2 forced-secondary set (electric storage heaters). Returns 0.0 when neither applies — the most common case - for gas/oil main systems whose cert doesn't lodge a secondary.""" + for gas/oil main systems whose cert doesn't lodge a secondary. + + `main_heating_fraction` on the cert is NOT consulted here: empirical + probe shows it tracks main-system-1 vs main-system-2 allocation in + multi-main configurations (99% of corpus has =1, meaning "single + main, 100% allocation"), not main-vs-secondary. Elmhurst applies + Table 11's 10% secondary even when main_heating_fraction=1; the + spec is silent on overriding (only the §A.2.2 forced-secondary rule + is explicit), and an S-B30 attempt to override yielded SAP MAE + +0.16 — the wrong direction. + """ if main is None: return 0.0 code = main.sap_main_heating_code diff --git a/packages/domain/src/domain/sap/rdsap/tests/test_cert_to_inputs.py b/packages/domain/src/domain/sap/rdsap/tests/test_cert_to_inputs.py index 2915dde0..2d13e2f5 100644 --- a/packages/domain/src/domain/sap/rdsap/tests/test_cert_to_inputs.py +++ b/packages/domain/src/domain/sap/rdsap/tests/test_cert_to_inputs.py @@ -16,6 +16,8 @@ from __future__ import annotations from typing import Final +import pytest + from datatypes.epc.domain.epc_property_data import MainHeatingDetail from domain.ml.tests._fixtures import ( @@ -78,6 +80,75 @@ def _typical_semi_detached_epc(): ) +def test_main_heating_fraction_does_not_override_table11_secondary_default() -> None: + # Arrange — the S-B30 attempt assumed `main_heating_fraction=1` meant + # "no secondary heating" and dropped the Table 11 default in that + # case. Empirically that gave SAP MAE +0.16 (worse). The cert + # software (Elmhurst) DOES apply Table 11's 10% secondary even when + # main_heating_fraction=1 — the field tracks multi-main allocation + # not main-vs-secondary. This test pins the spec-aligned behaviour + # to prevent regressions. + main = MainHeatingDetail( + has_fghrs=False, + main_fuel_type=26, + heat_emitter_type=1, + emitter_temperature=1, + main_heating_control=2106, + main_heating_category=2, + sap_main_heating_code=102, + main_heating_fraction=1, + ) + epc = make_minimal_sap10_epc( + total_floor_area_m2=_TYPICAL_TFA_M2, + habitable_rooms_count=4, + country_code="ENG", + sap_building_parts=[make_building_part()], + sap_heating=make_sap_heating( + main_heating_details=[main], + secondary_heating_type=691, + ), + ) + + # Act + inputs = cert_to_inputs(epc) + + # Assert — Table 11's 0.10 applies regardless of main_heating_fraction. + assert inputs.secondary_heating_fraction == pytest.approx(0.1, abs=0.001) + + +def test_main_heating_fraction_missing_falls_back_to_table11_default() -> None: + # Arrange — when main_heating_fraction isn't lodged AND the cert + # has a secondary system lodged, Table 11's 0.10 default still + # applies (the pre-S-B30 behaviour for ~340 of 30 000 corpus certs). + + main = MainHeatingDetail( + has_fghrs=False, + main_fuel_type=26, + heat_emitter_type=1, + emitter_temperature=1, + main_heating_control=2106, + main_heating_category=2, + sap_main_heating_code=102, + main_heating_fraction=None, + ) + epc = make_minimal_sap10_epc( + total_floor_area_m2=_TYPICAL_TFA_M2, + habitable_rooms_count=4, + country_code="ENG", + sap_building_parts=[make_building_part()], + sap_heating=make_sap_heating( + main_heating_details=[main], + secondary_heating_type=691, + ), + ) + + # Act + inputs = cert_to_inputs(epc) + + # Assert + assert inputs.secondary_heating_fraction == pytest.approx(0.1, abs=0.001) + + def test_minimal_cert_round_trips_through_calculator_and_returns_sap_result() -> None: # Arrange — a complete-enough RdSAP cert (envelope + heating + windows # + region) that the mapper can populate every CalculatorInputs field.