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

match vertices once per simple/collapsed pair #185

Merged
merged 1 commit into from
Feb 5, 2019

Conversation

yakra
Copy link
Contributor

@yakra yakra commented Feb 4, 2019

Closes #183.

@jteresco, requesting a review:
Changes were more extensive this time around. In particular, graph_list is now passed to write_subgraphs_tmg, and the entries are appended there. I want to be sure I didn't goof anything up that would cause the graph list entries to get mangled when we put them into the DB.

Copy link
Contributor

@jteresco jteresco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those time savings are excellent! As far as the code changes, I'm not seeing anything that concerns me, but I admit I haven't looked super carefully. I think what I'll do is merge it in tonight and see how it goes. I can always back up and regenerate for this week's Algorithms labs (which are going to be using the graphs extensively) if there's any hint of trouble.

Copy link
Contributor

@jteresco jteresco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Specifically addressing your request to look at the population of the graph list, I'm very confident that that change will not cause trouble.

@jteresco jteresco merged commit a5a96c7 into TravelMapping:master Feb 5, 2019
@yakra yakra deleted the GraphGen3 branch February 5, 2019 07:34
@yakra yakra mentioned this pull request Mar 29, 2019
@yakra yakra mentioned this pull request May 19, 2024
@yakra yakra mentioned this pull request Jun 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants