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

False positive "Discordant maxspeed and source:maxspeed" in Poland. #437

Closed
matkoniecz opened this issue Jan 20, 2019 · 9 comments
Closed

Comments

@matkoniecz
Copy link
Contributor

matkoniecz commented Jan 20, 2019

Tagging that road has rural speed limit is done in many ways.

  1. Explicit tagging of maxspeeds
  2. Explicit tagging of maxspeeds and symbolic value Pl:rural in source:maxspeed
  3. symbolic value Pl:rural in maxspeed
  4. at least one more system

This is messy and hairy and there is plenty to complain but in http://osmose.openstreetmap.fr/en/error/22358635600 there is no valid reason to complain about "Discordant maxspeed and source:maxspeed"

highway | residential |  
lit | no |  
maxspeed | 90 |  
maxspeed:bus | 70 |  
maxspeed:hgv:conditional | 90 @ (weight<=3.5); 70 @ (weight>3.5) |  
maxspeed:trailer | 70 |  
noname | yes |  
source:maxspeed | PL:rural |  
surface | asphalt

@frodrigo
Copy link
Member

frodrigo commented Mar 3, 2019

In Osmose and OSRM the PL:rural is 100 (I make both).

@frodrigo frodrigo added the bug label Mar 3, 2019
@matkoniecz
Copy link
Contributor Author

matkoniecz commented Mar 3, 2019

Oh right, it is even worse.

https://pl.wikipedia.org/wiki/Wzory_znak%C3%B3w_i_sygna%C5%82%C3%B3w_drogowych_w_Polsce#/media/File:Znak_D-39._Ograniczenia_pr%C4%99dko%C5%9Bci_w_Polsce_od_2011.svg

PL:rural means maxspeed 90 for cases where there is single lane in one direction and road is not an express road.

PL:rural means maxspeed 100 for dual carriageway with multiple lanes

also PL:rural means maxspeed 100 for express roads that are not dual carriageways

See previous discussion in streetcomplete/StreetComplete#1101

Also, search for "PL" in https://wiki.openstreetmap.org/wiki/Default_speed_limits that attempts to document this.


For OSRM intepreting PL:rural as 100 is probably fine (is there a big difference between 90 and 100?)

For Osmose I would allow both 90 and 100.

Or allow 90 only where oneway!=yes and motorroad!=yes and 100 only where oneway=yes or motorroad=yes.


And yes, sign D-39 is self-parody of overly complex rules.

https://commons.wikimedia.org/wiki/File:Znak_D-39._Ograniczenia_pr%C4%99dko%C5%9Bci_w_Polsce_od_2011.svg

@frodrigo
Copy link
Member

frodrigo commented Mar 3, 2019

I can't handle the full complexity, but I can do this:

PL:urban = [50, 60]
PL:rural = [90, 100]
PL:trunk = [100, 120]
PL:motorway = [140]

It's ok for you?

@matkoniecz
Copy link
Contributor Author

PL:urban should be always 50 (60 is during night 23:00 - 5:00 and handled with maxspeed:conditional).

PL:urban = [50]
PL:rural = [90, 100]
PL:trunk = [100, 120]
PL:motorway = [140]

will be OK

frodrigo added a commit to frodrigo/osmose-backend that referenced this issue Mar 4, 2019
@frodrigo frodrigo added the ready label Mar 4, 2019
jocelynj added a commit that referenced this issue Mar 4, 2019
* frodrigo/master:
  py3: user bytearray in place of str
  py3: osmbin, read storage file in binary mode
  py3: rename Str to Bytes in OsmBin
  Fix PL:rural and PL:trunk in TagFix_Maxspeed #437
  office=tax but office=government + government=tax in analyser_merge_service_public_FR #450
  Support multiple municipality_ref in analyser_osmosis_boundary_relation #429
  Fence with materal tag, better use fence_type tag #447
  Support maxspeed:forward and maxspeed:backward for all speed values in analyser_merge_traffic_signs #430
  Add barrier + maxheight in analyser_merge_traffic_signs #446
  Plugin Phone, auto enable
  Plugin Phone, better regex
  Plugin Phone validate allowed char and separator
  Plugin Phone, check for international prefix
  Review Phone plugin #290
  config: split France to departement level
  Update data source of analyser_merge_pitch_FR #466
  Merge, change tag for dancing school #469
  Fix typo #334
  White list transformer=minor_distribution. #111
  Better i18n strings
@matkoniecz
Copy link
Contributor Author

I get

Traceback (most recent call last):
  File "/data/project/osmose/frontend/errors.py", line 136, in graph
    data = errors_graph.make_plt(db, options, format)
  File "/data/project/osmose/frontend/errors_graph.py", line 113, in make_plt
    return plot(data, text+' '+src, format)
  File "/data/project/osmose/frontend/errors_graph.py", line 146, in plot
    locator.refresh()
  File "/data/project/osmose/frontend/osmose-frontend-venv/lib/python2.7/site-packages/matplotlib/dates.py", line 1269, in refresh
    dmin, dmax = self.viewlim_to_dt()
  File "/data/project/osmose/frontend/osmose-frontend-venv/lib/python2.7/site-packages/matplotlib/dates.py", line 1026, in viewlim_to_dt
    .format(vmin))
ValueError: view limit minimum -0.001 is less than 1 and is an invalid Matplotlib date value. This often happens if you pass a non-datetime value to an axis that has datetime units

in the second link

@frodrigo
Copy link
Member

frodrigo commented Mar 5, 2019

The github link miss the * at the end. But wait, the code was enabled this night, and the results are not yet here.

@frodrigo
Copy link
Member

frodrigo commented Mar 7, 2019

Backend server still not uptodate.

@frodrigo
Copy link
Member

Uptodate. We move from 18 000 to 900!

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

No branches or pull requests

2 participants