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

Save both forward and reverse datasources. #4352

Merged
merged 1 commit into from
Jul 28, 2017
Merged

Conversation

danpat
Copy link
Member

@danpat danpat commented Jul 27, 2017

This PR addresses #4346 where we were not correctly saving or retrieving both forward and reverse datasource attributes for road segments (roads can have data from different sources in different directions).

The basic summary is that we did not have separate storage for forward and reverse data - so we were randomly overwriting with whichever datasource was applied to the graph last. There was also an off-by-one bug that meant we were often seeing datasource values from other roads on the first/last segment of the road in question.

In the following two images, colours represent different sources of data:

Before this change, there was a lot of this going on:

28646941-272e38f8-7219-11e7-8ccd-1b50b5988c3f

After this change, the map is a lot more consistent, as expected:

screen shot 2017-07-27 at 12 49 27 pm

NOTE: this doubles the amount of disk/RAM needed to store datasource IDs. The good news is that we only use std::uint8_t for each segment.

I tested with mexico-latest.osm.pbf just to confirm, and the .geometry file increased by the expected amount:

before: 211,090,822
after: 223,418,490
% increase: ~5% (note this is just for this file, it's closer to 2% for the total dataset)

I would expect this to scale for the planet file as well.

NOTE 2: This PR changes the file format (we store more data now) - there's no way to backport this compatibly to 5.9.

Tasklist

  • add regression / cucumber cases (see docs/testing.md)
  • review
  • adjust for comments

@danpat danpat added this to the 5.10.0 milestone Jul 27, 2017
@danpat danpat added the Review label Jul 27, 2017
@danpat danpat requested a review from TheMarex July 27, 2017 20:12
@danpat danpat force-pushed the fix_reverse_datasources branch 2 times, most recently from 71930ca to 4b109c8 Compare July 27, 2017 20:18
@danpat danpat force-pushed the fix_reverse_datasources branch from 4b109c8 to be5fc50 Compare July 27, 2017 21:45
Copy link
Member

@TheMarex TheMarex left a comment

Choose a reason for hiding this comment

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

Looks good to me. 👍

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