Unidirectional traffic signal segments are currently not compressed.
This means traffic signals which are not on turns can be missed and
not applied the correct penalty.
This commit changes this behaviour to correctly handle the graph
compression. Additional tests are added to ensure there is no
regression for other cases (turns, restrictions).
Co-authored-by: Michael Bell <michael@mjjbell.com>
* print tracebacks and line numbers for Lua runtime errors
* revert format changes
* update changelog with lua traceback, #6564
* revert using protected_function for old GetStringListFromFunction and source_function #6564
* add unit test for line numbers in tracebacks, #6564
* apply clang-format (#6564)
* remove unused test helper function, #6564
* suppress leaksanitizer warnings in extract-tests, #6564
When the extractor encounters a lua runtime error, some osmium objects are not freed. In production this doesn't matter because these errors bring down OSRM. In the tests we catch them to ensure they occur, and the leaksanitizer flags them.
This change takes the existing typedefs for weight, duration and
distance, and makes them proper types, using the existing Alias
functionality.
Primarily this is to prevent bugs where the metrics are switched,
but it also adds additional documentation. For example, it now
makes it clear (despite the naming of variables) that most of the
trip algorithm is running on the duration metric.
I've not made any changes to the casts performed between metrics
and numeric types, they now just more explicit.
Weight and duration penalties are flipped in the lambda function
that applies penalties from traffic signals.
Duration is in deciseconds, whilst weight is multipled by
10^weight_precision, with weight_precision being 1 by default.
Therefore, for default routability profile, the penalties end up
being the same, hence why no tests picked this up.
If distance weight is used however, it will incorrectly apply an
additional penalty to the weight, and not add the traffic signal
delay to the duration in the routing graph.
To confuse things further, in some API responses the values are
correct because they use geometry data instead, but it's still
possible that a sub-optimal route was selected.
However, given the distance weight is in meters, and the additional
penalty per traffic light would be 20, it's unlikely this would
have changed the routing results.
In any case, we correct the function to apply the arguments correctly.
Due to some rather complex logic that tries to calculate intersection
angles by looking further up the road, it's possible to return
an intersection view that is missing a u-turn - something which
is assumed to exist in later guidance calculations.
We apply a fix here by ensuring the u-turn is always included in
the returned view.
Currently OSRM parses traffic signal nodes without consideration
for the direction in which the signal applies. This can lead
to duplicated routing penalties, especially when a forward and backward
signal are in close proximity on a way.
This commit adds support for directed signals to the extraction and
graph creation. Signal penalties are only applied in the direction
specified by the OSM tag.
We add the assignment of traffic directions to the lua scripts,
maintaining backwards compatibility with the existing boolean
traffic states.
As part of the changes to the internal structures used for tracking
traffic signals during extraction, we stop serialising/deserialising
signals to the `.osrm` file. The traffic signals are only used by
`osrm-extract` so whilst this is a data format change, it will not
break any existing user processes.
Currently there is an edge-case in the turn restriction implementation,
such that routes can not be found if the target input location snaps
to a way used in a (multi) via restriction.
With the addition of snapping input locations to multiple ways, we
can now also snap to the "duplicate" edges created for the restriction graph,
thereby fixing the problem.
This is achieved by adding the duplicate restriction edges to the
geospatial search RTree.
This does open up the possibility of multiple paths representing exactly
the same route - one using the original edge as a source, the other
using the duplicate restriction graph edge as source. This is fine,
as both edges are represented by the same geometry, so will generate
the same result.
This change unblocks the osrm-extract debug build, which is
currently failing on a maneuver override assertion.
The processing of maneuver overrides currently has three issues
- It assumes the via node(s) can't be compressed (the failing assertion)
- It can't handle via-paths containing incompressible nodes
- It doesn't interop with turn restriction on the same path
Turn restrictions and maneuver overrides both use the same
from-via-to path representation.
Therefore, we can fix these issues by consolidating their
structures and reusing the path representation for
turn restrictions, which already is robust to the above
issues.
This also simplifies some of the codebase by removing maneuver
override specific path processing.
There are ~100 maneuver overrides in the OSM database, so the
impact on processing and routing will be minimal.
Currently route results are annotated with additional path information,
such as geometries, turn-by-turn steps and other metadata.
These annotations are generated if they are not requested or returned
in the response.
Datasets needed to generate these annotations are loaded and available
to the OSRM process even when unused.
This commit is a first step towards making the loading of these datasets
optional. We refactor the code so that route annotations are only
generated if explicitly requested and needed in the response.
Specifically, we change the following annotations to be lazily generated:
- Turn-by-turn steps
- Route Overview geometry
- Route segment metadata
For example. a /route/v1 request with
steps=false&overview=false&annotations=false
would no longer call the following data facade methods:
- GetOSMNodeIDOfNode
- GetTurnInstructionForEdgeID
- GetNameIndex
- GetNameForID
- GetRefForID
- GetTurnInstructionForEdgeID
- GetClassData
- IsLeftHandDriving
- GetTravelMode
- IsSegregated
- PreTurnBearing
- PostTurnBearing
- HasLaneData
- GetLaneData
- GetEntryClass
Requests that include segment metadata and/or overview geometry
but not turn-by-turn instructions will also benefit from this,
although there is some interdependency with the step instructions
- a call to GetTurnInstructionForEdgeID is still required.
Requests for OSM annotations will understandably still need to
call GetOSMNodeIDOfNode.
Making these changes unlocks the optional loading of data contained in
the following OSRM files:
- osrm.names
- osrm.icd
- osrm.nbg_nodes (partial)
- osrm.ebg_nodes (partial)
- osrm.edges
The internal representation of turn restrictions expects only one
`from` way and only one `to` way.
`no_entry` and `no_exit` turn restrictions can have multiple `from` and
`to` ways respectively. This means they are not fully supported by
OSRM's restriction parser.
We complete support for these turn restriction types by parsing all
ways and converting a valid restriction with multiple `from`/`to` members
into multiple internal restrictions.
- Fix typo in util function name for_each_indexed.
- Use the overloaded functions for_each_indexed and for_each_pair
with a container argument where possible to improve readability.
This is a workaround for discrepancy between MSVC 19.27 and 19.28
about static const member definition
https://developercommunity2.visualstudio.com/t/discrepancy-between-msvc-1927-vs-1928-about-static/1255338
We can not use C++17 inline variable as a workaround suggested
in the issue report linked above, because the sol2 does not
seem to compile in C++17 mode:
third_party/sol2/sol2/sol.hpp: error C2039: 'object_type': is not a member of...
Duplicate restriction nodes in the edge-based-graph are currently
not in included in a mapping (.osrm.cnbg_to_ebg) from
node-based-graph edges to edge-based-graph nodes.
This mapping is used by the MLD partitioner to assign EBG nodes
to partitions.
The omission from the mapping means all restriction nodes are
included in a special 'invalid' partition. This special partition
will break the geolocation properties of the multi-level hierarchy.
The partition and its super levels will have a large number of
border nodes and very few internal paths between them.
Given the partitioner is the only consumer of the mapping, we fix
the issue by including the duplicate restriction nodes in the mapping,
so that they are correctly assigned to a partition.
This has measurable improvement on MLD routing.
For a country-sized routing network, the fix reduces routing and table
request computation time by ~2% and ~6% respectively.
Replace Travis for continuous integration with Github Actions.
The Github Actions pipeline is functionally equivalent, with
all the same build permutations supported.
Whilst the Github Actions offering is broadly equivalent to
Travis, a few changes have been made as part of the migration.
- The 'core' and 'optional' Travis stages have been consolidated
into one build matrix. This is due to the current inability in
Github Actions to share build steps between jobs, so this avoids
having to duplicate the steps.
Optional stage jobs will now run in parallel with core jobs,
but they still remain optional in the sense that they don't fail
the build.
- A number of existing Github Action plugins are used to replace
functionality provided by Travis or other tools:
Node setup, caching, Codecov, publishing release artifacts.
- Linux builds are updated to build on Ubuntu 18.04.
MacOS builds are updated to run on 10.15. Similar to the
Travis Xenial upgrade attempt, some changes are required due
to underlying platform and compiler upgrades. This means some
Node 10 toolchains will no longer be supported.
Whilst there is opportunity to upgrade some dependencies and
make the CI steps more idiomatic, I've left this for future changes
and just focussed on functional replication.
