Skip to content

graph construction cleanup - #188

Merged
jteresco merged 3 commits into
TravelMapping:masterfrom
yakra:GraphGen5
Mar 12, 2019
Merged

graph construction cleanup#188
jteresco merged 3 commits into
TravelMapping:masterfrom
yakra:GraphGen5

Conversation

@yakra

@yakrayakra commented Feb 5, 2019

Copy link
Copy Markdown
Contributor

self.vertex1 & self.vertex2 were already defined as graph.vertices[s.waypoint1.unique_name] & graph.vertices[s.waypoint2.unique_name], making these lookups unnecessary. Saves about 0.4s during graph setup.

The HighwayGraphCollapsedEdgeInfo constructor is already done this way.

@yakrayakra changed the title remove extraneous lookupsgraph construction cleanupFeb 5, 2019
@yakra

yakra commented Feb 5, 2019

Copy link
Copy Markdown
ContributorAuthor

May have some more commits to add to this.

@yakra

yakra commented Feb 6, 2019

Copy link
Copy Markdown
ContributorAuthor

31c9602:

We can visit each concurrrent HighwaySegment only once by checking whether it's the first segment in its concurrency list. Doing so removes the need to check a name for each to see if it's been visited, thus removing the need to store the visited & segment_name variables for each HighwaySegment. Thus we can remove 2 FOR loops (29 lines of code) and construct edges right away, getting segment names from HighwaySegment.segment_name() during construction.

Calling the HighwayGraphEdgeInfo and initial HighwayGraphCollapsedEdgeInfo constructors have been combined into the same FOR loop.

@yakra

Copy link
Copy Markdown
ContributorAuthor

Putting this on hold until I get yakra#64 sorted.

@yakra

Copy link
Copy Markdown
ContributorAuthor

This one should be OK.

@yakrayakra reopened this Feb 10, 2019
@jteresco

Copy link
Copy Markdown
Contributor

I'm holding off on merging in any additional graph generation changes while I'm using the graphs heavily in class. One more week of that then we'll have a break from all that and I'll be a little more adventurous.

@yakra

Copy link
Copy Markdown
ContributorAuthor

Understandable. That graphs are used in your classes is a pretty important reason to make sure any changes are done right, and that things behave in an expected, predictable way.

@yakrayakra mentioned this pull request Feb 11, 2019
@yakrayakra added the speed label Feb 11, 2019
@jteresco
jteresco merged commit 22e551d into TravelMapping:masterMar 12, 2019
@yakra
yakra deleted the GraphGen5 branch March 16, 2019 18:18
Sign up for freeto 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

@yakra@jteresco