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

fix: way surface type storage #1794

Merged
merged 15 commits into from
May 24, 2024
Merged

fix: way surface type storage #1794

merged 15 commits into from
May 24, 2024

Conversation

aoles
Copy link
Member

@aoles aoles commented May 21, 2024

Pull Request Checklist

  • 1. I have rebased the latest version of the main branch into my feature branch and all conflicts
    have been resolved.
  • 2. I have added information about the change/addition to functionality to the CHANGELOG.md file under the
    [Unreleased] heading.
  • 3. I have documented my code using JDocs tags.
  • 4. I have removed unnecessary commented out code, imports and System.out.println statements.
  • 5. I have written JUnit tests for any new methods/classes and ensured that they pass.
  • 6. I have created API tests for any new functionality exposed to the API.
  • 7. If changes/additions are made to the ors-config.json file, I have added these to the ors config documentation
    along with a short description of what it is for, and documented this in the Pull Request (below).
  • 8. I have built graphs with my code of the Heidelberg.osm.gz file and run the api-tests with all test passing
  • 9. I have referenced the Issue Number in the Pull Request (if the changes were from an issue).
  • 10. For new features or changes involving building of graphs, I have tested on a larger dataset
    (at least Germany), and the graphs build without problems (i.e. no out-of-memory errors).
  • 11. For new features or changes involving the graphbuilding process (i.e. changing encoders, updating the
    importer etc.), I have generated longer distance routes for the affected profiles with different options
    (avoid features, max weight etc.) and compared these with the routes of the same parameters and start/end
    points generated from the current live ORS.
    If there are differences then the reasoning for these MUST be documented in the pull request.
  • 12. I have written in the Pull Request information about the changes made including their intended usage
    and why the change was needed.
  • 13. For changes touching the API documentation, I have tested that the API playground renders correctly.

Fixes #1779.

Information about the changes

In order to be able to encode surface categories as 4 bit values their number has been reduced to 16 by removing the following ones:

  • Woodchips: Not particularly common (0.02%) or relevant for routing on ways, because typical features exhibiting this type of surface are located within playgrouds, parks, etc. Note that this has never been functional because its original value of 16 exceeds the available data type range of 4 bits.
  • Cobblestone: the OSM tag surface=cobblestone is quite generic as it is used for both sett and unhewn_cobblestone. As such it is ambiguous, because sett is already classified as "Paving Stones".
  • Fine gravel: ambiguous as according to OSM wiki surface=fine_gravel is used somewhat inconsistently. It may be used to specify fine loose gravel (...), but it is also sometimes used as an alias for compacted. Therefore, it seems to make sense to marge it with the more generic category for surface=gravel which has very large meaning range. Used for cases ranging from huge gravel pieces like track ballast used as surface, through small pieces of gravel to compacted surface.

Additionally, the category for surface=earth (0.25%) has been changed from "Ground" to "Dirt", and some less common surface types have been assigned to one of the existing categories. All the changes are summarized in
the table below. It lists the top 50 values of way surface tags which have a Wiki entry, with "Special" types of surfaces removed. The values from the change column override those from the current one.

value count % current change
asphalt 27299245 44.73 Asphalt
unpaved 11398669 18.68 Unpaved
paved 4049616 6.64 Paved
concrete 3360006 5.51 Concrete
paving_stones 3306257 5.42 Paving Stones
ground 3209076 5.26 Ground
gravel 2086265 3.42 Gravel
dirt 1538632 2.52 Dirt
grass 1158016 1.9 Grass
compacted 1027824 1.68 Compacted
sand 520011 0.85 Sand
sett 430232 0.7 Paving Stones
fine_gravel 397460 0.65 Fine Gravel Gravel
wood 208923 0.34 Wood
concrete:plates 177157 0.29 Concrete
cobblestone 151306 0.25 Cobblestone Paving Stones
earth 150580 0.25 Ground Dirt
pebblestone 121603 0.2 Compacted
metal 41043 0.07 Metal
grass_paver 40869 0.07 Grass_Paver
unhewn_cobblestone 31514 0.05 Paving Stones
mud 26101 0.04 Ground
concrete:lanes 26090 0.04 Concrete
rock 21325 0.03 Unpaved
stone 11348 0.02 Unpaved
woodchips 9924 0.02 Woodchips Unpaved
soil 7045 0.01 Dirt
bricks 6578 0.01 Paving Stones
brick 6563 0.01 Paving Stones
trail 5499 0.01
chipseal 3417 0.01 Asphalt
metal_grid 2618 0
cement 2595 0 Concrete
ice 2209 0 Ice
stepping_stones 1850 0
paving_stones:30 1636 0 Paving Stones
shells 1108 0 Unpaved
rocks 923 0 Unpaved
bitmac 543 0 Asphalt
snow 365 0 Ice
tarmac 272 0 Asphalt
salt 197 0 Unpaved
paving_stones:20 147 0 Paving Stones

aoles added 4 commits May 21, 2024 13:24
Use enumerations instead of simple integer constants in order to be able to store the surface types internally under different IDs than the corresponding API values.
Check whether values corresponding to the surface types can be reliably written and retrieved from the storage. The test currently fails because the number of different surface types exceeds the available data type range of 4 bits.
Remove some less common and relevant surface types such as "woodchips", or ambiguous ones: "fine gravel" and "cobblestone" in order to be able to encode them as 4-bit values. Note that "woodchips" has never been functional because its original value of 16 exceeds the available data type range of 4 bits.
Improves code readability and potentially performance.
@takb takb added this to To do in ors general May 21, 2024
@aoles aoles changed the title Fix way surface type storage fix: way surface type storage May 21, 2024
@github-actions github-actions bot added the fix label May 21, 2024
aoles added 9 commits May 22, 2024 00:14
Address issue reported by sonarcloud by replacing calls to `com.graphhopper.storage.Directory#find(java.lang.String)` by `com.graphhopper.storage.Directory#create(java.lang.String)`
According to OSM wiki `surface=earth` is a synonym of `surface=dirt` and is used for where surface is exposed earth/soil/dirt but it is not sand or gravel or rock.
Ways labelled with OSM tag `surface` set to "unhewn_cobblestone", "bricks" or "brick" are classified as surface type "Paving Stones" rather than "Unknown".
Ways labelled with OSM tag `surface` set to "rock", "rocks", "stone", "shells" or "salt" are classified as surface type "Unpaved" rather than "Unknown".
Ways labelled with OSM tag `surface` set to "chipseal", "bitmac" or "tarmac" are classified as surface type "Asphalt" rather than "Unknown". According to OSM wiki most of chipseal and bitmac surfaces are likely tagged as `surface=asphalt`, whereas `surface=tarmac` is often incorrectly used for asphalt/concrete surfaces as actual tarmacadam is very unusual.
Ways labelled with OSM tag `surface` set to "soil" are classified as surface type "Dirt" rather than "Unknown".
…crete"

According to OSM wiki `surface=cement` is often used, by mistake, instead of `surface=concrete`.
@github-actions github-actions bot added fix and removed fix labels May 24, 2024
aoles added 2 commits May 24, 2024 11:24
Road classification doesn't necessarily imply surface. For example, driveways which are supposed to be tagged as `highway=service` + `service=driveway` are in many cases not paved.
@aoles aoles marked this pull request as ready for review May 24, 2024 09:37
@github-actions github-actions bot added fix and removed fix labels May 24, 2024
@github-actions github-actions bot added fix and removed fix labels May 24, 2024
Copy link

sonarcloud bot commented May 24, 2024

@github-actions github-actions bot added fix and removed fix labels May 24, 2024
Copy link
Contributor

@sfendrich sfendrich left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for the fix and all the cleanup!

@sfendrich sfendrich merged commit a3e4c36 into main May 24, 2024
53 checks passed
ors general automation moved this from To do to Awaiting release May 24, 2024
@sfendrich sfendrich deleted the fix/way_surface_type_storage branch May 24, 2024 10:30
aoles added a commit that referenced this pull request May 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
ors general
  
Awaiting release
Development

Successfully merging this pull request may close these issues.

Extra Info - Surface returns paved instead of grass
2 participants