From ac749e427e2f8724077489cf6bff4a6adc6f2c32 Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Mon, 26 Jan 2026 19:06:50 +0000 Subject: [PATCH] first implementation for sloping ceiling insulation --- .idea/copilot.data.migration.ask2agent.xml | 6 ++ recommendations/Costs.py | 16 ++-- recommendations/RoofRecommendations.py | 59 ++++++++------- .../tests/test_roof_recommendations.py | 74 +++++++++---------- 4 files changed, 84 insertions(+), 71 deletions(-) create mode 100644 .idea/copilot.data.migration.ask2agent.xml diff --git a/.idea/copilot.data.migration.ask2agent.xml b/.idea/copilot.data.migration.ask2agent.xml new file mode 100644 index 00000000..1f2ea11e --- /dev/null +++ b/.idea/copilot.data.migration.ask2agent.xml @@ -0,0 +1,6 @@ + + + + + \ No newline at end of file diff --git a/recommendations/Costs.py b/recommendations/Costs.py index fd429afa..5f312f63 100644 --- a/recommendations/Costs.py +++ b/recommendations/Costs.py @@ -947,6 +947,10 @@ class Costs: Heuristic model based on retrofit guidance (Checkatrade, The Green Age) and analogy with internal wall insulation. + See _estimate_number_of_days_for_solid_floor for detailed explanation regarding assumptions + and methodology, however for the purpose of placeholder, this function mimics the approach + to that method but is detached to allow for future changes + Assumptions: - ~30 m² of sloping ceiling takes ~4 working days - Small jobs still require multiple days (setup, stripping, reboarding) @@ -968,7 +972,7 @@ class Costs: return labour_days @classmethod - def sloping_ceiling_insulation(cls, insulation_roof_area: float) -> Mapping[str, Any]: + def sloping_ceiling_insulation(cls, insulation_roof_area: float) -> Mapping[str, float]: """ This costing for this is based on Checkatrade desktop research, since we are yet to receive installer quotes. :param insulation_roof_area: Area of the sloping ceiling to be insulated @@ -994,10 +998,10 @@ class Costs: vat = total - (total / 1.2) return { - "total": total, - "contingency": total * contingency_rate, + "total": float(total), + "contingency": float(total * contingency_rate), "contingency_rate": contingency_rate, - "vat": vat, - "labour_hours": labour_hours, - "labour_days": labour_days, + "vat": float(vat), + "labour_hours": float(labour_hours), + "labour_days": float(labour_days), } diff --git a/recommendations/RoofRecommendations.py b/recommendations/RoofRecommendations.py index baaaa547..06ff306b 100644 --- a/recommendations/RoofRecommendations.py +++ b/recommendations/RoofRecommendations.py @@ -2,7 +2,7 @@ import math import pandas as pd from backend.Property import Property from backend.app.plan.schemas import MEASURE_MAP -from typing import List +from typing import List, Mapping, Any from datatypes.enums import QuantityUnits from recommendations.recommendation_utils import ( get_roof_u_value, r_value_per_mm_to_u_value, calculate_u_value_uplift, is_diminishing_returns, @@ -119,24 +119,6 @@ class RoofRecommendations: return (full_insulated_room_roof or room_roof_insulated_at_rafters) and not has_non_invasive_recommendation - def recommend_sloping_ceiling(self): - """ - Sloping ceiling insulation recommendations are different from other roof types, though - the description of the roof appears to be quite similar to a roof with a loft. In order to - deduce the roof type, we apply the following logic: - - 1) If the roof is descrbed as pitched, insulated, without a loft insulation thickness, it's - an insulated sloped ceiling - 2) If the roof insulation is assumed, it implies that the surveyor could not gain access to the - roof and therefore it's a loft - 3) If it's a pitched roof that is uninsulated and is NOT assumed, and there is not loft insulation - recommendation, this implies that the surveyor was able to gain access to the roof and there was no - loft insulation recommendation so it must be a sloping ceiling since loft insulation is a default - recommendation for an uninsualted loft - :return: - """ - pass - @staticmethod def is_sloping_ceiling_appropriate( is_pitched: bool, is_loft: bool, is_assumed: bool, has_sloping_ceiling_recommendation: bool, @@ -207,7 +189,7 @@ class RoofRecommendations: @staticmethod def is_flat_roof_insulation_appropriate( - is_flat: bool, measures: List, has_flat_roof_recommendation: bool + is_flat: bool, measures: List, has_flat_roof_recommendation: bool, primary_roof_is_sloped: bool ) -> bool: """ Determine if flat roof insulation is appropriate @@ -215,12 +197,17 @@ class RoofRecommendations: :param measures: List - list of measures :param has_flat_roof_recommendation: Boolean - indicates whether or not there is a flat roof non-invasive recommendation + :param primary_roof_is_sloped: Boolean - indicates if the primary roof is flat :return: Boolean + + When checking if has_flat_roof_recommendation and primary_roof_is_sloped, we need to check both + conditions. This is because within a default EPC recommendation, the EPC will pair these recommendations + together. Therefore, weneed to ensure the primary roof isn't sloped """ flat_roof_in_measures = "flat_roof_insulation" in measures - return (is_flat and flat_roof_in_measures) or has_flat_roof_recommendation + return (is_flat and flat_roof_in_measures) or (has_flat_roof_recommendation and not primary_roof_is_sloped) @staticmethod def is_room_roof_insulation_appropriate( @@ -272,6 +259,8 @@ class RoofRecommendations: ): return False + return True + @staticmethod def _is_primary_roof_sloped( is_pitched: bool, is_loft: bool, is_assumed: bool @@ -365,15 +354,15 @@ class RoofRecommendations: has_sloping_ceiling_recommendation=has_sloping_ceiling_recommendation, primary_roof_is_sloped=primary_roof_is_sloped ) - needs_loft_insulation = self.is_loft_insulation_appropriate( measures=measures, is_pitched=is_pitched, is_at_rafters=is_at_rafters, rir_over_loft=rir_over_loft, has_loft_insulation_recommendation=has_loft_insulation_recommendation, is_assumed=is_assumed, has_sloping_ceiling_recommendation=has_sloping_ceiling_recommendation ) - needs_flat_roof_insulation = self.is_flat_roof_insulation_appropriate( - is_flat=is_flat, measures=measures, has_flat_roof_recommendation=has_flat_roof_recommendation + is_flat=is_flat, measures=measures, + has_flat_roof_recommendation=has_flat_roof_recommendation, + primary_roof_is_sloped=primary_roof_is_sloped ) needs_rir_insulation = self.is_room_roof_insulation_appropriate( is_room_roof=is_room_roof, @@ -784,15 +773,31 @@ class RoofRecommendations: self.recommendations = recommendations - def recommend_sloping_ceiling(self, phase: int, u_value, sloping_ceiling_recommendation: dict = None): + def recommend_sloping_ceiling(self, phase: int, u_value, non_invasive_recommendations: List[Mapping[str, Any]]): """ - Recommend insulation for a sloping ceiling + Sloping ceiling insulation recommendations are different from other roof types, though + the description of the roof appears to be quite similar to a roof with a loft. In order to + deduce the roof type, we apply the following logic: + + 1) If the roof is descrbed as pitched, insulated, without a loft insulation thickness, it's + an insulated sloped ceiling + 2) If the roof insulation is assumed, it implies that the surveyor could not gain access to the + roof and therefore it's a loft + 3) If it's a pitched roof that is uninsulated and is NOT assumed, and there is not loft insulation + recommendation, this implies that the surveyor was able to gain access to the roof and there was no + loft insulation recommendation so it must be a sloping ceiling since loft insulation is a default + recommendation for an uninsualted loft + Since we don't have any materials from installers for this specific recommendation, we do not iterate through any materials. Instead, we provide a single recommendation, we estimated prices based on desk research. :return: """ + sloping_ceiling_recommendation = next( + (x for x in non_invasive_recommendations if ["type"] == "sloping_ceiling_insulation"), {} + ) + new_description = "Pitched, insulated" new_efficiency = "Good" @@ -811,7 +816,7 @@ class RoofRecommendations: } cost_result = self.costs.sloping_ceiling_insulation( - roof_area=self.property.roof_area # For a pitched roof, this is the pitched roof area + insulation_roof_area=self.property.roof_area # For a pitched roof, this is the pitched roof area ) self.recommendations = [ diff --git a/recommendations/tests/test_roof_recommendations.py b/recommendations/tests/test_roof_recommendations.py index b8cea10b..ef02d050 100644 --- a/recommendations/tests/test_roof_recommendations.py +++ b/recommendations/tests/test_roof_recommendations.py @@ -1,8 +1,8 @@ +import pytest from backend.Property import Property +from etl.epc.Record import EPCRecord from recommendations.RoofRecommendations import RoofRecommendations from recommendations.tests.test_data.materials import materials -from etl.epc.Record import EPCRecord -import pytest class TestRoofRecommendations: @@ -405,40 +405,38 @@ class TestRoofRecommendations: assert not roof_recommender14.recommendations # ~~~~~~~~~~~~ Sloping Ceiling Insulation ~~~~~~~~~~~~ - @pytest.mark.parameterize("roof", - [ - ( - # For this example, the roof is pitched, without insulation and the description - # isn't assumed - {'original_description': 'Pitched, no insulation', 'thermal_transmittance': None, - 'thermal_transmittance_unit': None, - 'is_pitched': True, 'is_roof_room': False, 'is_loft': False, 'is_flat': False, - 'is_thatched': False, - 'is_at_rafters': False, 'is_assumed': False, 'has_dwelling_above': False, - 'is_valid': True, - 'insulation_thickness': 'none'} - ) - ] - ) - def test_sloping_ceiling_valid(self, roof): - # All conditions are met and therefore we should produce a sloping ceiling recommendation + @pytest.mark.parametrize( + "roof, has_sloping_ceiling_recommendation, primary_roof_is_sloped, expected_result", + [ + ( + { + 'original_description': 'Pitched, no insulation', + 'thermal_transmittance': None, + 'thermal_transmittance_unit': None, + 'is_pitched': True, + 'is_roof_room': False, + 'is_loft': False, + 'is_flat': False, + 'is_thatched': False, + 'is_at_rafters': False, + 'is_assumed': False, + 'has_dwelling_above': False, + 'is_valid': True, + 'insulation_thickness': 'none' + }, + True, + True, + True, + ) + ] + ) + def test_sloping_ceiling_valid( + self, roof, has_sloping_ceiling_recommendation, primary_roof_is_sloped, expected_result + ): assert RoofRecommendations.is_sloping_ceiling_appropriate( - is_pitched=True, is_loft=False, is_assumed=False, has_sloping_ceiling_recommendation=True - ) - - # One condition not met - we cannot verify - assert not RoofRecommendations.is_sloping_ceiling_appropriate( - is_pitched=True, is_loft=True, is_assumed=False, has_sloping_ceiling_recommendation=True - ) - assert not RoofRecommendations.is_sloping_ceiling_appropriate( - is_pitched=False, is_loft=False, is_assumed=False, has_sloping_ceiling_recommendation=True, - primary_roof_is_sloped=True - ) - assert not RoofRecommendations.is_sloping_ceiling_appropriate( - is_pitched=True, is_loft=False, is_assumed=True, has_sloping_ceiling_recommendation=True, - primary_roof_is_sloped=True - ) - assert not RoofRecommendations.is_sloping_ceiling_appropriate( - is_pitched=True, is_loft=False, is_assumed=True, has_sloping_ceiling_recommendation=True, - primary_roof_is_sloped=True - ) + is_pitched=roof["is_pitched"], + is_loft=roof["is_loft"], + is_assumed=roof["is_assumed"], + has_sloping_ceiling_recommendation=has_sloping_ceiling_recommendation, + primary_roof_is_sloped=primary_roof_is_sloped + ) == expected_result