diff --git a/backend/Property.py b/backend/Property.py index 6a84fc09..f196f49b 100644 --- a/backend/Property.py +++ b/backend/Property.py @@ -490,7 +490,7 @@ class Property: for rec_id in rec_ids: sim_epc = self.simulation_epcs[rec_id].copy() rec_impact = [x for x in impact_summary if x["recommendation_id"] == rec_id][0] - # We update all of the features that should have an impact on the kwh model + # We update all features that should have an impact on the kwh model sim_epc.update( { diff --git a/backend/app/db/functions/recommendations_functions.py b/backend/app/db/functions/recommendations_functions.py index e690991a..09d6da83 100644 --- a/backend/app/db/functions/recommendations_functions.py +++ b/backend/app/db/functions/recommendations_functions.py @@ -1,5 +1,5 @@ -from typing import Any, Dict, List, Optional -from sqlalchemy import inspect, text, insert, delete, select, update +from typing import Any, Dict, List, Tuple +from sqlalchemy import inspect, text, insert, delete, select from sqlalchemy.orm import Session, Mapper from sqlalchemy.exc import SQLAlchemyError from sqlmodel import Session @@ -618,18 +618,72 @@ def clear_portfolio_in_batches( print("Portfolio cleared in batches.") -def get_plans_by_portfolio_id(portfolio_id: int) -> List[PlanModel]: - stmt = select(PlanModel).where(PlanModel.portfolio_id == portfolio_id) +def get_plans_by_scenario_ids(ids: List[int]) -> List[PlanModel]: + stmt = select(PlanModel).where(PlanModel.scenario_id.in_(ids)) with db_read_session() as session: session_any: Any = session # Typehint as Any to satisfy Pylance... return session_any.exec(stmt).scalars().all() -def get_scenario(scenario_id: int) -> Optional[ScenarioModel]: - stmt = select(ScenarioModel).where(ScenarioModel.id == scenario_id) +def get_most_recent_plans_by_portfolio_id(portfolio_id: int) -> List[PlanModel]: + # NOTE: This statement works for Postgres only, because of the Distinct + stmt = ( + select(PlanModel) + .where(PlanModel.portfolio_id == portfolio_id) + .distinct( + PlanModel.property_id, PlanModel.scenario_id + ) # one plan per property per scenario + .order_by( + PlanModel.property_id, + PlanModel.scenario_id, + PlanModel.created_at.desc(), + PlanModel.id.desc(), + ) + ) with db_read_session() as session: session_any: Any = session # Typehint as Any to satisfy Pylance... - return session_any.exec(stmt).scalar_one_or_none() + return session_any.exec(stmt).scalars().all() + + +def get_most_recent_plans_by_scenario_ids(scenario_ids: List[int]) -> List[PlanModel]: + # NOTE: This statement works for Postgres only, because of the Distinct + stmt = ( + select(PlanModel) + .where(PlanModel.scenario_id.in_(scenario_ids)) + .distinct( + PlanModel.property_id, PlanModel.scenario_id + ) # one plan per property per scenario + .order_by( + PlanModel.property_id, + PlanModel.scenario_id, + PlanModel.created_at.desc(), + PlanModel.id.desc(), + ) + ) + + with db_read_session() as session: + session_any: Any = session # Typehint as Any to satisfy Pylance + return session_any.exec(stmt).scalars().all() + + +def get_scenarios_by_portfolio_id(portfolio_id: int) -> List[ScenarioModel]: + stmt = select(ScenarioModel).where(ScenarioModel.portfolio_id == portfolio_id) + with db_read_session() as session: + session_any: Any = session # Typehint as Any to satisfy Pylance... + return session_any.exec(stmt).scalars().all() + + +def get_default_plans( + portfolio_id: int, +) -> List[PlanModel]: + plan_stmt = select(PlanModel).where( + (PlanModel.portfolio_id == portfolio_id) & (PlanModel.is_default == True) + ) + + with db_read_session() as session: + session_any: Any = session # Typehint as Any to satisfy Pylance... + plans: List[PlanModel] = session_any.exec(plan_stmt).scalars().all() + return plans def bulk_update_plans( diff --git a/backend/app/domain/classes/plan.py b/backend/app/domain/classes/plan.py index 7970abcd..e7455427 100644 --- a/backend/app/domain/classes/plan.py +++ b/backend/app/domain/classes/plan.py @@ -47,6 +47,7 @@ class Plan: valuation_increase=plan_model.valuation_increase, cost_of_works=plan_model.cost_of_works, contingency_cost=plan_model.contingency_cost, + name=plan_model.name, ) return cls(record=record, scenario=scenario, id=plan_model.id) @@ -60,6 +61,14 @@ class Plan: case _: raise NotImplementedError + @property + def cost(self) -> float: + return ( + self.record.cost_of_works + if self.record.cost_of_works is not None + else float("inf") + ) + def to_sqlalchemy(self) -> PlanPersistence: scenario_record = self.scenario.record @@ -129,6 +138,7 @@ class Plan: valuation_increase=record.valuation_increase, cost_of_works=record.cost_of_works, contingency_cost=record.contingency_cost, + name=record.name, ) return PlanPersistence(plan=plan_model, scenario=scenario_model) diff --git a/backend/app/domain/records/plan_record.py b/backend/app/domain/records/plan_record.py index 2df7a7c6..63a82993 100644 --- a/backend/app/domain/records/plan_record.py +++ b/backend/app/domain/records/plan_record.py @@ -29,3 +29,4 @@ class PlanRecord: valuation_increase: Optional[float] = None cost_of_works: Optional[float] = None contingency_cost: Optional[float] = None + name: Optional[str] = None diff --git a/backend/categorisation/categorisation_trigger_request.py b/backend/categorisation/categorisation_trigger_request.py index 9ef1d106..44ac0ff1 100644 --- a/backend/categorisation/categorisation_trigger_request.py +++ b/backend/categorisation/categorisation_trigger_request.py @@ -1,5 +1,12 @@ +from typing import List, Optional from pydantic import BaseModel class CategorisationTriggerRequest(BaseModel): portfolio_id: int + + scenarios_to_consider: Optional[List[int]] = None + scenario_priority_order: Optional[List[int]] = None + + +# {"portfolio_id": 556, "scenarios_to_consider": [1039,1041], "scenario_priority_order": [1041,1039]} diff --git a/backend/categorisation/handler/handler.py b/backend/categorisation/handler/handler.py index 20076613..9fb235d5 100644 --- a/backend/categorisation/handler/handler.py +++ b/backend/categorisation/handler/handler.py @@ -1,5 +1,6 @@ import json from typing import Any, Mapping + from backend.categorisation.categorisation_trigger_request import ( CategorisationTriggerRequest, ) @@ -12,6 +13,10 @@ logger = setup_logger() def handler(event: Mapping[str, Any], context: Any) -> None: + logger.info("Received message") + + logger.info(f"Number of events: {len(event.get('Records', []))}") + for record in event.get("Records", []): try: body_dict = json.loads(record["body"]) @@ -20,7 +25,12 @@ def handler(event: Mapping[str, Any], context: Any) -> None: logger.debug("Successfully validated request body") - process_portfolio(payload.portfolio_id) + process_portfolio( + payload.portfolio_id, + payload.scenarios_to_consider, + payload.scenario_priority_order, + ) except Exception as e: + logger.info("Handler exception") logger.error(f"Failed to process record: {e}") diff --git a/backend/categorisation/local_handler/docker-compose.yml b/backend/categorisation/local_handler/docker-compose.yml new file mode 100644 index 00000000..9529fdb2 --- /dev/null +++ b/backend/categorisation/local_handler/docker-compose.yml @@ -0,0 +1,11 @@ +version: "3.9" + +services: + categorisation-lambda: + build: + context: ../../../ + dockerfile: backend/categorisation/handler/Dockerfile + ports: + - "9000:8080" + env_file: + - ../../../.env \ No newline at end of file diff --git a/backend/categorisation/local_handler/invoke_local_lambda.py b/backend/categorisation/local_handler/invoke_local_lambda.py new file mode 100644 index 00000000..5ed23c2d --- /dev/null +++ b/backend/categorisation/local_handler/invoke_local_lambda.py @@ -0,0 +1,25 @@ +#!/usr/bin/env python3 +import json +import requests + +LAMBDA_URL = "http://localhost:9000/2015-03-31/functions/function/invocations" + +payload = { + "Records": [ + { + "body": json.dumps( + { + "portfolio_id": 556, + "scenarios_to_consider": [], + "scenario_priority_order": [], + } + ) + } + ] +} + +response = requests.post(LAMBDA_URL, json=payload) + +print("Status code:", response.status_code) +print("Response:") +print(response.text) diff --git a/backend/categorisation/local_runner.py b/backend/categorisation/local_runner.py index 599cbbbb..7de55bc0 100644 --- a/backend/categorisation/local_runner.py +++ b/backend/categorisation/local_runner.py @@ -1,10 +1,18 @@ +from typing import List + from backend.categorisation.processor import process_portfolio def main() -> None: portfolio_id = 556 + scenarios_to_consider: List[int] = [] + scenario_priority_order: List[int] = [] - process_portfolio(portfolio_id) + process_portfolio( + portfolio_id=portfolio_id, + scenarios_to_consider=scenarios_to_consider, + scenario_priority_order=scenario_priority_order, + ) if __name__ == "__main__": diff --git a/backend/categorisation/processor.py b/backend/categorisation/processor.py index 7c5698b7..09db2983 100644 --- a/backend/categorisation/processor.py +++ b/backend/categorisation/processor.py @@ -1,10 +1,12 @@ from collections import defaultdict -from typing import Dict, List +from typing import Dict, List, Optional from backend.app.db.functions.recommendations_functions import ( bulk_update_plans, - get_plans_by_portfolio_id, - get_scenario, + get_default_plans, + get_most_recent_plans_by_portfolio_id, + get_most_recent_plans_by_scenario_ids, + get_scenarios_by_portfolio_id, ) from backend.app.db.models.recommendations import PlanModel, ScenarioModel from backend.app.domain.classes.plan import Plan @@ -14,37 +16,152 @@ from utils.logger import setup_logger logger = setup_logger() -def process_portfolio(portfolio_id: int) -> None: - print(f"Processing portfolio {portfolio_id}") - plans: List[Plan] = _load_plans_for_portfolio(portfolio_id) - plans_by_property: Dict[int, List[Plan]] = _group_plans_by_property(plans) +def process_portfolio( + portfolio_id: int, + scenarios_to_consider: Optional[List[int]] = None, + scenario_priority_order: Optional[List[int]] = None, +) -> None: # TODO: make this a class + logger.info(f"Processing portfolio {portfolio_id}") - for uprn, property_plans in plans_by_property.items(): + all_scenarios: List[Scenario] = _load_scenarios_for_portfolio(portfolio_id) + plans_by_id: Dict[int, Plan] = {} # TODO: make this an in-memory repository class + if scenarios_to_consider: + if len(scenarios_to_consider) < 2: + raise ValueError( + "Cannot run auto categorisation for fewer than 2 scenarios" + ) + + # first get all plans that we're interested in + plans_for_consideration: List[Plan] = _load_plans_for_portfolio( + portfolio_id, all_scenarios, scenarios_to_consider + ) + for plan in plans_for_consideration: + if plan.id is not None: # just in case + plans_by_id[plan.id] = plan + + # then unset existing defaults on domain objects regardless of whether they're under consideration or not + default_plans: List[Plan] = _get_default_plans(portfolio_id, all_scenarios) + for plan in default_plans: + plan.set_default(False) + if plan.id is not None: # just in case + plans_by_id[plan.id] = plan + + logger.info(f"Successfully unset {len(default_plans)} default plan(s)") + + # then set new defaults on domain objects under consideration + plans_for_consideration_by_property: Dict[int, List[Plan]] = ( + _group_plans_by_property(plans_for_consideration) + ) + + for property_id, property_plans in plans_for_consideration_by_property.items(): if not property_plans: - raise ValueError(f"No plans for property {uprn}") + raise ValueError(f"No plans for property {property_id}") - cheapest_plan = _choose_cheapest_relevant_plan(property_plans) - _update_default_flags(property_plans, cheapest_plan) + try: + cheapest_plan = choose_cheapest_relevant_plan( + property_plans, scenario_priority_order + ) + except Exception: + logger.error(f"Failed to find cheapest plan for property {property_id}") + raise + + property_plans = _update_plan_objects(property_plans, cheapest_plan) + for plan in property_plans: + if plan.id is not None: # just in case + plans_by_id[plan.id] = plan + + logger.info("Successfully set defaults on Plan objects in memory") + + # then pass all domain objects to database to update (regardless of whether they've changed) + _update_plans_in_db(list(plans_by_id.values())) + logger.info(f"Successfully updated {len(plans_by_id)} Plans in database") -def _load_plans_for_portfolio(portfolio_id: int) -> List[Plan]: - plan_models = get_plans_by_portfolio_id(portfolio_id) - print(f"Got {len(plan_models)} plans from database") +def choose_cheapest_relevant_plan( + plans: List[Plan], scenario_priority_order: Optional[List[int]] = None +) -> Plan: + scenario_priority_order = scenario_priority_order or [] + + eligible_plans: List[Plan] = [plan for plan in plans if plan.is_compliant] or plans + if not eligible_plans: + raise ValueError("No plans available to choose from.") + + for plan in eligible_plans: + if plan.id is None: + # This should never actually happen, but plan.id is optional to cater + # for new plans. We are only working with already persisted plans here + raise ValueError( + f"All plans must have an ID, but found a plan with no ID: {plan}" + ) + + min_cost: float = min(plan.cost for plan in eligible_plans) + + cheapest_plans: List[Plan] = [ + plan for plan in eligible_plans if plan.cost == min_cost + ] + + for priority_scenario_id in scenario_priority_order: + for plan in cheapest_plans: + if plan.scenario.id == priority_scenario_id: + return plan + + return cheapest_plans[0] + + +def _get_default_plans(portfolio_id: int, scenarios: List[Scenario]) -> List[Plan]: + default_plan_models = get_default_plans(portfolio_id) + + scenario_map = {s.id: s for s in scenarios} + + return [ + Plan.from_sqlalchemy(p, scenario_map[p.scenario_id]) + for p in default_plan_models + if p.scenario_id in scenario_map + ] + + +def _load_scenarios_for_portfolio(portfolio_id: int) -> List[Scenario]: + scenario_models: List[ScenarioModel] = get_scenarios_by_portfolio_id(portfolio_id) + + return [Scenario.from_sqlalchemy(s) for s in scenario_models] + + +def _load_plans_for_portfolio( + portfolio_id: int, + all_scenarios: List[Scenario], + scenarios_to_consider: Optional[List[int]] = None, +) -> List[Plan]: + + if scenarios_to_consider: + logger.info(f"Getting plans for {len(scenarios_to_consider)} scenarios") + plan_models: List[PlanModel] = get_most_recent_plans_by_scenario_ids( + scenarios_to_consider + ) + logger.info(f"Got {len(plan_models)} plan models from database") + else: + logger.info( + f"No list of Plans to consider provided. Getting all Plans for portfolio {portfolio_id}" + ) + plan_models: List[PlanModel] = get_most_recent_plans_by_portfolio_id( + portfolio_id + ) plans: List[Plan] = [] + if not all_scenarios: + raise Exception(f"No scenarios found for Portfolio {portfolio_id}") + for model in plan_models: - if not model.scenario_id: + + scenario = next((s for s in all_scenarios if s.id == model.scenario_id)) + if not scenario: logger.info(f"No Scenario associated with Plan of ID {model.id}") continue - scenario_model = get_scenario(model.scenario_id) - plans.append( - Plan.from_sqlalchemy(model, Scenario.from_sqlalchemy(scenario_model)) - ) - print("Successfully mapped plan and scenario to domain object") + plans.append(Plan.from_sqlalchemy(model, scenario)) + logger.info(f"Got {len(plans)} Plans") return plans @@ -57,37 +174,26 @@ def _group_plans_by_property(plans: List[Plan]) -> Dict[int, List[Plan]]: return grouped -def _choose_cheapest_relevant_plan(plans: List[Plan]) -> Plan: - plans_to_consider: List[Plan] = [p for p in plans if p.is_compliant] or plans - - def plan_cost(plan: Plan) -> float: - return ( - plan.record.cost_of_works - if plan.record.cost_of_works is not None - else float("inf") - ) - - cheapest_plan = min(plans_to_consider, key=plan_cost) - - return cheapest_plan - - -def _update_default_flags(plans: List[Plan], cheapest_plan: Plan) -> None: - plans_to_update: List[Plan] = [] - +def _update_plan_objects(plans: List[Plan], cheapest_plan: Plan) -> List[Plan]: for plan in plans: should_be_default: bool = plan.id == cheapest_plan.id - if plan.record.is_default != should_be_default: - plan.set_default(should_be_default) - plans_to_update.append(plan) + plan.set_default(should_be_default) - if plans_to_update: - plan_models: List[PlanModel] = [] - scenario_models: List[ScenarioModel] = [] + if should_be_default: + logger.debug( + f"Setting Plan {plan.id} (Scenario Name: {plan.scenario.record.name}) to default" + ) - for plan in plans_to_update: - plan_model, scenario_model = plan.to_sqlalchemy() - plan_models.append(plan_model) - scenario_models.append(scenario_model) + return plans - bulk_update_plans(plan_models, scenario_models) + +def _update_plans_in_db(plans: List[Plan]) -> None: + plan_models: List[PlanModel] = [] + scenario_models: List[ScenarioModel] = [] + + for plan in plans: + plan_model, scenario_model = plan.to_sqlalchemy() + plan_models.append(plan_model) + scenario_models.append(scenario_model) + + bulk_update_plans(plan_models, scenario_models) diff --git a/backend/categorisation/tests/test_prioritised_plan_selected.py b/backend/categorisation/tests/test_prioritised_plan_selected.py new file mode 100644 index 00000000..a9529a53 --- /dev/null +++ b/backend/categorisation/tests/test_prioritised_plan_selected.py @@ -0,0 +1,160 @@ +from datetime import datetime +from typing import List, Optional +import pytest + +from backend.app.domain.classes.plan import Plan +from backend.app.domain.classes.scenario import Scenario +from backend.app.domain.records.plan_record import PlanRecord +from backend.app.domain.records.scenario_record import ScenarioRecord +from backend.app.db.models.portfolio import Epc, PortfolioGoal +from backend.categorisation.processor import choose_cheapest_relevant_plan + + +@pytest.fixture +def created_at_datetime() -> datetime: + return datetime.now() + + +def make_plan_record( + created_at: datetime, default: bool, cost_of_works: Optional[float] = 500.0 +) -> PlanRecord: + return PlanRecord( + property_id=1, + portfolio_id=1, + created_at=created_at, + is_default=default, + post_epc_rating=Epc.C, + cost_of_works=cost_of_works, + ) + + +def make_scenario(name: str, created_at: datetime, is_default: bool) -> Scenario: + record = ScenarioRecord( + name=name, + created_at=created_at, + housing_type="", + goal=PortfolioGoal.INCREASING_EPC, + goal_value="C", + trigger_file_path="", + multi_plan=False, + is_default=is_default, + ) + return Scenario(record=record, id=3 if is_default else 4) + + +def make_plan( + created_at: datetime, + default: bool, + cost_of_works: Optional[float] = 500.0, + name: str = "", +) -> Plan: + scenario = make_scenario(name, created_at, default) + plan_id = 1 if default else 2 + return Plan( + record=make_plan_record(created_at, default, cost_of_works), + scenario=scenario, + id=plan_id, + ) + + +def test_prioritised_scenario_selected(created_at_datetime: datetime) -> None: + # arrange + epc_c_plan = make_plan(created_at_datetime, True, name="EPC C") + minor_works_plan = make_plan(created_at_datetime, False, name="EPC C - Minor Works") + scenario_priority_order: List[int] = [4, 3] + expected_default_plan_id = 2 + + # act + actual_default_plan = choose_cheapest_relevant_plan( + plans=[epc_c_plan, minor_works_plan], + scenario_priority_order=scenario_priority_order, + ) + + # assert + assert actual_default_plan.id == expected_default_plan_id + + +def test_cheapest_plan_returned_if_not_in_priority_list( + created_at_datetime: datetime, +) -> None: + # arrange + epc_c_plan = make_plan( + created_at_datetime, True, cost_of_works=1000.0, name="EPC C" + ) + minor_works_plan = make_plan( + created_at_datetime, False, cost_of_works=100.0, name="EPC C - Minor Works" + ) + scenario_priority_order: List[int] = [3, 5] + expected_default_plan_id = 2 + + # act + actual_default_plan = choose_cheapest_relevant_plan( + plans=[epc_c_plan, minor_works_plan], + scenario_priority_order=scenario_priority_order, + ) + + # assert + assert actual_default_plan.id == expected_default_plan_id + + +def test_all_plans_zero_cost__highest_priority_returned( + created_at_datetime: datetime, +) -> None: + # arrange + epc_c_plan = make_plan(created_at_datetime, True, cost_of_works=0.0, name="EPC C") + minor_works_plan = make_plan( + created_at_datetime, False, cost_of_works=0.0, name="EPC C - Minor Works" + ) + scenario_priority_order: List[int] = [4, 3] + expected_default_plan_id = 2 + + # act + actual_default_plan = choose_cheapest_relevant_plan( + plans=[epc_c_plan, minor_works_plan], + scenario_priority_order=scenario_priority_order, + ) + + # assert + assert actual_default_plan.id == expected_default_plan_id + + +def test_some_plans_zero_cost__cheapest_returned( + created_at_datetime: datetime, +) -> None: + # arrange + epc_c_plan = make_plan(created_at_datetime, True, cost_of_works=0.0, name="EPC C") + minor_works_plan = make_plan( + created_at_datetime, False, cost_of_works=50.0, name="EPC C - Minor Works" + ) + scenario_priority_order: List[int] = [4, 3] + expected_default_plan_id = 1 + + # act + actual_default_plan = choose_cheapest_relevant_plan( + plans=[epc_c_plan, minor_works_plan], + scenario_priority_order=scenario_priority_order, + ) + + # assert + assert actual_default_plan.id == expected_default_plan_id + + +def test_all_plans_null_cost__highest_priority_returned( + created_at_datetime: datetime, +) -> None: + # arrange + epc_c_plan = make_plan(created_at_datetime, True, cost_of_works=None, name="EPC C") + minor_works_plan = make_plan( + created_at_datetime, False, cost_of_works=None, name="EPC C - Minor Works" + ) + scenario_priority_order: List[int] = [4, 3] + expected_default_plan_id = 2 + + # act + actual_default_plan = choose_cheapest_relevant_plan( + plans=[epc_c_plan, minor_works_plan], + scenario_priority_order=scenario_priority_order, + ) + + # assert + assert actual_default_plan.id == expected_default_plan_id diff --git a/backend/docker-compose-local-lambdas.yml b/backend/docker-compose-local-lambdas.yml new file mode 100644 index 00000000..50e9193b --- /dev/null +++ b/backend/docker-compose-local-lambdas.yml @@ -0,0 +1,11 @@ +version: "3.9" + +services: + categorisation-lambda: + build: + context: ../ + dockerfile: backend/categorisation/handler/Dockerfile + ports: + - "9000:8080" + env_file: + - ../.env \ No newline at end of file diff --git a/backend/engine/engine.py b/backend/engine/engine.py index 80d6d078..8f6eca3f 100644 --- a/backend/engine/engine.py +++ b/backend/engine/engine.py @@ -1053,14 +1053,18 @@ async def model_engine(body: PlanTriggerRequest): property_required_measures = [m for m in recommendations[p.id] if m[0]["type"] in body.required_measures] measures_to_optimise = [m for m in recommendations[p.id] if m[0]["type"] not in body.required_measures] - ventilation_included = "ventilation" in property_measure_types + # TODO - formalise property measure types into an enum + ventilation_included = ( + "ventilation" in property_measure_types or "mechanical_ventilation" in property_measure_types + ) # If a measure requiring ventilation is selected, and the property does not have ventilation, we enfore # its inclusion - needs_ventilation = any( - x in property_measure_types for x in assumptions.measures_needing_ventilation - ) and not p.has_ventilation and ventilation_included + needs_ventilation = optimiser_functions.check_needs_ventilation( + property_measure_types, assumptions.measures_needing_ventilation, p.has_ventilation, + ventilation_included + ) if not measures_to_optimise: # Nothing to do, we just reshape the recommendations @@ -1177,7 +1181,7 @@ async def model_engine(body: PlanTriggerRequest): recommendations=recommendations, selected=selected, ) - # Add best practice measures (ventilation/trickle vents) + # Add best practice measures (ventilation/trickle vents) - pass needs_ventilation flag selected = optimiser_functions.add_best_practice_measures(p.id, solution, recommendations, selected) # Final flattening - we pass what the battery SAP score would be, regardless if the battery was selected recommendations[p.id] = optimiser_functions.flatten_recommendations_with_defaults( diff --git a/recommendations/Recommendations.py b/recommendations/Recommendations.py index acd49e05..80cc06b4 100644 --- a/recommendations/Recommendations.py +++ b/recommendations/Recommendations.py @@ -499,8 +499,16 @@ class Recommendations: return predicted_appliances_cost_reduction, predicted_appliances_kwh_reduction @staticmethod - def _check_ventilation_out_of_bounds(sap_impact, ventilation_sap_limit): - return (sap_impact < ventilation_sap_limit) or (sap_impact >= 0) + def _check_ventilation_out_of_bounds(sap_impact: float, ventilation_sap_limit: float) -> bool: + """ + Checks if the SAP impact of a ventilation recommendation is out of bounds, which would indicate that the + recommendation is not appropriate. + :param sap_impact: The SAP impact of the ventilation recommendation, which is typically negative or zero + :param ventilation_sap_limit: The SAP limit for ventilation recommendations, which is typically a negative + number. E.g. -4 + :return: + """ + return (sap_impact < ventilation_sap_limit) or (sap_impact > 0) @staticmethod def _adjust_ventilation_sap(sap_impact, ventilation_sap_limit): @@ -574,6 +582,7 @@ class Recommendations: if rec_phase == starting_phase: return { "sap": float(property_instance.data["current-energy-efficiency"]), + "sap_prediction": float(property_instance.data["current-energy-efficiency"]), "carbon": float(property_instance.data["co2-emissions-current"]), "heat_demand": float(property_instance.data["energy-consumption-current"]), } @@ -591,12 +600,13 @@ class Recommendations: if not previous_phase_reps: return { "sap": float(property_instance.data["current-energy-efficiency"]), + "sap_prediction": 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") + keys = ("sap", "sap_prediction", "carbon", "heat_demand") return { key: np.median([item[key] for item in previous_phase_reps]) for key in keys @@ -691,7 +701,8 @@ class Recommendations: previous_phase_values: dict, current_phase_values: dict, adjustments: list, - property_instance, + property_instance: Property, + model_predicted_sap: float, ): # For the moment, we cap the number of SAP points that can be achieved by LEDs at 2 if rec["type"] == "low_energy_lighting": @@ -785,7 +796,6 @@ class Recommendations: # Update the current phase values current_phase_values["sap"] = previous_phase_values["sap"] + property_phase_impact["sap"] - elif rec["type"] == "loft_insulation": # When we have a loft insulation recommendation, where there is an extension and the existing # amount of loft insulation is already good, we limit the SAP points @@ -831,6 +841,27 @@ class Recommendations: # Update the current phase values current_phase_values["sap"] = previous_phase_values["sap"] + property_phase_impact["sap"] + elif rec["measure_type"] in ["roomstat_programmer_trvs", "time_temperature_zone_control"]: + # We trim the SAP point recommendations based on the minimum of the predicted and the survey SAP + # points + predicted_difference = model_predicted_sap - previous_phase_values["sap_prediction"] + proposed_impact = property_phase_impact["sap"] + numerically_the_same = np.isclose(proposed_impact, predicted_difference) + + if predicted_difference > 0 and (predicted_difference < proposed_impact) and not numerically_the_same: + # We constrain the impact based on what the model predicts. + # We update the proposed impact to be the predicted difference + adjustments.append( + { + "recommendation_id": rec["recommendation_id"], + "phase": rec["phase"], + # If we've made an adjustment, it will be negative + "sap_adjustment": property_phase_impact["sap"] - predicted_difference, + } + ) + property_phase_impact["sap"] = predicted_difference + # Update the current phase values + current_phase_values["sap"] = previous_phase_values["sap"] + property_phase_impact["sap"] return property_phase_impact, current_phase_values, adjustments @@ -963,7 +994,8 @@ class Recommendations: previous_phase_values=previous_phase_values, current_phase_values=current_phase_values, adjustments=adjustments, - property_instance=property_instance + property_instance=property_instance, + model_predicted_sap=phase_energy_efficiency_metrics["sap_change"], ) # Insert this information into the recommendation. diff --git a/recommendations/SecondaryHeating.py b/recommendations/SecondaryHeating.py index ee7eae1c..ef0fc2d2 100644 --- a/recommendations/SecondaryHeating.py +++ b/recommendations/SecondaryHeating.py @@ -18,6 +18,9 @@ class SecondaryHeating: def recommend(self, phase: int): # Reset self.recommendation = [] + if self.property.epc_record.secondheat_description in ["None", None]: + # No secondary heating system, so no recommendation to remove it + return if self.property.data['number-habitable-rooms'] > self.property.data['number-heated-rooms']: n_rooms = self.property.data['number-habitable-rooms'] - self.property.data['number-heated-rooms'] diff --git a/recommendations/optimiser/funding_optimiser.py b/recommendations/optimiser/funding_optimiser.py index 324e2c74..69a6bc48 100644 --- a/recommendations/optimiser/funding_optimiser.py +++ b/recommendations/optimiser/funding_optimiser.py @@ -655,6 +655,11 @@ def optimise_with_scenarios( 1) With air source heat pump AND required insulation """ + # Universally handle zero gain + if target_gain is not None: + if target_gain <= 0: + return pd.DataFrame([]) + solutions = [] paths = [] # Produce the unique list of measure types @@ -770,6 +775,11 @@ def optimise_with_scenarios( for fixed in fixed_selections: + if target_gain is not None: + if target_gain <= 0: + # If we don't have any gain, we don't actually need to do this + continue + # fixed = [(gi, oi, opt), ...] fixed_items = [opt for (_, _, opt) in fixed] fixed_groups = {gi for (gi, _, _) in fixed} diff --git a/recommendations/optimiser/optimiser_functions.py b/recommendations/optimiser/optimiser_functions.py index d704b3fb..6fd70c20 100644 --- a/recommendations/optimiser/optimiser_functions.py +++ b/recommendations/optimiser/optimiser_functions.py @@ -1,4 +1,5 @@ import pandas as pd +from typing import List, Dict, Any, Set import backend.app.assumptions as assumptions from backend.Property import Property from backend.app.plan.schemas import PlanTriggerRequest @@ -78,14 +79,14 @@ def prepare_input_measures( # if recs[0]["type"] == "solar_pv": # recs = [r for r in recs if ~r["has_battery"]] - # Only include measures with non-negative cost savings + # Only include measures with non-negative cost savings - we allow for a minor negative impact if eco_measures: recs_to_append = [ - rec for rec in recs if (rec["energy_cost_savings"] >= 0) or (rec["measure_type"] in eco_measures) + rec for rec in recs if (rec["energy_cost_savings"] >= -10) or (rec["measure_type"] in eco_measures) ] else: recs_to_append = [ - rec for rec in recs if (rec["energy_cost_savings"] >= 0) + rec for rec in recs if (rec["energy_cost_savings"] >= -10) ] if not recs_to_append: continue @@ -300,7 +301,12 @@ def add_required_measures(property_id, property_required_measures, recommendatio ] -def add_best_practice_measures(property_id, solution, recommendations, selected): +def add_best_practice_measures( + property_id: int, + solution: List[Dict[str, Any]], + recommendations: Dict[int, List[List[Dict[str, Any]]]], + selected: Set[str], +): """ Ensures best-practice measures like ventilation and trickle vents are included in the selected recommendations when appropriate. @@ -331,11 +337,11 @@ def add_best_practice_measures(property_id, solution, recommendations, selected) # If ventilation has been selected, or one of the measures needs ventilation, we need to ensure ventilation is # included - needs_ventilation = any( + measures_selected_needing_ventilation = any( x in [r["type"] for r in solution] for x in assumptions.measures_needing_ventilation - ) or len(ventilation_selected) > 0 + ) - if needs_ventilation: + if measures_selected_needing_ventilation or len(ventilation_selected) > 0: ventilation_rec = next( (r[0] for r in recommendations[property_id] if r[0]["type"] == "mechanical_ventilation"), None @@ -395,3 +401,30 @@ def flatten_recommendations_with_defaults(property_id, recommendations, selected # Flatten the nested list of lists into a single list return [rec for recommendations_by_type in final_recommendations for rec in recommendations_by_type] + + +def check_needs_ventilation( + property_measure_types: Set[str], + measures_needing_ventilation: List[str], + property_already_has_ventilation: bool, + ventilation_in_included_measures: bool +) -> bool: + """ + Function to check if we need to include ventilation based on the measures selected and the property + features + :param property_measure_types: The set of measure types recommended for the property + :param measures_needing_ventilation: The set of measure types that require ventilation + :param property_already_has_ventilation: Whether the property currently has ventilation + :param ventilation_in_included_measures: Whether ventilation is already included in the recommended + measures + :return: Boolean indicating whether ventilation needs to be included in the recommendations + + # TODO - none of the inputs of this function are well structured and so this is quite brittle - we should + consider refactoring to make this more robust + """ + + needs_ventilation = any( + x in property_measure_types for x in measures_needing_ventilation + ) + + return needs_ventilation and not property_already_has_ventilation and ventilation_in_included_measures diff --git a/recommendations/tests/test_optimiser_functions.py b/recommendations/tests/test_optimiser_functions.py index f0ca6dac..0a31ae2c 100644 --- a/recommendations/tests/test_optimiser_functions.py +++ b/recommendations/tests/test_optimiser_functions.py @@ -3,9 +3,19 @@ import numpy as np from types import SimpleNamespace from recommendations.tests.test_data.measures_to_optimise import measures_to_optimise from recommendations.optimiser import optimiser_functions +from recommendations.optimiser.funding_optimiser import optimise_with_scenarios from recommendations.optimiser.GainOptimiser import GainOptimiser from recommendations.optimiser.CostOptimiser import CostOptimiser -from recommendations.optimiser.StrategicOptimiser import StrategicOptimiser, Strategies +from recommendations.optimiser.StrategicOptimiser import StrategicOptimiser + + +@pytest.fixture +def property_instance(): + return SimpleNamespace( + id="P1", + has_ventilation=False, + data={"current-energy-efficiency": "52"}, + ) class TestPrepareInputMeasures: @@ -48,8 +58,9 @@ class TestPrepareInputMeasures: def test_filters_out_negative_cost_savings(self): recs = [ [{"recommendation_id": "bad1", "type": "loft_insulation", "total": 200, "kwh_savings": 100, - "energy_cost_savings": -5, "has_battery": False, - "partial_project_funding": 0, "partial_project_score": 0, "uplift_project_score": 0, }], + "energy_cost_savings": -100, "has_battery": False, + "partial_project_funding": 0, "partial_project_score": 0, "uplift_project_score": 0, + "measure_type": "roof_insulation"}], ] measures = optimiser_functions.prepare_input_measures(recs, goal="Energy Savings", needs_ventilation=False) assert measures == [] # should skip negative cost saving recs @@ -143,7 +154,9 @@ class TestAddBestPracticeMeasures: ] } selected = set() - updated = optimiser_functions.add_best_practice_measures(property_id, solution, recommendations, selected) + updated = optimiser_functions.add_best_practice_measures( + property_id, solution, recommendations, selected + ) assert "vent1" in updated assert "trickle1" in updated @@ -510,3 +523,322 @@ class TestStrategicOptimiser: assert opt.strategy_used.value == "case_2_solve_max_gain_under_budget" assert opt.solution_cost == 7787.068 assert opt.solution_gain == 28.8 + + +class TestCheckNeedsVentilation: + + def measure_types_includes_ventilation_no_existing_ventilation(self): + property_measure_types = {'mechanical_ventilation', 'cavity_wall_insulation', 'suspended_floor_insulation', + 'secondary_heating', 'loft_insulation', 'heating', 'low_energy_lighting'} + + measures_needing_ventilation = ['internal_wall_insulation', 'external_wall_insulation', + 'cavity_wall_insulation'] + + has_ventilation = False + + ventilation_included = True + + result = optimiser_functions.check_needs_ventilation( + property_measure_types, measures_needing_ventilation, has_ventilation, + ventilation_included + ) + + assert result == True + + def measure_types_includes_ventilation_existing_ventilation(self): + property_measure_types = {'mechanical_ventilation', 'cavity_wall_insulation', 'suspended_floor_insulation', + 'secondary_heating', 'loft_insulation', 'heating', 'low_energy_lighting'} + + measures_needing_ventilation = ['internal_wall_insulation', 'external_wall_insulation', + 'cavity_wall_insulation'] + + has_ventilation = True + + ventilation_included = True + + result = optimiser_functions.check_needs_ventilation( + property_measure_types, measures_needing_ventilation, has_ventilation, + ventilation_included + ) + + assert result == False + + def measure_types_includes_ventilation_existing_ventilation(self): + property_measure_types_without_ventilation = { + 'cavity_wall_insulation', 'suspended_floor_insulation', + 'secondary_heating', 'loft_insulation', 'heating', + 'low_energy_lighting' + } + + measures_needing_ventilation = ['internal_wall_insulation', 'external_wall_insulation', + 'cavity_wall_insulation'] + + has_ventilation = False + + ventilation_included = True + + result = optimiser_functions.check_needs_ventilation( + property_measure_types_without_ventilation, measures_needing_ventilation, has_ventilation, + ventilation_included + ) + + assert result == False + + +class TestOptimiseWithScenarios: + + def test_zero_gain(self, property_instance): + input_measures = [[{'id': '0_phase=0', 'cost': 16901.01977922431, 'gain': np.float64(2.0), + 'type': 'internal_wall_insulation+mechanical_ventilation', 'innovation_uplift': 0, + 'cost_minus_uplift': 16901.01977922431, 'raw_cost': 16341.019779224309, + 'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0, + 'already_installed': False, 'has_battery': False, 'array_size': 0}], + [{'id': '1_phase=1', 'cost': 1197.0, 'gain': 0, 'type': 'loft_insulation', + 'innovation_uplift': 0, 'cost_minus_uplift': 1197.0, 'raw_cost': 1197.0, + 'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0, + 'already_installed': False, 'has_battery': False, 'array_size': 0}, + {'id': '2_phase=1', 'cost': 1026.0, 'gain': 0, 'type': 'loft_insulation', + 'innovation_uplift': 0, 'cost_minus_uplift': 1026.0, 'raw_cost': 1026.0, + 'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0, + 'already_installed': False, 'has_battery': False, 'array_size': 0}, + {'id': '3_phase=1', 'cost': 855.0, 'gain': 0, 'type': 'loft_insulation', + 'innovation_uplift': 0, 'cost_minus_uplift': 855.0, 'raw_cost': 855.0, + 'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0, + 'already_installed': False, 'has_battery': False, 'array_size': 0}], + [{'id': '5_phase=3', 'cost': 5343.75, 'gain': 1, 'type': 'suspended_floor_insulation', + 'innovation_uplift': 0, 'cost_minus_uplift': 5343.75, 'raw_cost': 5343.75, + 'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0, + 'already_installed': False, 'has_battery': False, 'array_size': 0}], + [{'id': '6_phase=4', 'cost': 1009.5600000000001, 'gain': np.float64(0.9000000000000057), + 'type': 'time_temperature_zone_control', 'innovation_uplift': 0, + 'cost_minus_uplift': 1009.5600000000001, 'raw_cost': 1009.5600000000001, + 'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0, + 'already_installed': False, 'has_battery': False, 'array_size': 0}, + {'id': '7_phase=4', 'cost': 18979.9, 'gain': np.float64(6.9), 'type': 'air_source_heat_pump', + 'innovation_uplift': 0, 'cost_minus_uplift': 18979.9, 'raw_cost': 18979.9, + 'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0, + 'already_installed': False, 'has_battery': False, 'array_size': 0}], + [{'id': '8_phase=5', 'cost': 5420.0, 'gain': np.float64(9.0), 'type': 'solar_pv', + 'innovation_uplift': 0, 'cost_minus_uplift': 5420.0, 'raw_cost': 5420.0, + 'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0, + 'already_installed': False, 'has_battery': False, 'array_size': 3.6}, + {'id': '9_phase=5', 'cost': 6210.0, 'gain': np.float64(9.0), 'type': 'solar_pv', + 'innovation_uplift': 0, 'cost_minus_uplift': 6210.0, 'raw_cost': 6210.0, + 'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0, + 'already_installed': False, 'has_battery': True, 'array_size': 3.6}, + {'id': '10_phase=5', 'cost': 6820.0, 'gain': np.float64(9.0), 'type': 'solar_pv', + 'innovation_uplift': 0, 'cost_minus_uplift': 6820.0, 'raw_cost': 6820.0, + 'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0, + 'already_installed': False, 'has_battery': True, 'array_size': 3.6}, + {'id': '11_phase=5', 'cost': 7202.0, 'gain': np.float64(10.0), 'type': 'solar_pv', + 'innovation_uplift': 0, 'cost_minus_uplift': 7202.0, 'raw_cost': 7202.0, + 'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0, + 'already_installed': False, 'has_battery': False, 'array_size': 3.915}, + {'id': '12_phase=5', 'cost': 6495.0, 'gain': np.float64(10.0), 'type': 'solar_pv', + 'innovation_uplift': 0, 'cost_minus_uplift': 6495.0, 'raw_cost': 6495.0, + 'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0, + 'already_installed': False, 'has_battery': False, 'array_size': 3.92}, + {'id': '13_phase=5', 'cost': 7285.0, 'gain': np.float64(10.0), 'type': 'solar_pv', + 'innovation_uplift': 0, 'cost_minus_uplift': 7285.0, 'raw_cost': 7285.0, + 'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0, + 'already_installed': False, 'has_battery': True, 'array_size': 3.92}, + {'id': '14_phase=5', 'cost': 7895.0, 'gain': np.float64(10.0), 'type': 'solar_pv', + 'innovation_uplift': 0, 'cost_minus_uplift': 7895.0, 'raw_cost': 7895.0, + 'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0, + 'already_installed': False, 'has_battery': True, 'array_size': 3.92}, + {'id': '15_phase=5', 'cost': 5520.0, 'gain': np.float64(10.0), 'type': 'solar_pv', + 'innovation_uplift': 0, 'cost_minus_uplift': 5520.0, 'raw_cost': 5520.0, + 'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0, + 'already_installed': False, 'has_battery': False, 'array_size': 4.0}, + {'id': '16_phase=5', 'cost': 6310.0, 'gain': np.float64(10.0), 'type': 'solar_pv', + 'innovation_uplift': 0, 'cost_minus_uplift': 6310.0, 'raw_cost': 6310.0, + 'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0, + 'already_installed': False, 'has_battery': True, 'array_size': 4.0}, + {'id': '17_phase=5', 'cost': 6920.0, 'gain': np.float64(10.0), 'type': 'solar_pv', + 'innovation_uplift': 0, 'cost_minus_uplift': 6920.0, 'raw_cost': 6920.0, + 'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0, + 'already_installed': False, 'has_battery': True, 'array_size': 4.0}, + {'id': '18_phase=5', 'cost': 5840.0, 'gain': np.float64(13.0), 'type': 'solar_pv', + 'innovation_uplift': 0, 'cost_minus_uplift': 5840.0, 'raw_cost': 5840.0, + 'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0, + 'already_installed': False, 'has_battery': False, 'array_size': 5.2}, + {'id': '19_phase=5', 'cost': 6630.0, 'gain': np.float64(13.0), 'type': 'solar_pv', + 'innovation_uplift': 0, 'cost_minus_uplift': 6630.0, 'raw_cost': 6630.0, + 'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0, + 'already_installed': False, 'has_battery': True, 'array_size': 5.2}, + {'id': '20_phase=5', 'cost': 7240.0, 'gain': np.float64(13.0), 'type': 'solar_pv', + 'innovation_uplift': 0, 'cost_minus_uplift': 7240.0, 'raw_cost': 7240.0, + 'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0, + 'already_installed': False, 'has_battery': True, 'array_size': 5.2}, + {'id': '21_phase=5', 'cost': 8630.0, 'gain': np.float64(14.0), 'type': 'solar_pv', + 'innovation_uplift': 0, 'cost_minus_uplift': 8630.0, 'raw_cost': 8630.0, + 'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0, + 'already_installed': False, 'has_battery': False, 'array_size': 5.655}, + {'id': '22_phase=5', 'cost': 7660.0, 'gain': np.float64(14.0), 'type': 'solar_pv', + 'innovation_uplift': 0, 'cost_minus_uplift': 7660.0, 'raw_cost': 7660.0, + 'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0, + 'already_installed': False, 'has_battery': False, 'array_size': 5.66}, + {'id': '23_phase=5', 'cost': 8470.0, 'gain': np.float64(14.0), 'type': 'solar_pv', + 'innovation_uplift': 0, 'cost_minus_uplift': 8470.0, 'raw_cost': 8470.0, + 'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0, + 'already_installed': False, 'has_battery': True, 'array_size': 5.66}, + {'id': '24_phase=5', 'cost': 9090.0, 'gain': np.float64(14.0), 'type': 'solar_pv', + 'innovation_uplift': 0, 'cost_minus_uplift': 9090.0, 'raw_cost': 9090.0, + 'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0, + 'already_installed': False, 'has_battery': True, 'array_size': 5.66}, + {'id': '25_phase=5', 'cost': 7240.0, 'gain': np.float64(12.0), 'type': 'solar_pv', + 'innovation_uplift': 0, 'cost_minus_uplift': 7240.0, 'raw_cost': 7240.0, + 'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0, + 'already_installed': False, 'has_battery': False, 'array_size': 4.79}, + {'id': '26_phase=5', 'cost': 8050.0, 'gain': np.float64(12.0), 'type': 'solar_pv', + 'innovation_uplift': 0, 'cost_minus_uplift': 8050.0, 'raw_cost': 8050.0, + 'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0, + 'already_installed': False, 'has_battery': True, 'array_size': 4.79}, + {'id': '27_phase=5', 'cost': 8660.0, 'gain': np.float64(12.0), 'type': 'solar_pv', + 'innovation_uplift': 0, 'cost_minus_uplift': 8660.0, 'raw_cost': 8660.0, + 'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0, + 'already_installed': False, 'has_battery': True, 'array_size': 4.79}, + {'id': '28_phase=5', 'cost': 5740.0, 'gain': np.float64(12.0), 'type': 'solar_pv', + 'innovation_uplift': 0, 'cost_minus_uplift': 5740.0, 'raw_cost': 5740.0, + 'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0, + 'already_installed': False, 'has_battery': False, 'array_size': 4.8}, + {'id': '29_phase=5', 'cost': 6530.0, 'gain': np.float64(12.0), 'type': 'solar_pv', + 'innovation_uplift': 0, 'cost_minus_uplift': 6530.0, 'raw_cost': 6530.0, + 'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0, + 'already_installed': False, 'has_battery': True, 'array_size': 4.8}, + {'id': '30_phase=5', 'cost': 7140.0, 'gain': np.float64(12.0), 'type': 'solar_pv', + 'innovation_uplift': 0, 'cost_minus_uplift': 7140.0, 'raw_cost': 7140.0, + 'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0, + 'already_installed': False, 'has_battery': True, 'array_size': 4.8}, + {'id': '31_phase=5', 'cost': 8360.0, 'gain': np.float64(13.0), 'type': 'solar_pv', + 'innovation_uplift': 0, 'cost_minus_uplift': 8360.0, 'raw_cost': 8360.0, + 'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0, + 'already_installed': False, 'has_battery': False, 'array_size': 5.22}, + {'id': '32_phase=5', 'cost': 7470.0, 'gain': np.float64(13.0), 'type': 'solar_pv', + 'innovation_uplift': 0, 'cost_minus_uplift': 7470.0, 'raw_cost': 7470.0, + 'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0, + 'already_installed': False, 'has_battery': False, 'array_size': 5.22}, + {'id': '33_phase=5', 'cost': 8280.0, 'gain': np.float64(13.0), 'type': 'solar_pv', + 'innovation_uplift': 0, 'cost_minus_uplift': 8280.0, 'raw_cost': 8280.0, + 'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0, + 'already_installed': False, 'has_battery': True, 'array_size': 5.22}, + {'id': '34_phase=5', 'cost': 8890.0, 'gain': np.float64(13.0), 'type': 'solar_pv', + 'innovation_uplift': 0, 'cost_minus_uplift': 8890.0, 'raw_cost': 8890.0, + 'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0, + 'already_installed': False, 'has_battery': True, 'array_size': 5.22}, + {'id': '35_phase=5', 'cost': 5892.21, 'gain': np.float64(13.0), 'type': 'solar_pv', + 'innovation_uplift': 0, 'cost_minus_uplift': 5892.21, 'raw_cost': 5892.21, + 'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0, + 'already_installed': False, 'has_battery': False, 'array_size': 5.34}, + {'id': '36_phase=5', 'cost': 5320.0, 'gain': np.float64(8.0), 'type': 'solar_pv', + 'innovation_uplift': 0, 'cost_minus_uplift': 5320.0, 'raw_cost': 5320.0, + 'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0, + 'already_installed': False, 'has_battery': False, 'array_size': 3.2}, + {'id': '37_phase=5', 'cost': 6110.0, 'gain': np.float64(8.0), 'type': 'solar_pv', + 'innovation_uplift': 0, 'cost_minus_uplift': 6110.0, 'raw_cost': 6110.0, + 'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0, + 'already_installed': False, 'has_battery': True, 'array_size': 3.2}, + {'id': '38_phase=5', 'cost': 6720.0, 'gain': np.float64(8.0), 'type': 'solar_pv', + 'innovation_uplift': 0, 'cost_minus_uplift': 6720.0, 'raw_cost': 6720.0, + 'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0, + 'already_installed': False, 'has_battery': True, 'array_size': 3.2}, + {'id': '39_phase=5', 'cost': 6932.0, 'gain': np.float64(9.0), 'type': 'solar_pv', + 'innovation_uplift': 0, 'cost_minus_uplift': 6932.0, 'raw_cost': 6932.0, + 'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0, + 'already_installed': False, 'has_battery': False, 'array_size': 3.48}, + {'id': '40_phase=5', 'cost': 6295.0, 'gain': np.float64(9.0), 'type': 'solar_pv', + 'innovation_uplift': 0, 'cost_minus_uplift': 6295.0, 'raw_cost': 6295.0, + 'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0, + 'already_installed': False, 'has_battery': False, 'array_size': 3.48}, + {'id': '41_phase=5', 'cost': 7085.0, 'gain': np.float64(9.0), 'type': 'solar_pv', + 'innovation_uplift': 0, 'cost_minus_uplift': 7085.0, 'raw_cost': 7085.0, + 'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0, + 'already_installed': False, 'has_battery': True, 'array_size': 3.48}, + {'id': '42_phase=5', 'cost': 7695.0, 'gain': np.float64(9.0), 'type': 'solar_pv', + 'innovation_uplift': 0, 'cost_minus_uplift': 7695.0, 'raw_cost': 7695.0, + 'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0, + 'already_installed': False, 'has_battery': True, 'array_size': 3.48}, + {'id': '43_phase=5', 'cost': 5640.0, 'gain': np.float64(11.0), 'type': 'solar_pv', + 'innovation_uplift': 0, 'cost_minus_uplift': 5640.0, 'raw_cost': 5640.0, + 'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0, + 'already_installed': False, 'has_battery': False, 'array_size': 4.4}, + {'id': '44_phase=5', 'cost': 6430.0, 'gain': np.float64(11.0), 'type': 'solar_pv', + 'innovation_uplift': 0, 'cost_minus_uplift': 6430.0, 'raw_cost': 6430.0, + 'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0, + 'already_installed': False, 'has_battery': True, 'array_size': 4.4}, + {'id': '45_phase=5', 'cost': 7040.0, 'gain': np.float64(11.0), 'type': 'solar_pv', + 'innovation_uplift': 0, 'cost_minus_uplift': 7040.0, 'raw_cost': 7040.0, + 'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0, + 'already_installed': False, 'has_battery': True, 'array_size': 4.4}, + {'id': '46_phase=5', 'cost': 8090.0, 'gain': np.float64(12.0), 'type': 'solar_pv', + 'innovation_uplift': 0, 'cost_minus_uplift': 8090.0, 'raw_cost': 8090.0, + 'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0, + 'already_installed': False, 'has_battery': False, 'array_size': 4.785}, + {'id': '47_phase=5', 'cost': 7240.0, 'gain': np.float64(12.0), 'type': 'solar_pv', + 'innovation_uplift': 0, 'cost_minus_uplift': 7240.0, 'raw_cost': 7240.0, + 'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0, + 'already_installed': False, 'has_battery': False, 'array_size': 4.79}, + {'id': '48_phase=5', 'cost': 8050.0, 'gain': np.float64(12.0), 'type': 'solar_pv', + 'innovation_uplift': 0, 'cost_minus_uplift': 8050.0, 'raw_cost': 8050.0, + 'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0, + 'already_installed': False, 'has_battery': True, 'array_size': 4.79}, + {'id': '49_phase=5', 'cost': 8660.0, 'gain': np.float64(12.0), 'type': 'solar_pv', + 'innovation_uplift': 0, 'cost_minus_uplift': 8660.0, 'raw_cost': 8660.0, + 'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0, + 'already_installed': False, 'has_battery': True, 'array_size': 4.79}, + {'id': '50_phase=5', 'cost': 5520.0, 'gain': np.float64(10.0), 'type': 'solar_pv', + 'innovation_uplift': 0, 'cost_minus_uplift': 5520.0, 'raw_cost': 5520.0, + 'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0, + 'already_installed': False, 'has_battery': False, 'array_size': 4.0}, + {'id': '51_phase=5', 'cost': 6310.0, 'gain': np.float64(10.0), 'type': 'solar_pv', + 'innovation_uplift': 0, 'cost_minus_uplift': 6310.0, 'raw_cost': 6310.0, + 'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0, + 'already_installed': False, 'has_battery': True, 'array_size': 4.0}, + {'id': '52_phase=5', 'cost': 6920.0, 'gain': np.float64(10.0), 'type': 'solar_pv', + 'innovation_uplift': 0, 'cost_minus_uplift': 6920.0, 'raw_cost': 6920.0, + 'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0, + 'already_installed': False, 'has_battery': True, 'array_size': 4.0}, + {'id': '53_phase=5', 'cost': 7820.0, 'gain': np.float64(11.0), 'type': 'solar_pv', + 'innovation_uplift': 0, 'cost_minus_uplift': 7820.0, 'raw_cost': 7820.0, + 'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0, + 'already_installed': False, 'has_battery': False, 'array_size': 4.35}, + {'id': '54_phase=5', 'cost': 6675.0, 'gain': np.float64(11.0), 'type': 'solar_pv', + 'innovation_uplift': 0, 'cost_minus_uplift': 6675.0, 'raw_cost': 6675.0, + 'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0, + 'already_installed': False, 'has_battery': False, 'array_size': 4.35}, + {'id': '55_phase=5', 'cost': 7485.0, 'gain': np.float64(11.0), 'type': 'solar_pv', + 'innovation_uplift': 0, 'cost_minus_uplift': 7485.0, 'raw_cost': 7485.0, + 'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0, + 'already_installed': False, 'has_battery': True, 'array_size': 4.35}, + {'id': '56_phase=5', 'cost': 8095.0, 'gain': np.float64(11.0), 'type': 'solar_pv', + 'innovation_uplift': 0, 'cost_minus_uplift': 8095.0, 'raw_cost': 8095.0, + 'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0, + 'already_installed': False, 'has_battery': True, 'array_size': 4.35}, + {'id': '57_phase=5', 'cost': 5640.0, 'gain': np.float64(11.0), 'type': 'solar_pv', + 'innovation_uplift': 0, 'cost_minus_uplift': 5640.0, 'raw_cost': 5640.0, + 'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0, + 'already_installed': False, 'has_battery': False, 'array_size': 4.4}, + {'id': '58_phase=5', 'cost': 6430.0, 'gain': np.float64(11.0), 'type': 'solar_pv', + 'innovation_uplift': 0, 'cost_minus_uplift': 6430.0, 'raw_cost': 6430.0, + 'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0, + 'already_installed': False, 'has_battery': True, 'array_size': 4.4}, + {'id': '59_phase=5', 'cost': 7040.0, 'gain': np.float64(11.0), 'type': 'solar_pv', + 'innovation_uplift': 0, 'cost_minus_uplift': 7040.0, 'raw_cost': 7040.0, + 'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0, + 'already_installed': False, 'has_battery': True, 'array_size': 4.4}, + {'id': '60_phase=5', 'cost': 5692.21, 'gain': np.float64(11.0), 'type': 'solar_pv', + 'innovation_uplift': 0, 'cost_minus_uplift': 5692.21, 'raw_cost': 5692.21, + 'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0, + 'already_installed': False, 'has_battery': False, 'array_size': 4.45}]] + + solutions = optimise_with_scenarios( + p=property_instance, + input_measures=input_measures, + budget=None, + target_gain=0, + enforce_heat_pump_insulation=True, + enforce_fabric_first=False, + already_installed_sap=0, # To be passed to output + ) + + assert solutions.empty diff --git a/recommendations/tests/test_recommendations.py b/recommendations/tests/test_recommendations.py index e3bcbb2f..2218cd16 100644 --- a/recommendations/tests/test_recommendations.py +++ b/recommendations/tests/test_recommendations.py @@ -373,7 +373,7 @@ def test_filter_phase_adjustment(input_data, expected): "sap_impact, limit, expected", [ (1.0, -4, True), # positive SAP not allowed - (0.0, -4, True), # zero not allowed + (0.0, -4, False), # zero is allowed (-1.0, -4, False), # valid range (-3.9, -4, False), # valid range (-4.0, -4, False), # exact lower bound allowed @@ -401,7 +401,7 @@ def test_adjust_ventilation_sap(sap_impact, limit, expected): ) == expected -def test_get_previous_phase_values_starting_phase(property_instance): +def test_get_previous_phase_values_phase_0_starting_phase_0(property_instance): result = Recommendations._get_previous_phase_values( rec_phase=0, starting_phase=0, @@ -411,6 +411,7 @@ def test_get_previous_phase_values_starting_phase(property_instance): assert result == { "sap": 65.0, + "sap_prediction": 65.0, "carbon": 2.4, "heat_demand": 284.0, } @@ -441,8 +442,8 @@ def test_get_previous_phase_values_single_rep(property_instance): 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}, + {"phase": 1, "representative": True, "sap": 70, "carbon": 2.0, "heat_demand": 250, "sap_prediction": 70}, + {"phase": 1, "representative": True, "sap": 74, "carbon": 1.6, "heat_demand": 230, "sap_prediction": 74}, ] result = Recommendations._get_previous_phase_values( @@ -1476,7 +1477,9 @@ def test_lighting_and_loft_adjustment_combined(property_instance, heat_demand_pr assert adjustments2 == [ {'recommendation_id': '0_phase=0', 'phase': 0, 'sap_adjustment': np.float64(1.7)}, - {'recommendation_id': '4_phase=2', 'phase': 2, 'sap_adjustment': np.float64(4.0)} + {'recommendation_id': '4_phase=2', 'phase': 2, 'sap_adjustment': np.float64(4.0)}, + {'recommendation_id': '5_phase=3', 'phase': 3, 'sap_adjustment': np.float64(1.0)}, + {'recommendation_id': '6_phase=3', 'phase': 3, 'sap_adjustment': np.float64(1.0000000000000027)} ] @@ -1499,7 +1502,8 @@ def test_mechanical_ventilation_sap_floor(property_instance): previous_phase_values=previous_phase_values, current_phase_values=current_phase_values, adjustments=adjustments, - property_instance=property_instance + property_instance=property_instance, + model_predicted_sap=0 ) ) @@ -1538,7 +1542,8 @@ def test_mechanical_ventilation_no_floor_adjustment(property_instance): previous_phase_values=previous_phase_values, current_phase_values=current_phase_values, adjustments=adjustments, - property_instance=property_instance + property_instance=property_instance, + model_predicted_sap=0 ) ) @@ -1570,7 +1575,8 @@ def test_mechanical_ventilation_exactly_one_no_adjustment(property_instance): previous_phase_values=previous_phase_values, current_phase_values=current_phase_values, adjustments=adjustments, - property_instance=property_instance + property_instance=property_instance, + model_predicted_sap=0 ) ) @@ -1578,3 +1584,182 @@ def test_mechanical_ventilation_exactly_one_no_adjustment(property_instance): assert updated_adjustments == [] assert updated_current["sap"] == 1.0 assert updated_impact["sap"] == -1.0 + + +def test_mechanical_ventilation_sap_zero_no_adjustment(property_instance): + # Test when SAP = 0 + rec = { + "type": "mechanical_ventilation", + "recommendation_id": "mv_test", + "phase": 1, + } + + previous_phase_values = {'phase': 0, 'representative': True, 'recommendation_id': '0_phase=0', + 'measure_type': 'flat_roof_insulation', 'sap': 68.0, 'carbon': np.float64(0.5), + 'heat_demand': np.float64(300.1), 'sap_prediction': np.float64(71.7)} + current_phase_values = {'sap': 68.0, 'carbon': np.float64(0.5), 'heat_demand': np.float64(307.0)} + property_phase_impact = {'sap': 0, 'carbon': 0, 'heat_demand': np.float64(-6.899999999999977)} + adjustments = [] + + updated_impact, updated_current, updated_adjustments = ( + Recommendations._apply_measure_specific_rules( + rec=rec, + property_phase_impact=property_phase_impact, + previous_phase_values=previous_phase_values, + current_phase_values=current_phase_values, + adjustments=adjustments, + property_instance=property_instance, + model_predicted_sap=0 + ) + ) + + # SAP is already at 0 → no adjustment expected + assert updated_adjustments == [] + assert updated_current["sap"] == 68.0 + assert updated_impact["sap"] == 0 + + +def test_mv_valid_negative_no_adjustment(property_instance): + rec = {"type": "mechanical_ventilation", "recommendation_id": "mv", "phase": 1} + + previous = {"sap": 70.0} + current = {"sap": 67.0} + impact = {"sap": -3.0, "carbon": 0, "heat_demand": 0} + adjustments = [] + + updated_impact, updated_current, updated_adjustments = ( + Recommendations._apply_measure_specific_rules( + rec, impact, previous, current, adjustments, property_instance, 0 + ) + ) + + assert updated_adjustments == [] + assert updated_current["sap"] == 67.0 + assert updated_impact["sap"] == -3.0 + + +def test_mv_zero_impact_allowed(property_instance): + rec = {"type": "mechanical_ventilation", "recommendation_id": "mv", "phase": 1} + + previous = {"sap": 68.0, "sap_prediction": 71.7} + current = {"sap": 68.0} + impact = {"sap": 0.0, "carbon": 0, "heat_demand": 0} + adjustments = [] + + updated_impact, updated_current, updated_adjustments = ( + Recommendations._apply_measure_specific_rules( + rec, impact, previous, current, adjustments, property_instance, 0 + ) + ) + + assert updated_adjustments == [] + assert updated_current["sap"] == 68.0 + assert updated_impact["sap"] == 0.0 + + +def test_mv_positive_impact_corrected(property_instance): + rec = {"type": "mechanical_ventilation", "recommendation_id": "mv", "phase": 1} + + previous = {"sap": 60.0} + current = {"sap": 61.0} + impact = {"sap": 1.0, "carbon": 0, "heat_demand": 0} + adjustments = [] + + updated_impact, updated_current, updated_adjustments = ( + Recommendations._apply_measure_specific_rules( + rec, impact, previous, current, adjustments, property_instance, 0 + ) + ) + + assert len(updated_adjustments) == 1 + assert updated_current["sap"] == previous["sap"] + updated_impact["sap"] + assert updated_impact["sap"] <= 0 + + +def test_mv_below_lower_bound_corrected(property_instance): + rec = {"type": "mechanical_ventilation", "recommendation_id": "mv", "phase": 1} + + previous = {"sap": 70.0} + current = {"sap": 64.0} + impact = {"sap": -6.0, "carbon": 0, "heat_demand": 0} + adjustments = [] + + updated_impact, updated_current, updated_adjustments = ( + Recommendations._apply_measure_specific_rules( + rec, impact, previous, current, adjustments, property_instance, 0 + ) + ) + + assert len(updated_adjustments) == 1 + assert updated_impact["sap"] >= -4 + + +def test_mv_floor_triggered(property_instance): + rec = {"type": "mechanical_ventilation", "recommendation_id": "mv", "phase": 1} + + previous = {"sap": 2.0} + current = {"sap": 0.5} + impact = {"sap": -1.5, "carbon": 0, "heat_demand": 0} + adjustments = [] + + updated_impact, updated_current, updated_adjustments = ( + Recommendations._apply_measure_specific_rules( + rec, impact, previous, current, adjustments, property_instance, 0 + ) + ) + + assert updated_current["sap"] == 1.0 + assert updated_adjustments[0]["sap_adjustment"] > 0 + + +def test_mv_exactly_one_no_floor(property_instance): + rec = {"type": "mechanical_ventilation", "recommendation_id": "mv", "phase": 1} + + previous = {"sap": 2.0} + current = {"sap": 1.0} + impact = {"sap": -1.0, "carbon": 0, "heat_demand": 0} + adjustments = [] + + updated_impact, updated_current, updated_adjustments = ( + Recommendations._apply_measure_specific_rules( + rec, impact, previous, current, adjustments, property_instance, 0 + ) + ) + + assert updated_adjustments == [] + assert updated_current["sap"] == 1.0 + + +def test_lighting_no_cap(property_instance): + rec = {"type": "low_energy_lighting", "recommendation_id": "led", "phase": 1, + "co2_equivalent_savings": 0} + + previous = {"sap": 60.0, "carbon": 2.0} + current = {"sap": 61.0, "carbon": 2.0} + impact = {"sap": 1.0, "carbon": 0, "heat_demand": 0} + adjustments = [] + + updated_impact, updated_current, updated_adjustments = ( + Recommendations._apply_measure_specific_rules( + rec, impact, previous, current, adjustments, property_instance, 0 + ) + ) + + assert updated_adjustments == [] + + +def test_filter_phase_adjustments(): + example_adjustments = [ + {'recommendation_id': '0_phase=0', 'phase': 0, 'sap_adjustment': np.float64(1.7)}, + {'recommendation_id': '4_phase=2', 'phase': 2, 'sap_adjustment': np.float64(4.0)}, + {'recommendation_id': '5_phase=3', 'phase': 3, 'sap_adjustment': np.float64(1.0)}, + {'recommendation_id': '6_phase=3', 'phase': 3, 'sap_adjustment': np.float64(1.0000000000000027)} + ] + + res = Recommendations._filter_phase_adjustment(example_adjustments) + + assert res == [ + {'recommendation_id': '0_phase=0', 'phase': 0, 'sap_adjustment': np.float64(1.7)}, + {'recommendation_id': '4_phase=2', 'phase': 2, 'sap_adjustment': np.float64(4.0)}, + {'recommendation_id': '6_phase=3', 'phase': 3, 'sap_adjustment': np.float64(1.0000000000000027)} + ]