Delay highlighting #363

NoSuchThingAsRandom · 2020-10-07T11:00:06Z

Initial build for issue 170
Now highlights intersections and lanes for cars, bikes and walking. That have spent over the threshold (30s) time waiting.
This is highlighted on the map, and colour coordinated for the severity of the delay
In addition it is also shown as hotspots on the timeline bar, with more information available when hovered upon.

Also contains some "fixes" for Canocilsing imports @dabreegster
Including:

sim/src/lib.rs
pub use crate::render
geom/src/spped
import trim_f64 in wrong place
widgetry/src/text.rs
use crate assets in wrong place

Bike also works, but is treated as a car, so the avg speed is flagged as very slow (Needs fix)

Now uses min of (speed limit and vehicle max speed) for the maximum speed check Added trip blocked time query Lots of work on tool tips Icons are broke

Color assertion?

Added new custom TextSpan size function

# Conflicts: # game/src/colors.rs # game/src/info/trip.rs # geom/src/speed.rs # sim/src/analytics.rs # sim/src/events.rs # sim/src/lib.rs # sim/src/mechanics/driving.rs # sim/src/mechanics/walking.rs # sim/src/sim/queries.rs # sim/src/trips.rs # widgetry/src/text.rs

Doesn't quite match the existing "changes"

dabreegster

This is an awesome feature! I left lots of style comments; if you're pressed for time, feel free to focus on the other ones, and I can take care of the other things later.

dabreegster · 2020-10-07T17:53:36Z

game/src/info/trip.rs

@@ -2,24 +2,25 @@ use std::collections::BTreeMap;

 use maplit::btreemap;

-use geom::{ArrowCap, Distance, Duration, Percent, PolyLine, Polygon, Pt2D, Time};
-use map_model::{Map, Path, PathStep};
+use geom::{ArrowCap, Distance, Duration, Percent, Polygon, PolyLine, Pt2D, Time};


Hmm, the diff here reorders Polygon and PolyLine. cargo +nightly fmt undoes this. I'll poke around for an option to make it act like CLion, but we might be stuck cleaning this up from time to time.

Yeah, it's a tad annoying, and personally I prefer Polygon first, because it's alphabetical. But I'm not sure why cargo +nightly fmt does this?

I think cargo fmt considers PolyLine as two words, split at the camel — So "Poly Line" comes before "Polygon"

The split makes sense. I'm less opinionated about what this looks like and more interested in automating consistency (thanks to a few years working with gofmt). For now, I vote we continue to apply cargo +nightly fmt. If we can find an option or other CLI tool to align more with CLion, that's fine too as long as it's not hard for everyone to run

game/src/info/trip.rs

dabreegster · 2020-10-07T18:38:54Z

sim/src/mechanics/driving.rs

@@ -248,7 +248,25 @@ impl DrivingSimState {
        transit: &mut TransitSimState,
    ) -> bool {
        match car.state {
-            CarState::Crossing(_, _) => {
+            CarState::Crossing(time_int, dist_int) => {


Hmm, this is tricky. Crossing to Queued happens before the car reaches the end of the lane; they might even still be moving, stuck behind a slower vehicle. https://dabreegster.github.io/abstreet/trafficsim/discrete_event.html#cars gives more detail about this.

The delay at an intersection starts being counted here, at car.state = CarState::Queued { blocked_since: now }. Maybe we can just show delays at intersections and leave it a bit unspecified why the delay happened -- because of congestion upstream in the target lane, because of slow signal timing, because of a long queue of vehicles, because of a single slow vehicle in front of faster ones, etc.

But for this PR, I think it's fine to keep measuring it this way; we'll iterate on it next.

dabreegster · 2020-10-07T18:40:24Z

sim/src/mechanics/driving.rs

+                        .max_speed
+                        .unwrap_or(Speed::meters_per_second(100.0)),
+                );
+                let speed_percent: u8 = ((avg_speed / max_speed) * 100.0) as u8;


We should do this calculation in analytics.rs instead. Here, just fire an Event::AgentCrossedLane(trip, lane, avg_speed, max_speed) or something. The purpose of Event is to decouple the simulation details here from whatever choices we make later in Analytics about how to group/filter/process for data analysis/visualization.

dabreegster · 2020-10-07T18:41:02Z

sim/src/mechanics/driving.rs

@@ -357,6 +375,14 @@ impl DrivingSimState {
                        // Don't schedule a retry here.
                        return false;
                    }
+                    if let Some((trip, _)) = car.trip_and_person {
+                        let delay = (now - blocked_since).inner_seconds() as u8;
+                        // Maybe alter the minimum delay time to be recorded


Same, the event should always be sent with the Duration, and the filtering + transformation to u8 can happen later.

widgetry/src/text.rs

game/src/info/trip.rs

sim/src/mechanics/driving.rs

michaelkirk

Hey, nice work! I pulled it down to give it a go, but I'm not yet familiar enough with the codebase to immediately understand how to see this feature in action. Can you provide instructions on how to access it?

One thing I did notice, was some undesirable spacing between the trip timeline and the mode indicator which seems new to this branch:

before (from master):

after (as of 6c54cad):

To clarify - this is the trip info panel that you see after clicking on an agent.

game/src/info/trip.rs

sim/src/analytics.rs

dabreegster · 2020-10-08T18:46:56Z

game/src/info/trip.rs

+        let phase_width = box_width * percent_duration;
+        if idx == phases.len() - 1 {
+            if let Some(p) = progress_along_path {
+                icons.push(Widget::draw_batch(


Build break, I think this has to be icons_geom now?

Actually, I think just declaring let mut icons = Vec::new() up by the others should fix it. icons_geom is for the tooltips.

dabreegster · 2020-10-08T18:57:39Z

I regenerated the prebaked analytics data. montlake stayed the same size, and lakeslice increased from 61MB to 64MB -- that's a totally fine amount, nice! After we merge, I'll immediately followup with the uploaded data.

This pretty much LGTM, minus the build break. And if you can, run cargo +nightly fmt too, to deal with the slight import order difference. Next, I'm going to attempt some refactoring of info/trip.rs next to consolidate tooltips and all the different cases for trips (future, ongoing, cancelled, finished).

dabreegster · 2020-10-10T19:22:49Z

Thanks for the PR! Going to merge this now, and follow up with the new prebaked data, some cleanups, and a fix for the timeline indicator icon.

- cargo +nightly fmt - upload new prebaked data

Sam added 16 commits September 27, 2020 17:48

Initial storing of intersection delays

81bd040

Initial highlighting of delays on route

ed367d4

Added label of delay

25c098b

Merge remote-tracking branch 'upstream/master' into delay_along_route

b17dac2

Info overlay works

3717e5e

Added supoprt for pedestrian delays (only intersection delay currently)

fc0df6e

Bike also works, but is treated as a car, so the avg speed is flagged as very slow (Needs fix)

Merge branch 'master' into delay_highlighting

a6f6ceb

Capitalised describe trip phase

16bc362

Now uses min of (speed limit and vehicle max speed) for the maximum speed check Added trip blocked time query Lots of work on tool tips Icons are broke

Nearly complete, just fmting and a crash to fix

df85964

Color assertion?

Fixed text font colour crash

a84be68

Added new custom TextSpan size function

Cargo +nightly fmt

e7be3ba

Cargo +nightly fmt

75f37cd

Clion auto import formatting

f804d9b

Doesn't quite match the existing "changes"

Fixing merge issues

556f6d6

Fixing icon placement issues

14bb85e

dabreegster reviewed Oct 7, 2020

View reviewed changes

sim/src/mechanics/driving.rs Outdated Show resolved Hide resolved

Requested changes (Part 1)

6c54cad

michaelkirk reviewed Oct 8, 2020

View reviewed changes

game/src/info/trip.rs Outdated Show resolved Hide resolved

dabreegster reviewed Oct 8, 2020

View reviewed changes

game/src/info/trip.rs Outdated Show resolved Hide resolved

dabreegster reviewed Oct 8, 2020

View reviewed changes

game/src/info/trip.rs Outdated Show resolved Hide resolved

dabreegster reviewed Oct 8, 2020

View reviewed changes

sim/src/analytics.rs Outdated Show resolved Hide resolved

Requested changes (Part 2)

b4ad546

dabreegster reviewed Oct 8, 2020

View reviewed changes

Merge branch 'master' into delay_highlighting

3cff938

dabreegster merged commit 4c5a858 into a-b-street:master Oct 10, 2020

dabreegster linked an issue Oct 10, 2020 that may be closed by this pull request

Show speed / delay along a route #170

Closed

dabreegster added a commit that referenced this pull request Oct 10, 2020

Cleanups after #363:

e388581

- cargo +nightly fmt - upload new prebaked data

dabreegster added a commit that referenced this pull request Oct 10, 2020

More cleanup of #363: fix the build break in info/trip

1791ef7

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Delay highlighting #363

Delay highlighting #363

NoSuchThingAsRandom commented Oct 7, 2020

dabreegster left a comment

dabreegster Oct 7, 2020

NoSuchThingAsRandom Oct 7, 2020

michaelkirk Oct 8, 2020

dabreegster Oct 8, 2020

dabreegster Oct 7, 2020

dabreegster Oct 7, 2020

dabreegster Oct 7, 2020

michaelkirk left a comment

dabreegster Oct 8, 2020

dabreegster Oct 8, 2020

dabreegster commented Oct 8, 2020

dabreegster commented Oct 10, 2020

Delay highlighting #363

Delay highlighting #363

Conversation

NoSuchThingAsRandom commented Oct 7, 2020

dabreegster left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michaelkirk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dabreegster commented Oct 8, 2020

dabreegster commented Oct 10, 2020