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

fixes skim creation of simplified graph #248

Merged
merged 1 commit into from
Apr 8, 2021
Merged

Conversation

janzill
Copy link
Contributor

@janzill janzill commented Apr 8, 2021

fixes #247

@janzill
Copy link
Contributor Author

janzill commented Apr 8, 2021

@pedrocamargo it looks like you add links with a_node==b_node after network simplification and set the compact__id to compact_num_links+1, so the groupby object has one element more than the simplified graph.

Given that these are unused links, we do not want to include them in our skims I think.

@pedrocamargo pedrocamargo merged commit c14c08a into master Apr 8, 2021
@pedrocamargo pedrocamargo deleted the janzill/#247 branch April 8, 2021 03:49
@jamiecook
Copy link
Contributor

wouldn't it be better to not add links that go from a->a ?
also just wondering how you can know that [:-1] will catch all such cases...

@pedrocamargo
Copy link
Contributor

The thing is that we set node_b = node_a for all links that are not part of that particular graph, and are not in the graph used for computing paths. This is done for managing super networks in multi-class assignments, as that procedure guarantees the order of the links in the graphs for all modes/classes.
The solution fixes it because the +1 that was introduced is for the Forward-Star that indexes links in the graph with everything.

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

Successfully merging this pull request may close these issues.

Error during 'set_skimming' process
3 participants