From daf4449d0d21fdfd3601d28ce8c3cafb542871dd Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Fri, 10 Jul 2026 11:17:27 +0000 Subject: [PATCH] =?UTF-8?q?Optimiser=20objective=20is=20required=20on=20th?= =?UTF-8?q?e=20private=20helpers,=20hoisted=20in=20repair=20=F0=9F=9F=AA?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review findings on PR #1527: - The objective is required (no sap_rating default) on _repair_to_target, _best_repair_candidate and _rescored_groups: every caller already passes it, and a default would let a future call path silently optimise SAP for a carbon/bill goal while pyright stayed green. The default stays on the public optimise_package / optimise_package_fabric_first entry points. - _best_repair_candidate hoists objective(current) out of the candidate loop: current is loop-invariant, so for the Energy-Savings bill objective this was one full BillDerivation.derive per candidate per repair iteration for the same score. Co-Authored-By: Claude Fable 5 --- domain/modelling/optimisation/optimiser.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/domain/modelling/optimisation/optimiser.py b/domain/modelling/optimisation/optimiser.py index a40db733c..afb3fc4e8 100644 --- a/domain/modelling/optimisation/optimiser.py +++ b/domain/modelling/optimisation/optimiser.py @@ -454,7 +454,7 @@ def _repair_to_target( baseline_epc: EpcPropertyData, budget: Optional[float], target_sap: float, - objective: Callable[[Score], float] = sap_rating, + objective: Callable[[Score], float], ) -> OptimisedPackage: """Inject dependencies onto the warm-start, re-score for the truth, then greedy-add the untreated-group Option with the best marginal objective-per-£ @@ -522,15 +522,16 @@ def _best_repair_candidate( baseline_epc: EpcPropertyData, current: Score, budget: Optional[float], - objective: Callable[[Score], float] = sap_rating, + objective: Callable[[Score], float], ) -> Optional[ScoredOption]: - """The untreated-group Option giving the best **marginal** SAP-per-£ when - added to the current package — re-scored (not the role-1 signal) with any - ventilation dependency it newly triggers folded in, so both its SAP and its - incremental cost are truthful. Affordable when the resulting whole-package + """The untreated-group Option giving the best **marginal** objective-per-£ + when added to the current package — re-scored (not the role-1 signal) with + any ventilation dependency it newly triggers folded in, so both its gain and + its incremental cost are truthful. Affordable when the resulting whole-package cost is within ``budget`` and strictly improving. None if there is none.""" used: set[int] = _used_group_indices(groups, chosen) base_cost: float = _package_cost(_inject(chosen, dependencies)) + current_value: float = objective(current) best: Optional[ScoredOption] = None best_ratio: float = 0.0 for index, group in enumerate(groups): @@ -544,7 +545,7 @@ def _best_repair_candidate( if budget is not None and package_cost > budget: continue trial: Score = _score(scorer, baseline_epc, trial_selected) - marginal: float = objective(trial) - objective(current) + marginal: float = objective(trial) - current_value if marginal <= 0.0: continue incremental: float = package_cost - base_cost