From b37ee071b7bf4c32c1e748c86f04a714ec6c7a14 Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Fri, 10 Jul 2026 10:29:45 +0000 Subject: [PATCH] =?UTF-8?q?Fabric-first=20skips=20the=20phase-2=20re-score?= =?UTF-8?q?=20when=20it=20cannot=20change=20the=20plan=20=F0=9F=9F=AA?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review findings on PR #1526, behaviour-preserving: - No fabric groups, or phase 1 committed nothing → delegate to the plain optimise_package: the phase-2 prefix would be empty and its re-scoring would reproduce the role-1 signals the groups already carry, one full SAP-calculator run per Option for zero effect (the common case on already-insulated stock). - Phase 1 consumed every group → return the fabric package instead of a phase-2 pass that can only select the empty package. - _rescored_groups takes start_sap from the caller: the post-fabric baseline score is the phase-1 package score already in hand, not a fresh calculator run. - fabric_types → phase_one_types: the set holds everything phase 1 committed, injected dependencies included — that inclusion is what the outstanding-dependencies filter relies on, so the name must not invite narrowing it to FABRIC_MEASURE_TYPES. Co-Authored-By: Claude Fable 5 --- domain/modelling/optimisation/optimiser.py | 47 +++++++++++++++++++--- 1 file changed, 41 insertions(+), 6 deletions(-) diff --git a/domain/modelling/optimisation/optimiser.py b/domain/modelling/optimisation/optimiser.py index 73f5e9a1a..a1e45a58f 100644 --- a/domain/modelling/optimisation/optimiser.py +++ b/domain/modelling/optimisation/optimiser.py @@ -240,6 +240,17 @@ def optimise_package_fabric_first( fabric_groups: list[list[ScoredOption]] = [ group for group in groups if _is_fabric_group(group) ] + if not fabric_groups: + # Nothing for phase 1 to claim (the envelope is already treated): + # a plain run is identical and skips the phase-2 re-scoring. + return optimise_package( + groups=groups, + scorer=scorer, + baseline_epc=baseline_epc, + budget=budget, + target_sap=target_sap, + dependencies=dependencies, + ) fabric_package: OptimisedPackage = optimise_package( groups=fabric_groups, scorer=scorer, @@ -253,6 +264,18 @@ def optimise_package_fabric_first( and fabric_package.score.sap_continuous >= target_sap ): return fabric_package + if not fabric_package.selected: + # Phase 1 committed nothing (no fabric affordable or worth having), so + # the phase-2 prefix would be empty and its re-scoring would reproduce + # the signals the groups already carry: a plain run is identical. + return optimise_package( + groups=groups, + scorer=scorer, + baseline_epc=baseline_epc, + budget=budget, + target_sap=target_sap, + dependencies=dependencies, + ) # Phase 2 — the upgrade requirement is not met by fabric alone: optimise # the remaining groups (non-fabric, plus any fabric group phase 1 left @@ -264,24 +287,32 @@ def optimise_package_fabric_first( remaining_groups: list[list[ScoredOption]] = [ group for index, group in enumerate(groups) if index not in consumed ] + if not remaining_groups: + return fabric_package post_fabric_scorer = _PrefixedScorer( scorer, [scored.option.overlay for scored in fabric_package.selected] ) leftover_budget: Optional[float] = ( None if budget is None else budget - _package_cost(fabric_package.selected) ) - # A dependency phase 1 already injected (e.g. the wall's ventilation) is + # Everything phase 1 committed — its picks plus the dependencies it + # injected. A dependency already injected (e.g. the wall's ventilation) is # satisfied for the whole package: phase 2 must not force it in again. - fabric_types: set[MeasureType] = { + phase_one_types: set[MeasureType] = { scored.option.measure_type for scored in fabric_package.selected } outstanding_dependencies: list[MeasureDependency] = [ dependency for dependency in dependencies - if dependency.required.option.measure_type not in fabric_types + if dependency.required.option.measure_type not in phase_one_types ] top_up: OptimisedPackage = optimise_package( - groups=_rescored_groups(remaining_groups, post_fabric_scorer, baseline_epc), + groups=_rescored_groups( + remaining_groups, + post_fabric_scorer, + baseline_epc, + start_sap=fabric_package.score.sap_continuous, + ), scorer=post_fabric_scorer, baseline_epc=baseline_epc, budget=leftover_budget, @@ -306,12 +337,16 @@ def _rescored_groups( groups: list[list[ScoredOption]], scorer: Scorer, baseline_epc: EpcPropertyData, + *, + start_sap: float, ) -> list[list[ScoredOption]]: """The groups with every Option's role-1 warm-start signal re-scored through ``scorer`` — for phase 2, its independent gain on the post-fabric dwelling rather than the raw baseline, so options whose worth changes once - the envelope is treated (a boiler on an insulated home) are re-ranked.""" - start_sap: float = scorer.score(baseline_epc, []).sap_continuous + the envelope is treated (a boiler on an insulated home) are re-ranked. + ``start_sap`` is the score of ``baseline_epc`` through ``scorer`` with no + candidate applied — the caller already has it (the phase-1 package score), + so it is threaded in rather than re-computed.""" return [ [ ScoredOption(