From 0cf443e9a683e674a32d37d12ba2e8bb37107b85 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Thu, 5 Aug 2021 18:38:50 +0100 Subject: [PATCH] Use already-transformed polygons wherever possible Transforming full resolution polygons to linear coordinates is somewhat expensive. We can make things run faster by: - looking at `simple_polygons` which have fewer points and are therefore faster to transform. - reusing polygons that have already been transformed where possible --- app/broadcast_areas/models.py | 12 +++++++----- app/models/broadcast_message.py | 2 +- tests/app/broadcast_areas/test_broadcast_area.py | 16 ++++++++-------- 3 files changed, 16 insertions(+), 14 deletions(-) diff --git a/app/broadcast_areas/models.py b/app/broadcast_areas/models.py index b1a444703..b7f434bae 100644 --- a/app/broadcast_areas/models.py +++ b/app/broadcast_areas/models.py @@ -61,9 +61,9 @@ class BaseBroadcastArea(ABC): @cached_property def phone_density(self): - if not self.polygons.estimated_area: + if not self.simple_polygons.estimated_area: return 0 - return self.count_of_phones / square_metres_to_square_miles(self.polygons.estimated_area) + return self.count_of_phones / square_metres_to_square_miles(self.simple_polygons.estimated_area) @property def estimated_bleed_in_m(self): @@ -160,9 +160,11 @@ class CustomBroadcastArea(BaseBroadcastArea): @classmethod def from_polygon_objects(cls, polygon_objects): - return cls(name=None, polygons=polygon_objects.as_coordinate_pairs_lat_long) + instance = cls(name=None) + instance.polygons = polygon_objects + return instance - @property + @cached_property def polygons(self): return Polygons( # Polygons in the DB are stored with the coordinate pair @@ -189,7 +191,7 @@ class CustomBroadcastArea(BaseBroadcastArea): return broadcast_area_libraries.get_areas_with_simple_polygons([ # We only index electoral wards in the RTree overlap.data for overlap in rtree_index.query( - Rect(*self.polygons.bounds) + Rect(*self.simple_polygons.bounds) ) ]) diff --git a/app/models/broadcast_message.py b/app/models/broadcast_message.py index 46354073d..072c38f5b 100644 --- a/app/models/broadcast_message.py +++ b/app/models/broadcast_message.py @@ -88,7 +88,7 @@ class BroadcastMessage(JSONModel): broadcast_message_id=broadcast_message_id, )) - @property + @cached_property def areas(self): if 'ids' in self._dict['areas']: library_areas = self.get_areas(self.area_ids) diff --git a/tests/app/broadcast_areas/test_broadcast_area.py b/tests/app/broadcast_areas/test_broadcast_area.py index beb702212..a5d5090b5 100644 --- a/tests/app/broadcast_areas/test_broadcast_area.py +++ b/tests/app/broadcast_areas/test_broadcast_area.py @@ -276,24 +276,24 @@ def test_estimate_number_of_smartphones_for_population( @pytest.mark.parametrize('area, expected_phones_per_square_mile', ( ( # Islington (most dense in UK) - 'lad20-E09000019', 34_281 + 'lad20-E09000019', 32_662 ), ( # Cordwainer Ward (City of London) # This is higher than Islington because we inflate the # popualtion to account for daytime workers - 'wd20-E05009300', 496_480 + 'wd20-E05009300', 392_870 ), ( # Crewe East - 'wd20-E05008621', 3_460), + 'wd20-E05008621', 3_289), ( # Eden (Cumbria, least dense in England) - 'lad20-E07000030', 44.12 + 'lad20-E07000030', 43.41 ), ( # Highland (least dense in UK) - 'lad20-S12000017', 8.18 + 'lad20-S12000017', 6.97 ), )) def test_phone_density( @@ -317,15 +317,15 @@ def test_phone_density( ), ( # Crewe East - 'wd20-E05008621', 1_476 + 'wd20-E05008621', 1_504 ), ( # Eden (Cumbria, least dense in England) - 'lad20-E07000030', 3_844 + 'lad20-E07000030', 3_852 ), ( # Highland (least dense in UK) - 'lad20-S12000017', 4_759 + 'lad20-S12000017', 4_846 ), ( # No population data available