N-nearest offsets#1496
Conversation
traines-source
left a comment
traines-source
left a comment
There was a problem hiding this comment.
I think the general approach makes sense, but there are some things I noticed
This is a niche feature that some people wanted where we skip the OSR calculation entirely and just use beeline distances to discover nearby locations. Maybe it's not worth the effort to change this, although I think this could also benefit from the n-nearest feature. |
So I still dont get why we return locations that have no routes in And either way I think we could merge |
* add all paths in initial max walking distance * decrease radius expansion
I don't think there is a good reason other than that in most cases it probably doesn't really make a difference performance wise or otherwise.
They are not really returning the same things (offsets vs location)? But feel free to refactor if you see fit. |
|
btw, this is the notorious repertoire filter idea: motis-project/nigiri@master...mority:nigiri:repertoire-filter |
felixguendling
left a comment
felixguendling
left a comment
There was a problem hiding this comment.
implementation doesn't feel clean to me
it seems to also not guarantee the same behavior as before if the parameter is not set?
not sure if inside get_offsets is the right place to implement this
| double const distance, | ||
| nigiri::location_idx_t const not_equal_to = | ||
| nigiri::location_idx_t::invalid()); | ||
|
|
| get_stops_with_traffic(*r.tt_, rtt, *r.loc_tree_, pos, max_dist); | ||
| constexpr unsigned const kBeelineNearStationsFactor = 4U; | ||
|
|
||
| auto const max_beeline_seconds = std::max( |
There was a problem hiding this comment.
max? this will go beyond street_routing_max_near_stops_seconds_
shouldn't we throw 400 so the user knows that parameters were bad?
| get_stops_with_traffic(*r.tt_, rtt, *r.loc_tree_, pos, expanded_dist); | ||
| while (expanded_time * 2 <= max_beeline_seconds && | ||
| near_stops.size() < min_near_stations * kBeelineNearStationsFactor) { | ||
|
|
There was a problem hiding this comment.
no new line loop start
| default: 900 | ||
| minimum: 0 | ||
|
|
||
| - name: minNearStations |
There was a problem hiding this comment.
inconsistent - sometimes stops, sometimes stations
-> always stops
| - name: minNearStations | ||
| in: query | ||
| required: false | ||
| description: | |
There was a problem hiding this comment.
description doesn't help me to understand what's actually happening
| gbfs::gbfs_routing_data& gbfs_rd, | ||
| stats_map_t& stats) { | ||
| stats_map_t& stats, | ||
| unsigned const min_near_stations) { |
There was a problem hiding this comment.
keep stats last, add close to max param
| #include "motis/get_stops_with_traffic.h" | ||
|
|
||
| #include "osr/location.h" | ||
| #include "utl/helpers/algorithm.h" |
4 participants
Change routing behaviour to always find a mimium number of start/end stations or transfer options.
Closes #1477