From 90fbc593f990cd4ec61264ad413f876db114c7f8 Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Tue, 20 Jan 2026 19:41:54 +0000 Subject: [PATCH] handling fixed cost exceeding our budget, creating negative budget --- recommendations/Recommendations.py | 34 ++-- .../optimiser/funding_optimiser.py | 11 +- recommendations/tests/test_recommendations.py | 150 +++++++++++++++++- tox.ini | 2 +- 4 files changed, 181 insertions(+), 16 deletions(-) diff --git a/recommendations/Recommendations.py b/recommendations/Recommendations.py index 33558fae..c6fea3b6 100644 --- a/recommendations/Recommendations.py +++ b/recommendations/Recommendations.py @@ -1,7 +1,7 @@ import pandas as pd import numpy as np from backend.Property import Property -from typing import List, Mapping +from typing import List, Mapping, Any from itertools import groupby from recommendations.FloorRecommendations import FloorRecommendations from recommendations.WallRecommendations import WallRecommendations @@ -499,23 +499,26 @@ class Recommendations: return predicted_appliances_cost_reduction, predicted_appliances_kwh_reduction @staticmethod - def _check_veniltation_out_of_bounds(sap_impact, ventilation_sap_limit): + def _check_ventilation_out_of_bounds(sap_impact, ventilation_sap_limit): return (sap_impact < ventilation_sap_limit) or (sap_impact >= 0) @staticmethod def _adjust_ventilation_sap(sap_impact, ventilation_sap_limit): + if sap_impact >= 0: return -1 if sap_impact < ventilation_sap_limit: return ventilation_sap_limit + return sap_impact + @staticmethod def _filter_phase_adjustment(phase_adjustments): """ Utility function to select the entry from the dictionary, by phase, with the largest phase adjustment :param phase_adjustments: List of phase adjustments, in the form - [{"recommendation_id": str, "phase": int, "adjustment_amount": float}] + [{"recommendation_id": str, "phase": int, "sap_adjustment": float}] :return: """ filtered_adjustments = [] @@ -583,6 +586,15 @@ class Recommendations: if len(previous_phase_reps) == 1: return previous_phase_reps[0] + # It's unlikely that this will occur but this fallback will ensure that we don't + # run the next step and run a median of nothing, which will return None + if not previous_phase_reps: + return { + "sap": float(property_instance.data["current-energy-efficiency"]), + "carbon": float(property_instance.data["co2-emissions-current"]), + "heat_demand": float(property_instance.data["energy-consumption-current"]), + } + # Median fallback (including zero-length case) keys = ("sap", "carbon", "heat_demand") return { @@ -612,9 +624,9 @@ class Recommendations: @classmethod def _resolve_current_phase_sap( cls, - rec: Mapping[str, any], - previous_phase_values: Mapping[str, any], - phase_energy_efficiency_metrics: Mapping[str, any], + rec: Mapping[str, Any], + previous_phase_values: Mapping[str, Any], + phase_energy_efficiency_metrics: Mapping[str, Any], adjustments: list[dict], ) -> float: if rec.get("survey", False): @@ -713,14 +725,14 @@ class Recommendations: elif rec["type"] == "mechanical_ventilation": # ventilation is capped by having no greater and a -4 impact ventilation_sap_limit = -4 - ventilation_out_of_bounds = cls._check_veniltation_out_of_bounds( + ventilation_out_of_bounds = cls._check_ventilation_out_of_bounds( property_phase_impact["sap"], ventilation_sap_limit ) if ventilation_out_of_bounds: previous_modelled_sap = previous_phase_values.get("sap_prediction", 0) proposed_sap_impact = current_phase_values["sap"] - previous_modelled_sap - proposal_out_of_bounds = cls._check_veniltation_out_of_bounds( + proposal_out_of_bounds = cls._check_ventilation_out_of_bounds( proposed_sap_impact, ventilation_sap_limit ) if proposal_out_of_bounds: @@ -805,7 +817,7 @@ class Recommendations: return property_phase_impact, current_phase_values, adjustments @staticmethod - def _validate_recommendation_updates(rec: Mapping[str, any]): + def _validate_recommendation_updates(rec: Mapping[str, Any]): """ Utility function to validate that the recommendation updates have been applied correctly :param rec: updated recommendation @@ -821,11 +833,11 @@ class Recommendations: def calculate_recommendation_impact( cls, property_instance: Property, - all_predictions: Mapping[str, any], + all_predictions: Mapping[str, Any], recommendations: Mapping[int, List], representative_recommendations: Mapping[int, List], debug: bool = False - ) -> (Mapping[int, List], List[Mapping[str, any]]): + ) -> (Mapping[int, List], List[Mapping[str, Any]]): """ Given predictions from the model apis, with method will update the recommendations with the predicted diff --git a/recommendations/optimiser/funding_optimiser.py b/recommendations/optimiser/funding_optimiser.py index f9e471ce..a2f138ed 100644 --- a/recommendations/optimiser/funding_optimiser.py +++ b/recommendations/optimiser/funding_optimiser.py @@ -711,9 +711,12 @@ def optimise_with_scenarios( if kept: remaining_measures.append(kept) + remaining_budget = budget - fabric_cost if budget is not None else None + remaining_budget = 0 if remaining_budget < 0 else remaining_budget + picked_extra, extra_cost, extra_gain = run_optimizer( remaining_measures, - budget=budget - fabric_cost if budget is not None else None, + budget=remaining_budget, sub_target_gain=( target_gain - fabric_gain if target_gain is not None @@ -769,6 +772,12 @@ def optimise_with_scenarios( fixed_cost, fixed_gain = sum_cost_gain(fixed_items) + if budget is not None: + # If we have a budget, we cannot exceed it via our fixed cost. If we do, + # this is not a viable solution + if fixed_cost > budget: + continue + # Remaining measures (all other groups) remaining_measures = [ grp for gi, grp in enumerate(optimisation_measures) diff --git a/recommendations/tests/test_recommendations.py b/recommendations/tests/test_recommendations.py index 7a7930bf..a9915422 100644 --- a/recommendations/tests/test_recommendations.py +++ b/recommendations/tests/test_recommendations.py @@ -1,9 +1,6 @@ import pytest -import datetime import pandas as pd import numpy as np -from pandas import Timestamp -from numpy import nan from unittest.mock import Mock from recommendations.Recommendations import Recommendations @@ -372,6 +369,153 @@ def test_filter_phase_adjustment(input_data, expected): assert Recommendations._filter_phase_adjustment(input_data) == expected +@pytest.mark.parametrize( + "sap_impact, limit, expected", + [ + (1.0, -4, True), # positive SAP not allowed + (0.0, -4, True), # zero not allowed + (-1.0, -4, False), # valid range + (-3.9, -4, False), # valid range + (-4.0, -4, False), # exact lower bound allowed + (-4.1, -4, True), # below lower bound + ], +) +def test_check_ventilation_out_of_bounds(sap_impact, limit, expected): + assert Recommendations._check_ventilation_out_of_bounds( + sap_impact, limit + ) is expected + + +@pytest.mark.parametrize( + "sap_impact, limit, expected", + [ + (1.2, -4, -1), # positive → capped to -1 + (0.0, -4, -1), # zero → capped to -1 + (-5.0, -4, -4), # below limit → clamp + (-3.0, -4, -3.0), # already valid → unchanged + ], +) +def test_adjust_ventilation_sap(sap_impact, limit, expected): + assert Recommendations._adjust_ventilation_sap( + sap_impact, limit + ) == expected + + +def test_get_previous_phase_values_starting_phase(property_instance): + result = Recommendations._get_previous_phase_values( + rec_phase=0, + starting_phase=0, + impact_summary=[], + property_instance=property_instance, + ) + + assert result == { + "sap": 65.0, + "carbon": 2.4, + "heat_demand": 284.0, + } + + +def test_get_previous_phase_values_single_rep(property_instance): + impact_summary = [ + { + "phase": 0, + "representative": True, + "sap": 66, + "carbon": 2.2, + "heat_demand": 260, + } + ] + + result = Recommendations._get_previous_phase_values( + rec_phase=1, + starting_phase=0, + impact_summary=impact_summary, + property_instance=property_instance, + ) + + assert result["sap"] == 66 + assert result["carbon"] == 2.2 + assert result["heat_demand"] == 260 + + +def test_get_previous_phase_values_median(property_instance): + impact_summary = [ + {"phase": 1, "representative": True, "sap": 70, "carbon": 2.0, "heat_demand": 250}, + {"phase": 1, "representative": True, "sap": 74, "carbon": 1.6, "heat_demand": 230}, + ] + + result = Recommendations._get_previous_phase_values( + rec_phase=2, + starting_phase=0, + impact_summary=impact_summary, + property_instance=property_instance, + ) + + assert result["sap"] == np.median([70, 74]) + assert result["carbon"] == np.median([2.0, 1.6]) + assert result["heat_demand"] == np.median([250, 230]) + + +def test_compute_phase_impact_standard(): + previous = {"sap": 65, "carbon": 2.4, "heat_demand": 284} + current = {"sap": 64, "carbon": 2.6, "heat_demand": 300} + + impact = Recommendations._compute_phase_impact( + rec_type="loft_insulation", + previous_phase_values=previous, + current_phase_values=current, + ) + + # monotonicity enforced + assert impact["sap"] == 0 + assert impact["carbon"] == 0 + assert impact["heat_demand"] == 0 + + +def test_compute_phase_impact_mechanical_ventilation(): + previous = {"sap": 65, "carbon": 2.4, "heat_demand": 284} + current = {"sap": 63, "carbon": 2.4, "heat_demand": 284} + + impact = Recommendations._compute_phase_impact( + rec_type="mechanical_ventilation", + previous_phase_values=previous, + current_phase_values=current, + ) + + assert impact["sap"] == -2 + + +def test_resolve_current_phase_sap_with_adjustments(): + rec = {"phase": 3, "survey": False} + previous = {"sap": 65} + phase_metrics = {"sap_change": 70} + adjustments = [ + {"phase": 1, "sap_adjustment": 1.5}, + {"phase": 2, "sap_adjustment": 2.0}, + ] + + sap = Recommendations._resolve_current_phase_sap( + rec=rec, + previous_phase_values=previous, + phase_energy_efficiency_metrics=phase_metrics, + adjustments=adjustments, + ) + + assert sap == 70 - (1.5 + 2.0) + + +def test_validate_recommendation_updates_raises(): + rec = { + "sap_points": None, + "co2_equivalent_savings": None, + "heat_demand": None, + } + + with pytest.raises(ValueError): + Recommendations._validate_recommendation_updates(rec) + + def test_calculate_recommendation_impact(property_instance, heat_demand_predictions, carbon_predictions): ####### # Case 3 diff --git a/tox.ini b/tox.ini index e330f564..858a3f93 100644 --- a/tox.ini +++ b/tox.ini @@ -9,5 +9,5 @@ deps = -rbackend/engine/requirements.txt -rbackend/app/requirements/requirements.txt -rtest.requirements.txt -commands = pytest +commands = pytest {posargs}