added prune diminishing recommendations

model_data/recommendations/WallRecommendations.py

@@ -1,6 +1,6 @@
-import pint
 import re
 import itertools
+import math
 
 from model_data.Property import Property
 from model_data.ConservationAreaClient import ConservationAreaClient
@@ -217,6 +217,8 @@ class WallRecommendations(BaseUtility):
         self.property = property_instance
         self.year_built = self._year_property_was_built()
         self.uvalue_estimates = uvalue_estimates
+        # For audit purposes, when estimating u values we'll store it
+        self.estimated_u_value = None
 
         # Will contains a list of recommended measures
         self.recommendations = []
@@ -299,6 +301,7 @@ class WallRecommendations(BaseUtility):
                 u_value = self.DEFAULT_U_VALUES["solid_brick"]
             else:
                 u_value = self._get_walls_uvalue_estimate()
+            self.estimated_u_value = u_value
 
             if u_value >= self.BUILDING_REGULATIONS_PART_L_MAX_U_VALUE:
                 self.find_insulation(u_value)
@@ -306,6 +309,38 @@ class WallRecommendations(BaseUtility):
 
         raise NotImplementedError("Not implemented yet")
 
+    def _is_diminishing_returns(self, new_u_value, lowest_selected_u_value):
+        """
+        What are defines diminishing returns?
+        1) The new u value is lower than the lowest selected u value
+        2) The new u value is below the diminishing returns threshold
+        3) We already have some recommendations so there is no need to
+           insert another recommendation in
+        """
+
+        # if we don't have anything selected, lowest_selected_u_value will be missing
+        if lowest_selected_u_value is None:
+            if self.recommendations:
+                raise ValueError("Recommendations should be empty - investigate")
+            # This means that nothing has been selected yet
+            # the new u value is less than the threshold, however this MIGHT be the only
+            # solution and so we consider it
+            return False
+
+        # We should already have recommendations
+        if not self.recommendations:
+            raise ValueError("Recommendations should not be empty - investigate")
+
+        # We already have a solution that is suitable so we want to make sure that
+        # any new solutin actually has a higher u-value as it will either be
+        # 1) cheaper
+        # 2) thinner with a more efficient material
+        is_diminishing = (new_u_value < self.DIMINISHING_RETURNS_U_VALUE) and (
+            new_u_value < lowest_selected_u_value
+        )
+
+        return is_diminishing
+
     def find_insulation(self, u_value):
         """
         This function contains the logic for finding potential insulation measures for a property, depending
@@ -320,21 +355,34 @@ class WallRecommendations(BaseUtility):
         iwi_parts = [part for part in wall_parts if part["type"] == "internal_wall_insulation"]
 
         # Recommend external and internal wall insulation separately
+
+        lowest_selected_u_value = None
         for part in ewi_parts + iwi_parts:
 
             for depth in part["depths"]:
                 part_u_value = self.r_value_per_mm_to_u_value(depth, part["r_value_per_mm"])
 
                 _, new_u_value = self.calculate_u_value_uplift(u_value, part_u_value)
-                new_u_value = round(new_u_value, 2)
-                if new_u_value < self.DIMINISHING_RETURNS_U_VALUE and self.recommendations:
-                    # If we already have a solution that is suitable and does not cross the diminishing returns
-                    # threshold, but meets the part L requirements, we don't need to recommend anything
+                new_u_value = math.ceil(new_u_value * 100.0) / 100.0
+
+                # If I have a lowest U value and my new u value is higher than that but lower than the
+                # diminishing returns threshold, it can be considered
+
+                # If I have a lowest U value and my new u value is lower than the lowest value, it's
+                # further into the diminishing returns threshold and can shouldn't be
+
+                if self._is_diminishing_returns(new_u_value, lowest_selected_u_value):
                     continue
 
                 # We allow a small tolerance for error so we don't discount the recommendation entirely
-                # if it's close, since this is an estimated new u-value
-                if new_u_value - self.U_VALUE_ERROR <= self.BUILDING_REGULATIONS_PART_L_MAX_U_VALUE:
+                if new_u_value <= self.BUILDING_REGULATIONS_PART_L_MAX_U_VALUE:
+
+                    if lowest_selected_u_value is None:
+                        lowest_selected_u_value = new_u_value
+
+                    if new_u_value <= lowest_selected_u_value:
+                        lowest_selected_u_value = new_u_value
+
                     self.recommendations.append(
                         self._get_recommended_part(part, depth, new_u_value)
                     )
@@ -368,6 +416,21 @@ class WallRecommendations(BaseUtility):
                         ]
                         self.recommendations.append(recommendation)
 
+        self.prune_diminishing_recommendations()
+
+    def prune_diminishing_recommendations(self):
+        # For any recommendations, if we have at least 1 reommendation that does not exhibit diminishing returns
+        # we trim all others that are beyond the diminishing returns threshold
+
+        # We first check if we have any recommendations that are not diminishing returns
+        not_diminishing_return = [
+            rec for rec in self.recommendations if rec["new_u_value"] >= self.DIMINISHING_RETURNS_U_VALUE
+        ]
+        if not not_diminishing_return:
+            self.recommendations = [
+                rec for rec in self.recommendations if rec["new_u_value"] >= self.DIMINISHING_RETURNS_U_VALUE
+            ]
+
     def _get_walls_uvalue_estimate(self):
 
         """

model_data/tests/test_wall_recommendations.py

@@ -1,6 +1,7 @@
 import os
 import pytest
 import pickle
+from unittest.mock import Mock
 from model_data.recommendations.WallRecommendations import WallRecommendations
 
 
@@ -20,6 +21,18 @@ class TestWallRecommendations:
         ) as f:
             return pickle.load(f)
 
+    @pytest.fixture
+    def mock_wall_rec_instance(self):
+        # Creating a mock instance of WallRecommendations with the necessary attributes
+        property_mock = Mock()
+        property_mock.full_sap_epc = {"lodgement-date": "2000-01-01"}  # or any date you want
+        property_mock.data = {"construction-age-band": "1950"}  # or any other data that fits your tests
+
+        uvalue_estimates_mock = Mock()
+
+        mock_wall_rec_instance = WallRecommendations(property_mock, uvalue_estimates_mock)
+        return mock_wall_rec_instance
+
     def test_init(self, input_properties, uvalue_estimates):
         obj = WallRecommendations(property_instance=input_properties[0], uvalue_estimates=uvalue_estimates)
         assert obj
@@ -61,7 +74,7 @@ class TestWallRecommendations:
         assert recommender.property.data["property-type"] == "Flat"
 
         recommender.recommend()
-        # This should result in some recommendations, all of which should be insulation
+        # This should result in some recommendations, all of which should be internal insulation
         assert recommender.recommendations
 
         rec_types = {rec["type"] for rec in recommender.recommendations}
@@ -73,6 +86,97 @@ class TestWallRecommendations:
             recommender.recommendations
         )
 
-    def test_nothing(self, input_properties, uvalue_estimates):
-        assert input_properties
-        assert uvalue_estimates
+    def test_solid_brick_insulation(self, input_properties, uvalue_estimates):
+        """
+        This tests a property with a wall description of Solid brick, as built, insulation (assumed)
+        The property was built in 1991, after cavity walls were introduced
+        However, we're told this property is solid brik so we assume no cavity.
+        We're also told that it has some insulation already
+
+        We'll need to estimate the u-value for this property since we don't know what the insulation is
+        and how thick it is. We'll estimate the u-value based on similar properties, by matching properties
+        with similar attributes such as the walls energy efficiency rating, the property type, the number of rooms,
+        etc
+
+        This property is not in a conservation area, however it's a flat so we don't recommend external wall insulation
+        """
+        recommender = WallRecommendations(property_instance=input_properties[6], uvalue_estimates=uvalue_estimates)
+
+        assert recommender.property.walls["original_description"] == "Solid brick, as built, insulated (assumed)"
+        assert recommender.year_built == 1991
+        assert not recommender.ewi_valid
+        assert recommender.property.in_conservation_area == "not_in_conservation_area"
+        assert recommender.property.data["property-type"] == "Flat"
+        assert recommender.estimated_u_value is None
+
+        recommender.recommend()
+
+        # We should result in some recommendations, all of which should be internal wall insulation
+        assert recommender.recommendations
+        assert recommender.estimated_u_value == 0.45
+
+        rec_types = {rec["type"] for rec in recommender.recommendations}
+        assert rec_types == {"internal_wall_insulation"}
+
+        # Check the recommendations provide a u value below the minimum
+        assert all(
+            rec["new_u_value"] < WallRecommendations.BUILDING_REGULATIONS_PART_L_MAX_U_VALUE for rec in
+            recommender.recommendations
+        )
+
+        for rec in recommender.recommendations:
+            assert rec["new_u_value"] < 0.3
+
+    def test_is_diminishing_returns_no_recommendations(self, mock_wall_rec_instance):
+        # We have no recommendations but the new u value is below the diminishing returns threshold
+        # however since there are no recommendaions yet, we should include it and say
+        # it is NOT diminishing
+        mock_wall_rec_instance.recommendations = []
+
+        new_u_value = 0.24
+        assert new_u_value < WallRecommendations.DIMINISHING_RETURNS_U_VALUE
+
+        assert not mock_wall_rec_instance._is_diminishing_returns(new_u_value, None)
+
+    def test_is_diminishing_returns_diminishing(self, mock_wall_rec_instance):
+        # We have a recommendation already and the new u value falls below the diminishing returns
+        # threshold and is also lower than the lowest selected u value, so we should say this IS
+        # diminishing returns
+        mock_wall_rec_instance.recommendations = [Mock()]
+
+        new_u_value = 0.2
+        lowest_selected_u_value = 0.23
+        assert new_u_value < WallRecommendations.DIMINISHING_RETURNS_U_VALUE
+
+        assert mock_wall_rec_instance._is_diminishing_returns(new_u_value, lowest_selected_u_value)
+
+    def test_is_diminishing_returns_is_diminishing(self, mock_wall_rec_instance):
+        # We have a recommendation already and the new u value falls below the diminishing returns
+        # threshold and it's lower than the lowest selected u value, so we should say this IS DIMINISHIN
+        mock_wall_rec_instance.recommendations = [Mock()]
+
+        new_u_value = 0.24
+        lowest_selected_u_value = 0.26
+
+        assert new_u_value < WallRecommendations.DIMINISHING_RETURNS_U_VALUE
+
+        assert mock_wall_rec_instance._is_diminishing_returns(new_u_value, lowest_selected_u_value)
+
+    def test_is_diminishing_returns_not_diminishing(self, mock_wall_rec_instance):
+        # We have a recommendation already and the new u value falls below the diminishing returns
+        # threshold however it's higher than the lowest selected u value, so we should say this is NOT
+        # diminishing returns
+        mock_wall_rec_instance.recommendations = [Mock()]
+
+        new_u_value = 0.24
+        lowest_selected_u_value = 0.22
+
+        assert new_u_value < WallRecommendations.DIMINISHING_RETURNS_U_VALUE
+
+        assert not mock_wall_rec_instance._is_diminishing_returns(new_u_value, lowest_selected_u_value)
+
+    def test_is_diminishing_returns_error_in_recommendations(self, mock_wall_rec_instance):
+        # Testing case where there's an error in recommendations
+        mock_wall_rec_instance.recommendations = []
+        with pytest.raises(ValueError):
+            mock_wall_rec_instance._is_diminishing_returns(0.2, 0.24)