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

feat(trip-explorer): speed for station pairs #909

Merged
merged 25 commits into from
Jan 15, 2024

Conversation

rudiejd
Copy link
Contributor

@rudiejd rudiejd commented Dec 17, 2023

Motivation

Closes #803

Changes

  • Add speed graph on trip explorer

image

Testing Instructions

  • Navigate to trip explorer page for all lines
  • Verify that the new speed graph is populated correctly

@github-actions github-actions bot added dependencies Changes to dependencies backend Change to backend code frontend Change to frontend code labels Dec 17, 2023
Copy link
Member

@devinmatte devinmatte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is awesome! A few initial thoughts in no particular order:

We should display both of the two station pairs since this is between two
Screenshot 2023-12-17 at 7 38 49 PM
Screenshot 2023-12-17 at 7 38 52 PM

We should add the same aggregate numbers below the chart like we do in travel times
Screenshot 2023-12-17 at 7 38 40 PM

Personally I think speed should go between headways and dwells in the chart order

I was able to get the page to bug out with http://localhost:3000/red/trips/single/?from=70073&to=70093&date=2023-12-17 so there's probably some missing stops in the dataset?

Overall, this is very cool, just needs some polish. We can try and get this into beta before the next meeting to get people in labs to poke around with it

@rudiejd
Copy link
Contributor Author

rudiejd commented Dec 18, 2023

thanks for taking a look!! yep, this is definitely still a WIP - just threw it together yesterday/today. Those are all good. There were a couple other things I wanted to take a look at too, let me know your thoughts:

  • some of the speeds seem a bit suspect, for example average of 20mph some hours for davis to quincy center. I wonder if I should be including the dwells in addition to travel_time_sec
  • there is no monthly/weekly aggregated version of this chart
  • there is no benchmark. I could calculate a benchmark using the benchmark travel time.
  • the distance calculations from traversing that list are right, but unfortunately the dataset includes some bizarre distances - for example, JFK / UMass (Braintree) => Ashmont (Ashmont) was listed as 20 miles. I haven't debugged this super deeply yet

@devinmatte
Copy link
Member

there is no monthly/weekly aggregated version of this chart

This certainly isn't necessary for a first pass. We can start with a single day chart only

there is no benchmark. I could calculate a benchmark using the benchmark travel time.

there's no benchmark for dwells, so it's not critical

some of the speeds seem a bit suspect, for example average of 20mph some hours for davis to quincy center. I wonder if I should be including the dwells in addition to travel_time_sec

Yeah maybe @PatrickCleary or @idreyn could help you track down the accuracy of these since they worked on the line-wide speed numbers. I can also poke around after the holiday

@rudiejd
Copy link
Contributor Author

rudiejd commented Dec 18, 2023

I was able to get the page to bug out with http://localhost:3000/red/trips/single/?from=70073&to=70093&date=2023-12-17 so there's probably some missing stops in the dataset?

I poked around a bit on this. It seems that 70073 is Charles MGH on the Alewife - Braintree route and 70093 is Ashmont on the Alewife - Ashmont route. By this dataset, that is unreachable since you can't access 70093 on the same route. I originally used stop_id for the distance calculation since it was readily available on the FE, but it seems like it might be better to use station_id for the distances based on this quirk 😅

@rudiejd rudiejd force-pushed the feat/speed-for-station-pairs branch from 5ddd6c5 to b698481 Compare December 19, 2023 02:34
@rudiejd rudiejd changed the title feat: speed for station pairs (WIP) feat: speed for station pairs Dec 19, 2023
@rudiejd
Copy link
Contributor Author

rudiejd commented Dec 19, 2023

I changed this to use stations instead of stops, fixing the Charles => Ashmont issue, fixed the distance calculation by only traversing in one direction at a time, added a legend. Should be ready for review - I updated the screenshot

I also added a benchmark speed calculation. I can show it on the graph if that's something we want, but I think it might be a little confusing considering it'll be the inverse of the travel time benchmark

@rudiejd rudiejd force-pushed the feat/speed-for-station-pairs branch from bd5ca30 to 0351ef4 Compare December 21, 2023 01:10
@devinmatte
Copy link
Member

Damn seeing full speeds for tracks that are repaired is wild
Screenshot 2023-12-20 at 8 25 18 PM

I do think this will be more interesting in aggregate (multi-day) datasets to see improvement/decline than it will be for most single day views (since it really is just another way to describe travel times) but this is quite interesting to see when a track is fixed.

@rudiejd
Copy link
Contributor Author

rudiejd commented Dec 23, 2023

image
Added aggregate data.

One small issue is that some of the green line station distances e.g. Ball Square => Back of the Hill are still undefined, and the aggregate charts don't accept undefined data, so you end up with these weird graphs of 0 speed. I'll

@PatrickCleary
Copy link
Member

The green line station distances we use are just measured on Google Maps. So if you're interested in replacing some of those null values, you could add them that way!

@rudiejd
Copy link
Contributor Author

rudiejd commented Dec 23, 2023

@PatrickCleary I ended up figuring out the problem with the data - I was relying on the route-id field being the line name, but it was not Green for all of the green, but rather Green-A, Green-B, etc. but thank you! Green line looks to be fixed now
image

I refactored the script for calculating station distances and moved it out to its own file, because the logic was getting pretty complicated. It is still messy and can be refactored further, but I didn't want to invest too much time for potentially a throwaway script

Copy link
Member

@devinmatte devinmatte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some initial comments

@austinjpaul
Copy link
Contributor

Mentioned in the meeting, just writing here for not forgetting - I think my preference would be to add speed as a toggle for the travel-times chart, since it is the same data displayed two different ways, just scaled by a factor. That way we don't end up with too many charts on page.

@rudiejd
Copy link
Contributor Author

rudiejd commented Jan 12, 2024

@austinjpaul totally agree! I'll rework it as a toggle on the travel times graph

@rudiejd rudiejd force-pushed the feat/speed-for-station-pairs branch from 07a7ee8 to fea4954 Compare January 14, 2024 22:42
@github-actions github-actions bot removed the dependencies Changes to dependencies label Jan 14, 2024
@rudiejd rudiejd changed the title feat: speed for station pairs feat(trip-explorer): speed for station pairs Jan 15, 2024
@rudiejd rudiejd merged commit c10290c into transitmatters:main Jan 15, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend Change to backend code frontend Change to frontend code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Speed for station pairs
6 participants