From 11f87c250a3c5170417221194aca5241cbdc50f2 Mon Sep 17 00:00:00 2001 From: Dobson Date: Tue, 23 Apr 2024 10:56:17 +0100 Subject: [PATCH 01/10] Address issue - add graphfcn to remove non allowable links - update parameters to customise this behaviour - update prepare_data to use osmnx bbox and remove depreciation error - test new behaviour --- swmmanywhere/graph_utilities.py | 37 +++++++++++++++++++++++++++++++++ swmmanywhere/parameters.py | 9 ++++++++ swmmanywhere/prepare_data.py | 3 ++- tests/test_graph_utilities.py | 25 +++++++++++++++++++++- 4 files changed, 72 insertions(+), 2 deletions(-) diff --git a/swmmanywhere/graph_utilities.py b/swmmanywhere/graph_utilities.py index db89f43f..e85e63a9 100644 --- a/swmmanywhere/graph_utilities.py +++ b/swmmanywhere/graph_utilities.py @@ -220,6 +220,43 @@ def __call__(self, for u, v, key in edges_to_remove: G.remove_edge(u, v, key) return G + +@register_graphfcn +class remove_non_pipe_allowable_links(BaseGraphFunction): + """remove_non_pipe_allowable_links class.""" + def __call__(self, + G: nx.Graph, + topology_derivation: parameters.TopologyDerivation, + **kwargs) -> nx.Graph: + """Remove non-pipe allowable links. + + This function removes links that are not allowable for pipes. The non- + allowable links are specified in the 'omit_edges' attribute of the + topology_derivation parameter. If the omit property is present in the + 'highway' attribute of the edge (e.g., 'motorway' is a category under + 'highway'), the edge is removed. If the omit property is not a 'highway' + attribute, the edge is removed if the omit property is not null in the + edge data (e.g., any type of 'bridge'). + + Args: + G (nx.Graph): A graph + topology_derivation (parameters.TopologyDerivation): A TopologyDerivation + parameter object + **kwargs: Additional keyword arguments are ignored. + + Returns: + G (nx.Graph): A graph + """ + edges_to_remove = [] + for u, v, data in G.edges(data=True): + for omit in topology_derivation.omit_edges: + if data.get('highway', None) == omit: + edges_to_remove.append((u, v)) + elif data.get(omit, None): + edges_to_remove.append((u, v)) + for u, v in edges_to_remove: + G.remove_edge(u, v) + return G @register_graphfcn class format_osmnx_lanes(BaseGraphFunction, diff --git a/swmmanywhere/parameters.py b/swmmanywhere/parameters.py index 32724c13..f5fd4891 100644 --- a/swmmanywhere/parameters.py +++ b/swmmanywhere/parameters.py @@ -81,6 +81,15 @@ class TopologyDerivation(BaseModel): unit = "-", description = "Weights for topo derivation") + omit_edges: list = Field(default = ['motorway', + 'motorway_link', + 'corridor', + 'bridge', + 'tunnel'], + min_items = 1, + unit = "-", + description = "OSM paths pipes are not allowed under") + chahinian_slope_scaling: float = Field(default = 1, le = 1, ge = 0, diff --git a/swmmanywhere/prepare_data.py b/swmmanywhere/prepare_data.py index 2c04a1ab..e87b4703 100644 --- a/swmmanywhere/prepare_data.py +++ b/swmmanywhere/prepare_data.py @@ -103,8 +103,9 @@ def download_street(bbox: tuple[float, float, float, float]) -> nx.MultiDiGraph: ``truncate_by_edge set`` to True. """ west, south, east, north = bbox + bbox = (north, south, east, west) # not sure why osmnx uses this order graph = ox.graph_from_bbox( - north, south, east, west, network_type="drive", truncate_by_edge=True + bbox = bbox, network_type="drive", truncate_by_edge=True ) return cast("nx.MultiDiGraph", graph) diff --git a/tests/test_graph_utilities.py b/tests/test_graph_utilities.py index 23528f64..366d1ba9 100644 --- a/tests/test_graph_utilities.py +++ b/tests/test_graph_utilities.py @@ -243,7 +243,30 @@ def test_pipe_by_pipe(): for u, d in G.nodes(data=True): assert 'chamber_floor_elevation' in d.keys() assert math.isfinite(d['chamber_floor_elevation']) - + +def get_edge_types(G): + """Get the edge types in the graph.""" + edge_types = set() + for u,v,d in G.edges(data=True): + if isinstance(d['highway'], list): + edge_types.union(d['highway']) + else: + edge_types.add(d['highway']) + return edge_types + +def test_remove_non_pipe_allowable_links(): + """Test the remove_non_pipe_allowable_links function.""" + G = load_graph(Path(__file__).parent / 'test_data' / 'street_graph.json') + # Ensure some invalid paths + topology_params = parameters.TopologyDerivation(omit_edges = ['primary', 'bridge']) + assert len(set([d.get('bridge',None) for u,v,d in G.edges(data=True)])) > 1 + assert 'primary' in get_edge_types(G) + + G_ = gu.remove_non_pipe_allowable_links(G, topology_params) + assert 'primary' not in get_edge_types(G_) + assert len(set([d.get('bridge',None) for u,v,d in G_.edges(data=True)])) == 1 + + def test_iterate_graphfcns(): """Test the iterate_graphfcns function.""" G = load_graph(Path(__file__).parent / 'test_data' / 'graph_topo_derived.json') From 4f2038c6a4e41a48acd14fa4cdc97919de24ea6d Mon Sep 17 00:00:00 2001 From: Dobson Date: Tue, 23 Apr 2024 11:03:09 +0100 Subject: [PATCH 02/10] Update parameters.py no need for corridor unless doing pedetrians --- swmmanywhere/parameters.py | 1 - 1 file changed, 1 deletion(-) diff --git a/swmmanywhere/parameters.py b/swmmanywhere/parameters.py index f5fd4891..f7d86a18 100644 --- a/swmmanywhere/parameters.py +++ b/swmmanywhere/parameters.py @@ -83,7 +83,6 @@ class TopologyDerivation(BaseModel): omit_edges: list = Field(default = ['motorway', 'motorway_link', - 'corridor', 'bridge', 'tunnel'], min_items = 1, From a421aa01fcc47b20fdc5400692d1d3a3f8f76c66 Mon Sep 17 00:00:00 2001 From: Dobson Date: Tue, 23 Apr 2024 11:05:26 +0100 Subject: [PATCH 03/10] Update graph_utilities.py fix keys and duplicates --- swmmanywhere/graph_utilities.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/swmmanywhere/graph_utilities.py b/swmmanywhere/graph_utilities.py index e85e63a9..58e610e4 100644 --- a/swmmanywhere/graph_utilities.py +++ b/swmmanywhere/graph_utilities.py @@ -247,15 +247,15 @@ def __call__(self, Returns: G (nx.Graph): A graph """ - edges_to_remove = [] - for u, v, data in G.edges(data=True): + edges_to_remove = set() + for u, v, keys, data in G.edges(data=True,keys = True): for omit in topology_derivation.omit_edges: if data.get('highway', None) == omit: - edges_to_remove.append((u, v)) + edges_to_remove.add((u, v, keys)) elif data.get(omit, None): - edges_to_remove.append((u, v)) - for u, v in edges_to_remove: - G.remove_edge(u, v) + edges_to_remove.add((u, v, keys)) + for u, v, keys in edges_to_remove: + G.remove_edge(u, v, keys) return G @register_graphfcn From 309bf0d40622fa6125ce61ce6e30874f5a868180 Mon Sep 17 00:00:00 2001 From: Dobson Date: Tue, 23 Apr 2024 11:10:09 +0100 Subject: [PATCH 04/10] Update requirements.txt --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 5f8f7623..144d08cf 100644 --- a/requirements.txt +++ b/requirements.txt @@ -148,7 +148,7 @@ numpy==1.26.4 # swmmanywhere (pyproject.toml) # tifffile # xarray -osmnx==1.8.1 +osmnx==1.9.1 # via swmmanywhere (pyproject.toml) packaging==23.2 # via From 9ca3c106158c0cd6f098de83fdfc0aade8e2cda2 Mon Sep 17 00:00:00 2001 From: Dobson Date: Tue, 23 Apr 2024 11:13:17 +0100 Subject: [PATCH 05/10] Update demo_config.yml --- tests/test_data/demo_config.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/test_data/demo_config.yml b/tests/test_data/demo_config.yml index 5ac91fe8..ecbb15aa 100644 --- a/tests/test_data/demo_config.yml +++ b/tests/test_data/demo_config.yml @@ -15,6 +15,7 @@ starting_graph: null graphfcn_list: - assign_id - format_osmnx_lanes + - remove_non_pipe_allowable_links - double_directed - fix_geometries - split_long_edges From 72124237300635c434c0a5b3350efdc18fcaf06f Mon Sep 17 00:00:00 2001 From: Dobson Date: Tue, 23 Apr 2024 11:14:51 +0100 Subject: [PATCH 06/10] Update dev-requirements.txt --- dev-requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev-requirements.txt b/dev-requirements.txt index 0730b32f..4bad4a92 100644 --- a/dev-requirements.txt +++ b/dev-requirements.txt @@ -178,7 +178,7 @@ numpy==1.26.4 # swmmanywhere (pyproject.toml) # tifffile # xarray -osmnx==1.8.1 +osmnx==1.9.1 # via swmmanywhere (pyproject.toml) packaging==23.2 # via From ca84160b7e40303dec50e0f3717f9ee8b2879eb6 Mon Sep 17 00:00:00 2001 From: barneydobson Date: Thu, 25 Apr 2024 09:08:58 +0100 Subject: [PATCH 07/10] Update swmmanywhere/graph_utilities.py MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Diego Alonso Álvarez <6095790+dalonsoa@users.noreply.github.com> --- swmmanywhere/graph_utilities.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/swmmanywhere/graph_utilities.py b/swmmanywhere/graph_utilities.py index 58e610e4..523ed089 100644 --- a/swmmanywhere/graph_utilities.py +++ b/swmmanywhere/graph_utilities.py @@ -254,8 +254,8 @@ def __call__(self, edges_to_remove.add((u, v, keys)) elif data.get(omit, None): edges_to_remove.add((u, v, keys)) - for u, v, keys in edges_to_remove: - G.remove_edge(u, v, keys) + for edges in edges_to_remove: + G.remove_edge(*edges) return G @register_graphfcn From 08013814c1ea08847789e809f3ea5a87e8d872ab Mon Sep 17 00:00:00 2001 From: Dobson Date: Thu, 25 Apr 2024 09:19:14 +0100 Subject: [PATCH 08/10] Update graph_utilities.py improve docstring --- swmmanywhere/graph_utilities.py | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/swmmanywhere/graph_utilities.py b/swmmanywhere/graph_utilities.py index 58e610e4..de8c9699 100644 --- a/swmmanywhere/graph_utilities.py +++ b/swmmanywhere/graph_utilities.py @@ -231,13 +231,18 @@ def __call__(self, """Remove non-pipe allowable links. This function removes links that are not allowable for pipes. The non- - allowable links are specified in the 'omit_edges' attribute of the - topology_derivation parameter. If the omit property is present in the - 'highway' attribute of the edge (e.g., 'motorway' is a category under - 'highway'), the edge is removed. If the omit property is not a 'highway' - attribute, the edge is removed if the omit property is not null in the - edge data (e.g., any type of 'bridge'). - + allowable links are specified in the `omit_edges` attribute of the + topology_derivation parameter. There two cases handled: + 1. The `highway` property of the edge. In osmnx, `highway` is a category + that contains the road type, e.g., motorway, trunk, residential. If + `omit_edges` contains a value that is a category under `highway`, the + edge is removed. + 2. Any other properties of the edge that are in `omit_edges`. If the + property is not null in the edge data, the edge is removed. e.g., + if `bridge` is in `omit_edges` and the `bridge` entry of the edge + is NULL, then the edge is retained, if it is something like 'yes', + or 'viaduct' then the edge is removed. + Args: G (nx.Graph): A graph topology_derivation (parameters.TopologyDerivation): A TopologyDerivation @@ -251,8 +256,10 @@ def __call__(self, for u, v, keys, data in G.edges(data=True,keys = True): for omit in topology_derivation.omit_edges: if data.get('highway', None) == omit: + # Check whether the 'highway' property is 'omit' edges_to_remove.add((u, v, keys)) elif data.get(omit, None): + # Check whether the 'omit' property of edge is not None edges_to_remove.add((u, v, keys)) for u, v, keys in edges_to_remove: G.remove_edge(u, v, keys) From 9cf9f9195650e32c00afe65ea969d3f25bf1d5b0 Mon Sep 17 00:00:00 2001 From: Dobson Date: Thu, 25 Apr 2024 09:24:56 +0100 Subject: [PATCH 09/10] Imrpvoe commenting --- swmmanywhere/graph_utilities.py | 6 +++--- tests/test_graph_utilities.py | 4 ++++ 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/swmmanywhere/graph_utilities.py b/swmmanywhere/graph_utilities.py index 02735225..b9068026 100644 --- a/swmmanywhere/graph_utilities.py +++ b/swmmanywhere/graph_utilities.py @@ -234,9 +234,9 @@ def __call__(self, allowable links are specified in the `omit_edges` attribute of the topology_derivation parameter. There two cases handled: 1. The `highway` property of the edge. In osmnx, `highway` is a category - that contains the road type, e.g., motorway, trunk, residential. If - `omit_edges` contains a value that is a category under `highway`, the - edge is removed. + that contains the road type, e.g., motorway, trunk, primary. If the + edge contains a value in the `highway` property that is in `omit_edges`, + the edge is removed. 2. Any other properties of the edge that are in `omit_edges`. If the property is not null in the edge data, the edge is removed. e.g., if `bridge` is in `omit_edges` and the `bridge` entry of the edge diff --git a/tests/test_graph_utilities.py b/tests/test_graph_utilities.py index 366d1ba9..fe597056 100644 --- a/tests/test_graph_utilities.py +++ b/tests/test_graph_utilities.py @@ -259,7 +259,11 @@ def test_remove_non_pipe_allowable_links(): G = load_graph(Path(__file__).parent / 'test_data' / 'street_graph.json') # Ensure some invalid paths topology_params = parameters.TopologyDerivation(omit_edges = ['primary', 'bridge']) + + # Test that an edge has a non-None 'bridge' entry assert len(set([d.get('bridge',None) for u,v,d in G.edges(data=True)])) > 1 + + # Test that an edge has a 'primary' entry under highway assert 'primary' in get_edge_types(G) G_ = gu.remove_non_pipe_allowable_links(G, topology_params) From 35d274dfc7f68284708353c17435de890ba96a71 Mon Sep 17 00:00:00 2001 From: Dobson Date: Thu, 25 Apr 2024 09:27:00 +0100 Subject: [PATCH 10/10] Update graph_utilities.py --- swmmanywhere/graph_utilities.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/swmmanywhere/graph_utilities.py b/swmmanywhere/graph_utilities.py index b9068026..133123ac 100644 --- a/swmmanywhere/graph_utilities.py +++ b/swmmanywhere/graph_utilities.py @@ -233,7 +233,7 @@ def __call__(self, This function removes links that are not allowable for pipes. The non- allowable links are specified in the `omit_edges` attribute of the topology_derivation parameter. There two cases handled: - 1. The `highway` property of the edge. In osmnx, `highway` is a category + 1. The `highway` property of the edge. In `osmnx`, `highway` is a category that contains the road type, e.g., motorway, trunk, primary. If the edge contains a value in the `highway` property that is in `omit_edges`, the edge is removed.