-
Notifications
You must be signed in to change notification settings - Fork 2
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
G-MAP mobility profile-based routing [Review only - DO NOT MERGE] #197
base: ibi-dev-2.x
Are you sure you want to change the base?
Conversation
src/main/java/org/opentripplanner/apis/gtfs/mapping/RouteRequestMapper.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opentripplanner/graph_builder/module/osm/OsmModule.java
Outdated
Show resolved
Hide resolved
@@ -85,6 +88,8 @@ public class StreetEdge | |||
*/ | |||
private float bicycleSafetyFactor; | |||
|
|||
public Map<MobilityProfile, Float> profileCost = new HashMap<>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you notice that this causes a noticeable slowdown you can also create an array of and the use MobilityProfile.ordinal
to access the values. This also saves some memory as arrays are extremely runtime- and space-efficient.
However, my gut feeling is that performance is not a huge priority so it's probably good enough as it is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Won't do at this time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@leonardehrenfried SonarLint suggests using EnumMap
instead of HashMap
, however, using EnumMap
makes OTP unable to deserialize the graph. Have you seen this issue elsewhere?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
EnumMap is indeed faster than a HashMap. EnumMap also says that it is serializable.
In OTP we do use enum map sometimes but I cannot find an example where it is actually store in the graph file.
Can you post the stacktrace please?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need to post the stacktrace, I tried it out myself.
Serializing an enum map is indeed tricky as the default serialization uses JDK internals. However, if you want to do it anyway, here is a commit that shows you how to do it: 797d727
Of course you need to swap out the TransitMode
with the enum that you want to serialize.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, that looks useful.
This branch seems to fail the graph build when no profile data is configured. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've left a few comments but none of them seem very urgent to me, particularly because it's not intended to be merged. If this branch works for you then this is good to go.
…ng of CSV parsing outcome
…side of same street
Summary
This PR adds support for mobility profile-based routing for the ITS4US G-MAP project, in which Arcadis IBI participates. A set of mobility profiles are predefined. For each mobility profile, special weights/costs are added, at graph build time, from a CSV file, to certain paths in the OSM street network. The CSV file and the predefined mobility profiles are sourced from a third-party proprietary server that provides costs for each mobility profile for select OSM paths. Profile-based routing is performed based on a new parameter in the
plan
graphQL endpoint.Issue
None
Unit tests
TBA
Documentation
TBA
Changelog
N/A
Bumping the serialization version id
TBA