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

network ring bug #655

Closed
dfolch opened this issue Jul 9, 2015 · 15 comments
Closed

network ring bug #655

dfolch opened this issue Jul 9, 2015 · 15 comments

Comments

@dfolch
Copy link
Member

dfolch commented Jul 9, 2015

In the case of a ring road with no intersections, the Network builder fails. The traceback is not informative to the user. This is a rare case, so is a low priority.

code

ring_road

@dfolch dfolch added this to the Release + 1 milestone Jul 9, 2015
@dfolch
Copy link
Member Author

dfolch commented Feb 27, 2016

Here is the troublemaker shapefile. unconnected_ring.zip

@chiragvartak
Copy link

The above mentioned shapefile is not the only one that causes problems. I managed to reproduce the error in at least the files Polygon.shp and 10740.shp. (Figure below). There might be more.
I found another recurring error: 'Point' object has no attribute 'vertices', when I tried to create a Network with the shapefiles: baltim.shp, burkitt.shp, crimes.shp, schools,shp, juvenile.shp. Again, there might be more. (Should I create a new issue regarding this?)
Also, I feel it would be great to have a method that would give us a list-of-all-files-with-a-specified-extension, given a list-of-names-of-folders in pysal/examples. Something like:
pysal.examples.get_files(L=['10740', 'geodanet'], ext='shp')
which would then return:
['10740.shp', 'crimes.shp', 'schools.shp', 'streets.shp']
Thus, doing pysal.examples.get_files(L=pysal.examples.available(), ext='shp') would give us all the shapefiles and thus testing a function on all of them could be easily done!
(So, create a new issue regarding this too?)
Thoughts, anyone?
network_error

@sjsrey
Copy link
Member

sjsrey commented Feb 27, 2016

I think the idea of getting a listing of files by type is an interesting one - maybe creating a separate issue from the network thread here would be a good way to start. We could then flesh out the ideas more fully under that issue.

@sjsrey
Copy link
Member

sjsrey commented Feb 27, 2016

@chiragvartak the extraction of the network works on a polyline shape file, while a points shape file is then used to snap the points onto the network.

We probably should be more explicit in the argument types as currently the signature is vague on the shapefile type.

@chiragvartak
Copy link

The docstring of extractgraph says:

Using the existing network representation, create a graph based representation, by removing all nodes with neighbor incidence of two.

What is the reason behind doing this? Why aren't we representing the shapefile as a graph as it is?

@ljwolf
Copy link
Member

ljwolf commented Feb 28, 2016

When using a polyline shapefile as a network, sometimes you can remove some nodes without changing the resulting topology. I think the original implementor, @jlaura, saw it'd be helpful to keep both the raw polyline set read in from the file, as well as a simplified representation with the same topology.

Also, just to be clear, the errors you're noticing in constructing a network from something like baltim.shp is because baltim.shp does not describe a network. Neither does Polygon.shp, but there's probably some more graceful way we could bail out in these cases.

@jlaura
Copy link
Member

jlaura commented Feb 29, 2016

@ljwolf is correct that this is going to be a question of storing not just a single representation (potentially).

For example, assume that we read in the raw polyline shapefile and convert to a networkx style graph. This is no longer a road network representation, but a reduced topological representation. That might be great for things like djikstras due to performance (assuming that the true distance is attributed to the topological edge). What about doing point snapping and edge counts? The original shapefile is one potential representation, as is the topological representation, as are a myriad of other representations (equal edge segmentation / aggregation over the original shapefile to attempt to standardize edge lengths for example).

In relation to #746, the issue is going to be how to manage different representations of the network and maybe guess at the appropriate realization to use for a given metric.

@chiragvartak
Copy link

Okay, I get it. But now it presents me with another problem. Presently one thing that is being done to simplify the topology is to remove the bridge nodes i.e. nodes with 2 neighbors. But a ring has all nodes with 2 neighbors! Hence, all nodes are removed. The code is doing exactly what is was designed to do but perhaps not what it was intended to do.
Any suggestions on how to handle this?
Also, I've fixed the code to make it more robust, like handling cases where a 'shape' has multiple parts, etc. Create a pull request w.r.t that?

@jlaura
Copy link
Member

jlaura commented Mar 1, 2016

What about pre-processing the full topology to identify cycles. Then apply
the remove the incidence two logic to the non-cycles.

Question: Should a ring be reduced to a point given the current logic?

On Tue, Mar 1, 2016 at 10:19 AM, chiragvartak notifications@github.com
wrote:

Okay, I get it. But now it presents me with another problem. Presently one
thing that is being done to simplify the topology is to remove the bridge
nodes i.e. nodes with 2 neighbors. But a ring has all nodes with 2
neighbors! Hence, all nodes are removed. The code is doing exactly what is
was designed to do but perhaps not what it was intended to do.

Any suggestions on how to handle this?

Also, I've fixed the code to make it more robust, like handling cases
where a 'shape' has multiple parts, etc. Create a pull request w.r.t that?


Reply to this email directly or view it on GitHub
#655 (comment).

chiragvartak added a commit to chiragvartak/pysal that referenced this issue Mar 1, 2016
Fixes pysal#655, and some other minor changes are made:
(1) Added a parameter 'own_edges' to the Network class constructor
(2) Modified the code to work with shapefiles that have shapes with
multiple parts
(3) Added an example to pysal/examples
@chiragvartak
Copy link

@jlaura With the current logic, the cycle is supposed to be reduced to nothing. But it doesn't happen.
So, I fixed it and the modified code applies the following logic: A cycle is ignored (actually, it is processed and reduced to nothing) and the other non-cycles are processed properly. I guess this should be alright?

@dfolch
Copy link
Member Author

dfolch commented Mar 1, 2016

I don't think an isolated ring should be removed entirely. Similarly, I don't think an isolated line should be removed either. We might consider a point (@jlaura) or a triangle. A triangle has the least number of vertices of something looking like a ring. In the north of the Waverly data (Waverly.zip) is a self intersecting road. I think the loop part of this gets reduced to a point under the current code in master.

@jlaura
Copy link
Member

jlaura commented Mar 2, 2016

@dfolch Agreed that removal is not appropriate. The question is, what is appropriate. I could see an argument for a point, line, or triangle. Do we have any references how this is handled in the literature?

@jGaboardi
Copy link
Member

@dfolch @ljwolf @jlaura @chiragvartak
Here is a gist notebook describing and demonstrating the 'loop road' issue.
Millstream Road

@jGaboardi
Copy link
Member

Waverly.zip

@jGaboardi
Copy link
Member

This issue is resolved with pysal/spaghetti#185. For a demonstration of the new functionality with unconnected_ring.shp see resolved-unconnected_ring.ipynb.

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

No branches or pull requests

6 participants