mirror of
https://github.com/Hestia-Homes/Model.git
synced 2026-06-08 11:17:27 +00:00
docs: pin spec-aligned secondary-heating fraction per Appendix A
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 <noreply@anthropic.com>
This commit is contained in:
parent
3ab09845e7
commit
f14f76daf8
3 changed files with 84 additions and 1 deletions
|
|
@ -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,
|
||||
)
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue