Closed AlexandraKapp closed 3 years ago
Same comment as for #33: Could you please merge current master branch with your PR? This time i fear the conflicts may not be so trivial - i hope it's not too difficult for you to reconcile your PR with the changes I've made in the meantime. Feel free to ask any for any advice or suggestions here. Thanks!
EDIT: Once these 2 PRs have been merged, new version will go straight to CRAN. Also check out bottom of the current README
, with newly added "allcontributors" section :smile:
As I was looking into it to fix the arrival time. The isotrips
(debugging at line 76 in isochrone.R
)
https://github.com/ATFutures/gtfs-router/blob/16997c7ba14a4691c3fb3018ff3ab271639e17a6/R/isochrone.R#L76
are now not necessarily starting with the start location ("Schonlein") anymore (as they did before), but instead also start with the next stop "Kottbusser Tor". Is this intended behavior?
Yeah, that's because of the creation of transfers.txt
, which enables walking from nominated start station to additional, nearby stations. I guess it would be good to be able to switch that off, so isochrones could be generated only from specified stations, right? That's very easy - i'll open a new issue to do that.
Ah I see! I think it makes sense to include the transfers as start points - maybe rather include the walking time (min_transfer_time) in order to get a more realistic result?
I'm still not sure though, if Schönleinstr and Kottbusser Tor are transfers? In the timetable that I used to get the above screenshot they are not linked as transfers (also, they are 650m appart) (Maybe I'm missing sth?)
This is what I use for debugging: https://github.com/ATFutures/gtfs-router/blob/16997c7ba14a4691c3fb3018ff3ab271639e17a6/tests/testthat/test-isochrone.R#L10-L19
Whoops, i accidentally closed that via a commit - sorry!! But that commit nevertheless fixed the faulty behaviour you reported. I'll re-open for you to confirm, and hopefully we can merge asap. Thanks for bearing with me here!
It does solve the faulty behavior - but I think it brings back the issue from #38
It does solve the faulty behavior - but I think it brings back the issue from #38
yeah, i was afraid of that - working on solving both now ...
should all be good now!
still having the same issue with the new timetable 🤔
I don't see any issues on master branch using the VBB feed you sent me:
library(gtfsrouter)
gtfs <- extract_gtfs ("VBB_gtfs.zip")
#> â–¶ Unzipping GTFS archive
#> âś” Unzipped GTFS archive
#> â–¶ Extracting GTFS feedâś” Extracted GTFS feed
#> â–¶ Converting stop times to secondsâś” Converted stop times to seconds
#> â–¶ Converting transfer times to secondsâś” Converted transfer times to seconds
gtfs <- gtfs_timetable (gtfs, day = 3)
start_time <- 12 * 3600 + 1200
end_time <- start_time + 1200
ic <- gtfs_isochrone (gtfs,
from = "Schönlein",
start_time = start_time,
end_time = end_time)
#> Loading required namespace: geodist
#> Loading required namespace: lwgeom
#> Registered S3 method overwritten by 'spatstat':
#> method from
#> print.boxx cli
#> Linking to GEOS 3.8.0, GDAL 3.0.4, PROJ 6.3.1
ic$start_point
#> Simple feature collection with 1 feature and 2 fields
#> geometry type: POINT
#> dimension: XY
#> bbox: xmin: 13.42224 ymin: 52.49318 xmax: 13.42224 ymax: 52.49318
#> geographic CRS: WGS 84
#> stop_name stop_id geometry
#> 1 U Schönleinstr. (Berlin) 070201084102 POINT (13.42224 52.49318)
nrow (ic$mid_points)
#> [1] 461
Created on 2020-08-17 by the reprex package (v0.3.0)
Nor on your current PR, which gives this:
setwd ("/data/mega/code/repos/atfutures/gtfs-router")
devtools::load_all (".", export_all = FALSE)
#> Loading gtfsrouter
gtfs <- extract_gtfs ("VBB_gtfs.zip")
#> â–¶ Unzipping GTFS archive
#> âś” Unzipped GTFS archive
#> â–¶ Extracting GTFS feedâś” Extracted GTFS feed
#> â–¶ Converting stop times to secondsâś” Converted stop times to seconds
#> â–¶ Converting transfer times to secondsâś” Converted transfer times to seconds
gtfs <- gtfs_timetable (gtfs, day = 3)
start_time <- 12 * 3600 + 1200
end_time <- start_time + 1200
ic <- gtfs_isochrone (gtfs,
from = "Schönlein",
start_time = start_time,
end_time = end_time)
#> Loading required namespace: geodist
#> Loading required namespace: lwgeom
#> Registered S3 method overwritten by 'spatstat':
#> method from
#> print.boxx cli
#> Linking to GEOS 3.8.0, GDAL 3.0.4, PROJ 6.3.1
ic$start_point
#> Simple feature collection with 1 feature and 2 fields
#> geometry type: POINT
#> dimension: XY
#> bbox: xmin: 13.42224 ymin: 52.49318 xmax: 13.42224 ymax: 52.49318
#> geographic CRS: WGS 84
#> stop_name stop_id geometry
#> 1 U Schönleinstr. (Berlin) 070201084102 POINT (13.42224 52.49318)
nrow (ic$mid_points)
#> [1] 461
ic$mid_points
#> Simple feature collection with 461 features and 3 fields
#> geometry type: POINT
#> dimension: XY
#> bbox: xmin: 13.34024 ymin: 52.42303 xmax: 13.4895 ymax: 52.55226
#> geographic CRS: WGS 84
#> First 10 features:
#> stop_name stop_id earliest_arrival
#> 1 U Kottbusser Tor (Berlin) 070201084002 44760
#> 2 U Moritzplatz (Berlin) 070201083902 44880
#> 3 U Heinrich-Heine-Str. (Berlin) 070201083802 44970
#> 4 U Heinrich-Heine-Str. (Berlin) 070101005163 44970
#> 5 U Märkisches Museum (Berlin) 070101005178 45480
#> 6 U Kottbusser Tor (Berlin) 070201084002 44760
#> 7 U Kottbusser Tor (Berlin) 070201012401 44760
#> 8 U Prinzenstr. (Berlin) 070201012501 45060
#> 9 U Hallesches Tor (Berlin) 070201012601 45180
#> 10 U Möckernbrücke (Berlin) 070201012701 45270
#> geometry
#> 1 POINT (13.41775 52.49905)
#> 2 POINT (13.41095 52.50374)
#> 3 POINT (13.41617 52.51086)
#> 4 POINT (13.41617 52.51086)
#> 5 POINT (13.40877 52.51201)
#> 6 POINT (13.41775 52.49905)
#> 7 POINT (13.41775 52.49905)
#> 8 POINT (13.40653 52.49827)
#> 9 POINT (13.39176 52.49777)
#> 10 POINT (13.38326 52.49894)
Created on 2020-08-17 by the reprex package (v0.3.0)
Based on that, I'm happy to merge as is, and we can deal with any subsequent issues if and when they may arise.
I face the issue in connection with the customly created transfers:
gtfs <- extract_gtfs("VBB_gtfs.zip")
gtfs$transfers <- gtfsrouter::gtfs_transfer_table (gtfs, network_times = FALSE)
tt <- gtfs_timetable(gtfs, day = "Monday")
i <- gtfs_isochrone(tt, "Boxhagener Platz", start_time = 8*3600 + 5*60, end_time = 8*3600 + 10*60)
i$mid_points %>% nrow # returns 122233
But as this PR is not actually related to this issue, we dont need to handle it at this point
Here's the output I get with custom transfers.txt
, which confirms that #38 is fixed at least for me. This output is from your current PR, with same output generated by current master
(without the additional earliest_arrival
column on mid_points
). Please re-open and comment further in #38 after this merge if you're still seeing that issue.
library(gtfsrouter)
gtfs <- extract_gtfs ("./VBB_gtfs.zip")
#> â–¶ Unzipping GTFS archive
#> âś” Unzipped GTFS archive
#> â–¶ Extracting GTFS feedâś” Extracted GTFS feed
#> â–¶ Converting stop times to secondsâś” Converted stop times to seconds
#> â–¶ Converting transfer times to secondsâś” Converted transfer times to seconds
#transfers <- gtfs_transfer_table (gtfs, network_times = FALSE)
#saveRDS (transfers, "transfers.Rds")
transfers <- readRDS ("transfers.Rds")
gtfs$transfers <- transfers
gtfs <- gtfs_timetable (gtfs, day = 3)
start_time <- 12 * 3600 + 1200
end_time <- start_time + 1200
ic <- gtfs_isochrone (gtfs,
from = "Schönlein",
start_time = start_time,
end_time = end_time)
#> Loading required namespace: geodist
#> Loading required namespace: lwgeom
#> Registered S3 method overwritten by 'spatstat':
#> method from
#> print.boxx cli
#> Linking to GEOS 3.8.0, GDAL 3.0.4, PROJ 6.3.1
ic$start_point
#> Simple feature collection with 1 feature and 2 fields
#> geometry type: POINT
#> dimension: XY
#> bbox: xmin: 13.42224 ymin: 52.49318 xmax: 13.42224 ymax: 52.49318
#> geographic CRS: WGS 84
#> stop_name stop_id geometry
#> 1 U Schönleinstr. (Berlin) 070201084102 POINT (13.42224 52.49318)
nrow (ic$mid_points)
#> [1] 679
ic$mid_points
#> Simple feature collection with 679 features and 3 fields
#> geometry type: POINT
#> dimension: XY
#> bbox: xmin: 13.34024 ymin: 52.42303 xmax: 13.4895 ymax: 52.55226
#> geographic CRS: WGS 84
#> First 10 features:
#> stop_name stop_id earliest_arrival
#> 1 U Kottbusser Tor (Berlin) 070201084002 44760
#> 2 U Kottbusser Tor (Berlin) 070201012401 44760
#> 3 U Prinzenstr. (Berlin) 070201012501 45060
#> 4 U Hallesches Tor (Berlin) 070201012601 45180
#> 5 U Möckernbrücke (Berlin) 070201012701 45270
#> 6 U Gleisdreieck (Berlin) 070201012801 45360
#> 7 U KurfĂĽrstenstr. (Berlin) 070201012901 45480
#> 8 U Nollendorfplatz (Berlin) 070201013001 45570
#> 9 U Wittenbergplatz (Berlin) 070201013101 45690
#> 10 U Hermannplatz (Berlin) 070201084201 44610
#> geometry
#> 1 POINT (13.41775 52.49905)
#> 2 POINT (13.41775 52.49905)
#> 3 POINT (13.40653 52.49827)
#> 4 POINT (13.39176 52.49777)
#> 5 POINT (13.38326 52.49894)
#> 6 POINT (13.37429 52.49959)
#> 7 POINT (13.36286 52.49978)
#> 8 POINT (13.35383 52.49964)
#> 9 POINT (13.34256 52.50211)
#> 10 POINT (13.42472 52.48696)
Created on 2020-08-17 by the reprex package (v0.3.0)
Finally found some time to get into the C++ Code. Hope this is what you had in mind with you're suggested solution @mpadge
Fixes #30