worldbank / GOSTnets

Convenience wrapper for networkx analysis using geospatial information, focusing on OSM
https://worldbank.github.io/GOSTnets/
MIT License
22 stars 15 forks source link

Including ferries in GOSTNet networks #9

Closed rbanick closed 4 years ago

rbanick commented 4 years ago

The GOSTNetsload_osm routine filters solely on highways and thus excludes ferries from routing analysis. This degrades the accuracy of the modeled OD matrix when ferries play an important role in network connectivity.

As ferry routes are recorded as lines (route=ferry) in OSM they are theoretically compatible with GOSTNets, but I'm not sure how devilish the details are. A bonus bit of realism would be assigning wait time costs to ferry start/end nodes as in the asia_railways_v2 implementation.

A useful reference case is from Cox's Bazaar, Bangladesh, where communities across the bay to the north are connected to the town by ferry (see below screenshot). A roads-only network routes origin nodes in this community over a very long road path going first north, then back south. Some offshore island communities to the north are similarly connected to the mainland only by ferries.

image

andresfchamorro commented 4 years ago

Hi Robert,

Thanks for flagging this. I came across this issue and made some edits to _loadosm to address this. Part of the solution is in a branch of the older version of our repository: https://github.com/worldbank/GOST_PublicGoods/blob/andres-patch/GOSTNets/GOSTNets/LoadOSM.py

There were a few more changes to actually get it to work. I will update this on the current repo.

rbanick commented 4 years ago

Hi Andres,

Thanks for the quick reply, on a Sunday no less. Unfortunately inserting the linked code into my LoadOSM.py file didn't work. I updated the SQL select statement and inserted the elif text but without success. Note I also tried selecting on "route" instead of "ferry", since route is the key used in OSM.

Could you provide a bit more guidance on how to successfully integrate this code, or ideally push a change to the main GOSTNets repo so I can pull down easily there?

andresfchamorro commented 4 years ago

I just pushed the changes I had made to the main repo. You should be able to pull and include ferries when running _OSM_tonetwork by adding the following parameter:

G_load = losm.OSM_to_network(outOSM, includeFerries=True)

rbanick commented 4 years ago

I've pulled down the code and am trying to run this with the new library. I'm now getting an error, but perhaps that represents progress. Error message below:

---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-7-6550eeb1bb02> in <module>
      2 # This solution no longer worked after a restart...
      3 
----> 4 cxb = OSM_to_network(f, includeFerries=True)

~/git/GOSTnets/GOSTnets/load_osm.py in __init__(self, osmFile, includeFerries)
     42         '''
     43         self.osmFile = osmFile
---> 44         self.roads_raw = self.fetch_roads_and_ferries(osmFile) if includeFerries else self.fetch_roads(osmFile)
     45 
     46     def generateRoadsGDF(self, in_df = None, outFile='', verbose = False):

~/git/GOSTnets/GOSTnets/load_osm.py in fetch_roads_and_ferries(self, data_path)
    129                     highway=feature.GetField('highway')
    130                     roads.append([osm_id,highway,shapely_geo])
--> 131                 elif "ferry" in feature.GetField('other_tags'):
    132                     osm_id = feature.GetField('osm_id')
    133                     shapely_geo = loads(feature.geometry().ExportToWkt())

TypeError: argument of type 'NoneType' is not iterable

This error only appears when the includeFerries=True parameter is supplied. Dropping this parameter loads in OSM data as per normal without a problem.

I'm using a PBF for all of Bangladesh downloaded directly from Geofabrik as of 4 days ago and I clip to my AOI after processing, not before. The PBF has been working fine up until now so I don't have any reason to suspect it's the source of problems, though I confess I don't know the inner workings of Geofabrik PBFs that well.

d3netxer commented 4 years ago

I made a small change and push it back up to the GOSTnets master branch, please try it again @rbanick. Also, fyi the module is now named 'load_osm.py'.

and I made a small test here: https://github.com/worldbank/GOST_PublicGoods/blob/shapefile_import/Implementations/RobertBanick_ShapefileTToG/load_osm_file_w_ferries_test.ipynb

rbanick commented 4 years ago

Good news and bad news. The good news -- this works! Ferries are successfully filtered through. Thank you all!

The bad news, and I am kicking myself for not recognizing this earlier, is that ferries are of course connected to the highway network via piers tagged as man_made=pier, as per here. Thus the ferry lines that do appear are disconnected graphs. We'll need to include pier under the "other_tags" filtering to successfully link ferries to the network.

Also, fun fact, if you are using admin boundaries to clip your roads inputs, they'll exclude any ferry line segments that are 100% over water. Save yourself the confusion and use convex hulls or manually adjusted boundary files!

d3netxer commented 4 years ago

Okay, well that is interesting. I just push another update that should capture the man_made=pier ways as well.

Screen Shot 2020-02-11 at 10 04 43 PM

See pic, you are still going to need to edit OSM and connect the roads to the piers where they should be.

d3netxer commented 4 years ago

I'm glad you found out how extra costs were assigned to model border crossings in the the asia_railways_v2 implementation. I think something similar could definitely be done to model ferries.

However, it would probably just be simpler/easier to add the appropriate costs to the Ferry edges.

rbanick commented 4 years ago

I ran the updated code against updated data this morning -- success! This is working. Thank you all!

As you suggested @d3netxer I manually connected highways to all the relevant piers yesterday so the source data was fine. And I just assigned slow speeds to the ferry edges to reflect waiting times. Easy does it.

One thing to note for future implementations: it's necessary to assign speeds to piers in the speed dictionary. However, in some cases piers and highway tags overlap, how would GOSTNets resolve the conflict? I'm not sure if this is bad tagging or a good way of reflecting piers that can be driven on vs. untagged piers that are implicitly walking only.

I'm leaving this issue open in case you all have additional comments, otherwise feel free to close it.

rbanick commented 4 years ago

2 steps forward, one half step back.

A curious thing occurs with ferries in the updated GOSTNets. Those attached to piers with a highway=* tag take on the associated highway tag in the graph object, i.e. they are represented and routed as highway=* themselves. Those which are attached to piers without a highway tag, that is just manmade=pier, stay tagged as ferries with the corresponding speed assigned to ferries.

This was not a big problem for me as the associated highways had near-equivalent speeds to the ferries but obviously could cause problems in some other cases

In Cox's Bazaar district where I'm working you can see this by comparing results from the northern island and the southern tip, with tagged piers, to the bay, with untagged piers.

d3netxer commented 4 years ago

Ok that is interesting, I wasn't expecting ferries to have a highway tag. I can have it check to see if it is a ferry first, that way ferries won't be tagged as highways.

rbanick commented 4 years ago

Hi @d3netxer

I returned to GOSTNets today to update my Cox's Bazar analysis with some improved destinations data. Unfortunately I found that the includeFerries=True option for load_osm is now mysteriously breaking the graph object by removing many roads from the analysis. A specific breakdown of roads selected with and without ferries included is below.

Feature count with includeFerries=True

unclassified      4740
living_street     3359
track             1438
road               677
service            620
tertiary           369
secondary          152
pier                95
primary             21
ferry                6
trunk                3
tertiary_link        1

Feature count with includeFerries=False

unclassified      5129
living_street     3448
track             1643
service            700
road               692
tertiary           587
secondary          471
primary            194
trunk              126
trunk_link           5
tertiary_link        1

One clue may be that it removes most primary/secondary roads but mysteriously preserves those carrying additional tags (like bridge=yes).

I'm emailing you a copy of the notebook I've been using. A quick compare of edges generated using includeFerries=False/True should show you what I'm talking about. I'm using a fresh download of Geofabrik data for Bangladesh as of today.

d3netxer commented 4 years ago

good catch @rbanick. While debugging I found out that the code was checking first if the way had 'other_tags' and then checked if this subset was a ferry. It would then skip over and just check if a way was a highway. Therefore it was skipping highways that have 'other_tags', which is incorrect.

I added another if statement and committed to master, the new 'if' statement captures highways that have 'other_tags'.

Also, while debugging I found bridge that has both a highway tag and a pier tag! https://www.openstreetmap.org/way/424594370#map=17/21.51974/91.89865

rbanick commented 4 years ago

Thanks for the quick work @d3netxer ! I think some of the internals got scrambled as I'm now getting this error when I run OSM_to_network(f, includeFerries=False):

NameError                                 Traceback (most recent call last)
<ipython-input-7-ef438d9bcb90> in <module>
      1 # includeFerries must be set to true
      2 
----> 3 cxb = OSM_to_network(f, includeFerries=True)

~\.conda\envs\test\lib\site-packages\gostnets-1.0.1-py3.6.egg\GOSTnets\load_osm.py in __init__(self, osmFile, includeFerries)
     43         """
     44         self.osmFile = osmFile
---> 45         self.roads_raw = self.fetch_roads_and_ferries(osmFile) if includeFerries else self.fetch_roads(osmFile)
     46 
     47     def generateRoadsGDF(self, in_df = None, outFile='', verbose = False):

~\.conda\envs\test\lib\site-packages\gostnets-1.0.1-py3.6.egg\GOSTnets\load_osm.py in fetch_roads_and_ferries(self, data_path)
    179 
    180             if len(roads) > 0:
--> 181                 print("hwy_count is {}".format(hwy_count))
    182                 road_gdf = gpd.GeoDataFrame(roads,columns=['osm_id','infra_type','geometry'],crs={'init': 'epsg:4326'})
    183                 return road_gdf

NameError: name 'hwy_count' is not defined

This doesn't occur when includeFerries=False.

d3netxer commented 4 years ago

oh, I think I was printing a hwy_count during testing and left some of the variables in, I just took them out and re-committed to master. please try again

rbanick commented 4 years ago

That works! Thanks!