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

Cleanup the profiles #1478

Merged
merged 7 commits into from
May 17, 2015
Merged

Cleanup the profiles #1478

merged 7 commits into from
May 17, 2015

Conversation

TheMarex
Copy link
Member

@emiltin could you have a look at the changes I made to the bike profile? I added safety penalties and disables the usage of public transportation by default.

@emiltin
Copy link
Contributor

emiltin commented May 15, 2015

Adding a penally for all primary, secondary and tertiary because they're unsafe seems a bit drastic to me. For example, in Copenhagen, most of such road have bike lanes (separated from car traffic with a raised curbs), and would not be considered unsafe. Most adults would choose them for going to school and work, for example.

Nørrebrogade is one of the busiest bike streets in Copenhagen, and is tagged as a tertiary: http://www.openstreetmap.org/way/27432854. Parts of it don't even allow cars.

One of the busiest car roads is also tagged as tertiary: http://www.openstreetmap.org/way/8122273. Even this road is not considered unsafe for bikes, since there are bike tracks, and in many places the bike tracks are even separated from car lanes with trees.

For this reason I would recommend considering at least the 'cycleway' tag. Note that this tag is particular tricky since you can have 'opposite_lane/track' and this can be combimed with oneway tags.

You migh also consider access tags for cars and other motor vehicles; if they're not allowed, it should be safer. Maxspeed for cars could be another thing to look at.

I would also treat primary/secondary/tertiary different.

Yes, bike routing is generally more complex that cars :-)

Another issues with all this is the fact that speed and impedance is not separated, so the only way you can prefer certain streets is to manipulate the speed, resulting in abnormal travel times. Just because a street is unsafe doesn't mean you go slow.

@emiltin
Copy link
Contributor

emiltin commented May 15, 2015

Bringing bikes on trains and metros is debatable, but I think it's fine to disable that be default. In any case it's not really working as it should, I think because some train lines are tagged using relations rather than using the route tag.

@emiltin
Copy link
Contributor

emiltin commented May 15, 2015

Btw, I think it would be great to refactor the profiles and create some common structure for the usual processing flow of access, oneway, surface, speeds, etc.

@TheMarex
Copy link
Member Author

Thanks for the feedback @emiltin! I don't really want to change the default behavior, just make it easier in general to configure stuff. I'm not too happy about the penalties as well, as it skews the ETAs a lot and I hope we can separate the travel time and edge weight in the future. But it is seems like good heuristic, especially in the US.

@systemed has probably a lot of experience tuning penalties like that for GB.

For this reason I would recommend considering at least the 'cycleway' tag. Note that this tag is particular tricky since you can have 'opposite_lane/track' and this can be combimed with oneway tags.

The penalty only gets applied when the road is not tagged with cycleway, because the speed gets overwritten here.

But yeah the control flow is totally confusing. I wonder how we could make this hierarchy more explicit.

@TheMarex TheMarex force-pushed the fix/profiles-cleanup branch from c862480 to de2f069 Compare May 16, 2015 12:40
@TheMarex
Copy link
Member Author

Tests pass, I'm going to merge this first version.

TheMarex added a commit that referenced this pull request May 17, 2015
@TheMarex TheMarex merged commit 6d9c3bc into develop May 17, 2015
@TheMarex TheMarex deleted the fix/profiles-cleanup branch May 17, 2015 22:09
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.

2 participants