Repository navigation
Ignore custom cost fields in CAR routing - #972
Conversation
This ignores custom costs (e.g. from elevation data) for the CAR street mode. Custom speeds for cars are handled separately.
abyrd
left a comment
There was a problem hiding this comment.
This change assumes all CostField instances apply to all modes except cars. CostFields were intended to be generic, and could in principle apply costs to walking, bicycling, or driving. Our assumption was that users would know which modes they intended their costs to apply to, and would only employ such graphs for the intended modes.
In practice, the existing CostField implementations (elevation and sun exposure) do not apply to driving, so this change does provide an immediate improvement to car results.
However, it gives a false impression that mode is taken into account by CostFields, and bakes the undocumented assumption about which modes CostFields apply to into traversal logic. We should clearly document this assumption and the fact that it is an unrealistic simplification, as this has the potential to cause confusion in future development.
Although sun and hill costs arguably apply to both walking and biking, these costs would likely be different for biking. CostFields do not receive any information about what mode of transport is in use, so they cannot modify their behavior per mode. Implicitly, a CostField only applies to a single mode of transport (rather than "all modes but car").
Ideally CostFields would know which modes they applied to, and the list of CostFields would be filtered before the search happens. The logic currently in the StreetRouter constructor could be factored into a mode-aware factory method that would yield the correct TraversalTimeCalculator.
We don't need to implement this now, but should document it in the Javadoc of the relevant classes. I will propose some simple Javadoc changes.
|
StreetRouter is also explicitly single-use and can only have one StreetMode, so that mode could be specified up front and the TraversalTimeCalculator chosen accordingly (avoiding the MultistageTraversalTimeCalculator construct entirely when no CostFields apply). |
abyrd
left a comment
There was a problem hiding this comment.
Approved (added two commits with Javadoc and line comments)
This ignores custom costs (e.g. from elevation data) for the CAR street mode. Custom speeds for cars are handled separately.
Fixes #970