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

max_walk_time overwrites max_trip_duration in travel_time_matrix when mode = "WALK" #353

Closed
1 task
pachecotaina opened this issue Oct 16, 2023 · 12 comments
Closed
1 task

Comments

@pachecotaina
Copy link

pachecotaina commented Oct 16, 2023

Hi,

I am working with travel times for Prague (CZ) and I It looks like the max_walk_time is overwriting max_trip_duration when mode is only WALK.

The package documentation says max_walk_time should only be applied when mode is TRANSIT.

  • max_walk_time An integer. The maximum walking time (in minutes) to access and egress the transit network, or to make transfers within the network (...).

Reproducible example here

library(r5r)

# build transport network
r5r_core <- setup_r5("USING ONLY OSM FROM GEOFRABIK")

# load origin/destination points
origins<- "SEE FILE ATTACHED"
destinations <- "SEE FILE ATTACHED"

# IF max_walk_time = 15 the result has 5,985 lines
# IF max_walk_time = max_trip_duration = 120L the result has 474,288 lines
ttm <- travel_time_matrix(
                      mode = "WALK",
                      output_dir = NULL,
                      r5r_core = r5r_core,
                      origins = origins,
                      destinations = destinations,
                      hour_departure = "07:30:00",
                      time_window = 10L,         # time window (minutes) 
                      percentiles = 50L,         # get median travel times
                      mode_egress = "WALK",
                      fare_structure = NULL,     # if we want to include travel cost
                      max_fare = Inf,            # max value that trips can cost
                      max_walk_time = 15, # 1km  # max time (minutes) to access and egress the transit network
                      max_bike_time = Inf,       # max time (minutes) to access and egress the transit network
                      max_car_time = Inf,        # max time (minutes) to access and egress the transit network
                      max_trip_duration = 120L,  # max trip duration (minutes) - 2 hours
                      walk_speed = 4,            # Average walk speed in km/h 
                      bike_speed = 16,           # Average bike speed in km/h 
                      max_rides = 3,             # max number of public transport rides allowed in the same trip
                      max_lts = 2,               # maximum level of traffic stress that cyclists will tolerate
                      draws_per_minute = 5L,
                      n_threads = Inf,
                      verbose = FALSE,
                      progress = FALSE)

Files:
CZ001F_pointspop1000.csv
OSM network from geofrabik.de

@rafapereirabr
Copy link
Member

@dhersz , could you please have a look at this ?

@dhersz
Copy link
Member

dhersz commented Oct 19, 2023

Hello @pachecotaina, thanks for opening the issue.

Indeed, this is in fact something that we have explicitly thought about and is what we consider expected behavior.

We can adjust the documentation if you think it is ambiguous.

In particular, what were you expecting with the function call you posted above? Walking trips of up to 120 minutes? If so, how do you think max_walk_time would affect the results in this case?

@pachecotaina
Copy link
Author

In my opinion the documentation it is ambiguous. When reading it my understanding is that max_walk_time only affects walking times to access and egress transit stops or stations, and to make transfers within the network.

An integer. The maximum walking time (in minutes) to access and egress the transit network, or to make transfers within the network. Defaults to no restrictions, as long as max_trip_duration is respected. The max time is considered separately for each leg (e.g. if you set max_walk_time to 15, you could potentially walk up to 15 minutes to reach transit, and up to another 15 minutes to reach the destination after leaving transit). Defaults to Inf, no limit.

@rafapereirabr
Copy link
Member

How about we rewrite the documentation like this (changed text in bold:

  • An integer. The maximum walking time (in minutes) to access and egress the transit network, or to make transfers within the network. The max time is considered separately for each leg (e.g. if you set max_walk_time to 15, you could potentially walk up to 15 minutes to reach transit, and up to another 15 minutes to reach the destination after leaving transit). Defaults to no restrictions up to the limit set in max_trip_duration.

And the function could throw and error if the user sets max_walk_time longer than max_trip_duration.

@pachecotaina
Copy link
Author

Not sure if this change reflects the way the function is working. I would go for something like:

  • An integer. The maximum walking time (in minutes) to access and egress the transit network, or to make transfers within the network. The max time is considered separately for each leg (e.g. if you set max_walk_time to 15, you could potentially walk up to 15 minutes to reach transit, and up to another 15 minutes to reach the destination after leaving transit). When mode is only "WALK" this parameter overwrites max_trip_duration, that is max_trip_duration will be the same as max_walk_time.

With this description there is no need to change the function.

@rafapereirabr
Copy link
Member

@dhersz , what do you think?

@dhersz
Copy link
Member

dhersz commented Nov 21, 2023

My proposal:

  • An integer. The maximum walking time (in minutes) to access and egress the transit network, to make transfers within the network or to complete walk-only trips. Defaults to no restrictions (numeric value of Inf), as long as max_trip_duration is respected. When routing transit trips, the max time is considered separately for each leg (e.g. if you set max_walk_time to 15, you could get trips with an up to 15 minutes walk leg to reach transit and another up to 15 minutes walk leg to reach the destination after leaving transit).

Basically tried to make it clear that the parameter also affects the output when considering walk-only trips and that the restrictions of walking to/from the transit network only affects transit trips. What do you think @pachecotaina?

@rafapereirabr
Copy link
Member

@dhersz , I like your text. I would just add one sentence at the end to emphasize that "In walk-only trips, the max_walk_time is overwritten by max_trip_duration".

@dhersz
Copy link
Member

dhersz commented Nov 21, 2023

Not necessarily. If max_trip_duration > max_walk_time, then max_walk_time still prevails. The stricter restriction applies.

@pachecotaina
Copy link
Author

pachecotaina commented Nov 21, 2023 via email

@rafapereirabr
Copy link
Member

"In walk-only trips, the max_walk_time is overwritten by max_trip_duration".

In this case, the last sentence should read:

  • "In walk-only trips, whenever max_walk_time differs from max_trip_duration, the lowest value is considered."
    Better now?

@dhersz , I assume this is the same behavior for max_bike_time, so we should probably update this documentation as well. correct?

@rafapereirabr
Copy link
Member

ok, the final version of the documenation is this one below. I've implemented the same change to max_bike_time. This will be updated in the dev version in my next commit and available in the next r5r release. Thanks @pachecotaina and @dhersz .

#' @param max_walk_time An integer. The maximum walking time (in minutes) to
#'   access and egress the transit network, to make transfers within the network
#'   or to complete walk-only trips. Defaults to no restrictions (numeric value
#'   of `Inf`), as long as `max_trip_duration` is respected. When routing
#'   transit trips, the max time is considered separately for each leg (e.g. if
#'   you set `max_walk_time` to 15, you could get trips with an up to 15 minutes
#'   walk leg to reach transit and another up to 15 minutes walk leg to reach
#'   the destination after leaving transit. In walk-only trips, whenever
#'   `max_walk_time` differs from `max_trip_duration`, the lowest value is
#'   considered.
#' @param max_bike_time An integer. The maximum cycling time (in minutes) to
#'   access and egress the transit network, to make transfers within the network
#'   or to complete bicycle-only trips. Defaults to no restrictions (numeric
#'   value of `Inf`), as long as `max_trip_duration` is respected. When routing
#'   transit trips, the max time is considered separately for each leg (e.g. if
#'   you set `max_bike_time` to 15, you could get trips with an up to 15 minutes
#'   cycle leg to reach transit and another up to 15 minutes cycle leg to reach
#'   the destination after leaving transit. In bicycle-only trips, whenever
#'   `max_bike_time` differs from `max_trip_duration`, the lowest value is
#'   considered.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants