Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use an undirected approach rather than double directed #42

Closed
barneydobson opened this issue Feb 13, 2024 · 2 comments · Fixed by #142
Closed

Use an undirected approach rather than double directed #42

barneydobson opened this issue Feb 13, 2024 · 2 comments · Fixed by #142

Comments

@barneydobson
Copy link
Collaborator

barneydobson commented Feb 13, 2024

Following discussion with @cheginit

Intuitively, the topology derivation should start from an undirected path, as the dominant flow of water may occur in either direction along the pipe. The slope of the pipe means that we will of course strongly prefer one direction to another, but from a feasibility perspective, it may be that some uphill pipes are needed - or that a negative slope can be corrected for in the design stage by laying the pipe deeper at one end to compensate. Regardless - we know that uphill pipes exist in real systems.

The current way to address this in the code is to make a double directed graph (see graph_utilities.py/double_directed). This is a directed graph where every two nodes that are connected have an edge in both directions. This allows a negative slope to be set for one edge flowing in the uphill direction, and positive in the downhill. From a topology perspective, if we want to strongly enforce downhill slopes wherever possible, we would set the surface_slope_scaling and surface_slope_exponent parameters to be very large and thus in graph_utilities.py/derive_topology the shortest path algorithm will select these.

@cheginit has pointed out that this feels overly confusing, and that there is likely to be a more elegant approach that can accommodate for this using an undirected graph. This is an issue to discuss how to do that.

https://osmnx.readthedocs.io/en/stable/user-reference.html#osmnx.convert.to_undirected - osmnx.to_undirected preserves arcs if different geometries and otherwise combines - maybe something like could be handy

@barneydobson barneydobson mentioned this issue Feb 13, 2024
13 tasks
@barneydobson
Copy link
Collaborator Author

@cheginit so I am having some difficulties with your network cleaning because of interactions with other graphfcns, and particularly this one. I will set out my thoughts and a proposal below:

  • As above, the only valid reason for a double directed graph is to enable asymmetric slope calculations for weighting. Thus, double_directed should probably only be called as part of the weight calculation.
  • This will massively simplify split_long_edges, addressing Refactor split_long_edges #39 .
  • The lane width calculations will need to be performed elsewhere. As I think you identified somewhere, worrying about width and lanes and calculating all these things in different places is complicated. Instead, as part of format_osmnx_lanes, we perform the lane buffering, write it to a shapefile, and load that shapefile alongside buildings.geoparquet as part of calculate_contributing_area - then there is no need to worry about lanes or widths again.

@cheginit
Copy link
Collaborator

I still don't quite follow what's your difficulty, so I'll articulate my understanding of the problem. The main objective is to accommodate negative slopes in BUSN pipes based on slope. So, instead of assigning two different slope attributes to edges, you decided to duplicate edges that might have negative slopes. Am I understanding this correctly? I am still not clear on how this would help with enforcing adverse slopes, though.

Regarding the split_long_edges function, have you looked at my approach in the subcatchment notebooks that uses shapely.segmentize? You can also the merge_points function in the notebook.

About the lane width, I think it makes sense to save the graph with polygon streets and then use it later on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants