Start impact prediction mode #83

dabreegster · 2025-01-17T16:12:59Z

A start to #36. After some changes to one neighbourhood in Edinburgh:

The impact is mostly localized around there:

Justifying why Queen Street got busier with examples:

simple way.

many requests

dabreegster · 2025-01-17T16:13:46Z

backend/src/impact.rs

+
+use crate::{IntersectionID, MapModel, RoadID};
+
+// TODO Rename?


There's a few too many things called "impact" in the frontend. Maybe "area-wide traffic prediction"?

dabreegster · 2025-01-17T16:16:41Z

backend/src/impact.rs

+    }
+}
+
+/// Deterministically produce a bunch of OD pairs, just as a fallback when there's no real data


Not sure yet what approach to take to have real OD data only for some places, will tackle next.

Seems like a reasonable approach for expediency.

It might avoid trouble to regenerate trips where i1 == i2, but maybe it won't be an issue in practice.

dabreegster · 2025-01-17T16:19:41Z

backend/src/lib.rs

@@ -294,6 +295,31 @@ impl LTN {
        .map_err(err_to_js)?)
    }

+    /// Returns GJ with a LineString per road, with before/after counts


General reflection: It's unsatisfying to have 3 layers where types are expressed either as code or docs:

In the wasm.ts layer as TS

Here in the Rust WASM "interface" layer, which usually just returns JSON strings

In a more internal method on MapModel, where usually we could have more info on types. (If we used https://docs.rs/geojson/latest/geojson/ser/index.html structs, maybe)

It works fine so far, just one of the costs of a cross-language layer.

Yeah, I can see the frustration.

While writing rust, my instinct is that json (including geojson), should only be used as a serialization format to talk to wasm - in other words avoid ever doing application logic with geojson, and do everything with fully expressive rust types.

By that rubrik, this kind of thing is "bad": https://github.com/a-b-street/ltn/pull/83/files#diff-f71d03fb4aef16f1e3c21b14e2c5e077e0d40c8c1af7f897bba4a71f337040d6R87

In typescript I'm less sure - in theory we should be able to write typescript signatures compatible with geojson that also encapsulate our more specific needs (e.g. a LineString Feature that has a "foo: Number" foreign member). (Update: looks like you're already doing this part, great! Feature<LineString, { kind: "before" }>)

Given that I'm brand new to this code base, maybe such a principled stance isn't worth it in practice, so I wouldn't make any radical changes based just on my instincts yet, but consider it.

avoid ever doing application logic with geojson, and do everything with fully expressive rust types.

There's a bunch of other places in the code currently violating this, creating features and doing lots of set_property. The WASM layer in lib.rs is at least the one place to stringify JSON or parse inputs. It would indeed be nice to write structs using geojson::ser for most app-level code to output, especially if we could wire up something like https://docs.rs/ts-rs/latest/ts_rs/ to have more rigor about keeping wasm.ts in sync.

For the moment, I think the codebase is small/simple enough to keep playing it kind of loose, so I won't prioritize it yet.

dabreegster · 2025-01-17T16:20:44Z

backend/src/lib.rs

+    pub fn predict_impact(&mut self) -> Result<String, JsValue> {
+        self.map.rebuild_router(1.0);
+        let mut impact = self.map.impact.take().unwrap();
+        let out = impact.recalculate(&self.map);


impact is mutable, but also needs to access other parts of the map model, hence the Option and this "take, then put back" approach. Could be restructured to lift impact out of MapModel, but not too important.

dabreegster · 2025-01-17T16:21:24Z

backend/src/map_model.rs

@@ -33,11 +34,11 @@ pub struct MapModel {

    // TODO Wasteful, can share some
    // This is guaranteed to exist, only Option during MapModel::new internals
-    pub router_original: Option<Router>,
+    pub router_before: Option<Router>,


Renamed for consistency. "before" means before any user edits are applied. "after" means the current state of user edits.

dabreegster · 2025-01-17T16:24:05Z

backend/src/route.rs

@@ -14,6 +14,11 @@ pub struct Router {
    pub main_road_penalty: f64,
 }

+pub struct Route {


I did some detangling here to separately express a route as a sequence of steps and then render as a linestring. I keep reinventing variations of this code for the "OSM-based map model" layer in many projects, and in many others, I've recently consolidated on https://github.com/a-b-street/15m/tree/main/graph/src. It could be later worth trying to migrate this codebase to use that too (and take advantage of all its nice routing and route-rendering bits), but there are complications with doing so (how modal filters are modelled, partly) and it's not really a priority. The general tension is between duplicate code between projects and coming up with something general enough to handle a bunch of projects.

dabreegster · 2025-01-17T16:24:54Z

web/src/App.svelte

This is an example how to wire up a new UI mode

dabreegster · 2025-01-17T16:25:49Z

web/src/ImpactDetailMode.svelte

+      Pick a different road
+    </Link>
+
+    <div style="display: flex; justify-content: space-between;">


Some of this and above could be refactored with ViewShortcuts into some kind of list-navigation, maybe

dabreegster · 2025-01-17T16:28:47Z

web/src/PredictImpactMode.svelte

I'm not really happy with any of the styling choices here yet, but it's a start and roughly recreates what's in v1. There's two things we probably want to communicate:

Did a particular road get more or less traffic than before edits? What roads changed the most? A diverging red/green color is used for this.

What's the level of traffic after, in absolute terms?

A road going from 10 trips to 50 trips seems like a dramatic change by color, but 50 trips total is nothing, so it should be thin.

michaelkirk · 2025-01-17T19:47:10Z

backend/src/impact.rs

+                })
+                .as_object()
+                .unwrap()
+                .clone(),


minor nit: is this clone necessary - since it's the last thing in the method, I'd expect a move should be sufficient.

Sadly yes, as_object returns a ref, and there's no into_object helper

michaelkirk

Embarrassing question, but how do I get to this view?

michaelkirk · 2025-01-17T19:58:55Z

backend/src/lib.rs

@@ -294,6 +295,31 @@ impl LTN {
        .map_err(err_to_js)?)
    }

+    /// Returns GJ with a LineString per road, with before/after counts


Yeah, I can see the frustration.

While writing rust, my instinct is that json (including geojson), should only be used as a serialization format to talk to wasm - in other words avoid ever doing application logic with geojson, and do everything with fully expressive rust types.

By that rubrik, this kind of thing is "bad": https://github.com/a-b-street/ltn/pull/83/files#diff-f71d03fb4aef16f1e3c21b14e2c5e077e0d40c8c1af7f897bba4a71f337040d6R87

In typescript I'm less sure - in theory we should be able to write typescript signatures compatible with geojson that also encapsulate our more specific needs (e.g. a LineString Feature that has a "foo: Number" foreign member). (Update: looks like you're already doing this part, great! Feature<LineString, { kind: "before" }>)

Given that I'm brand new to this code base, maybe such a principled stance isn't worth it in practice, so I wouldn't make any radical changes based just on my instincts yet, but consider it.

michaelkirk · 2025-01-17T20:16:24Z

backend/src/route.rs

+        LineString::new(pts)
+    }
+
+    pub fn crosses_road(&self, road: RoadID) -> bool {


To make sure I'm understanding our terminology: a RoadID corresponds to a segment of the road network which has no intersections in its interior. Is that right?

So there's no way to "cross" a road without traveling along it.

In common language, traveling "through" an intersection would mean traveling from one RoadID and then switching to another RoadID at the point of the intersection (even though in common language we'd call it the same "road" all along).

The above is all totally fine - just check if I'm misunderstanding.

All correct. It will get a bit more complicated for #30, because the transition from one Road to another Road within an Intersection will have to be modeled explicitly. (Called "turn" or "movement" in abstreet.) Any ideas for more clear terms, happy to use

dabreegster · 2025-01-17T21:43:58Z

Embarrassing question, but how do I get to this view?

From the pick neighbourhood page. I would add it to edit mode too, but the "other related actions" nav is already overflowing:

I think the small breadcrumb nav is useful in the top bar, but I'm not really sure where to put the "other actions". The left sidebars get busy too. Other people have had feedback that it's hard to see those actions on the top right.

And to be fair, the old version wasn't really that much better; the top was meant to be mutually exclusive modes, all of which can be jumped to whenever. And the bottom had a few specific to one neighbourhood:

I'll leave this alone for now, but ideas welcome

michaelkirk

Embarrassing question, but how do I get to this view?

Thanks for the instructions! For transparency, I did look in the breadcrumbs, but I was drilled down into the neighborhood screen, not up a level at the "study area" screen.

I think it makes sense where it is, since we measure impact spanning all the neighborhoods.

I think mostly this speaks to me not having a good intuition yet for the data hierarchy.

Let me see if I have it right:

A "project" vs. "study area" are synonymous (or one-to-one).

vs. a neighborhood, which is a subset of a study area. A study area can have many neighborhoods.

When you are placing filters, you are drilled down to "editing a neighborhood", not editing the entire study area.

But because edits in one neighborhood might affect traffic in another, impact measurements must be done at the scale of the entire project/study-area.

Is all that correct?

Anyway, the changes look good, and seem like a good basis for this view. Thanks for walking me through it.

michaelkirk · 2025-01-17T22:03:26Z

web/src/PredictImpactMode.svelte

+            Pick neighbourhood
+          </Link>
+        </li>
+        <li>Predict impact mode</li>


Suggested change

<li>Predict impact mode</li>

<li>Predict impact</li>

"mode" feels redundant in the UI

dabreegster · 2025-01-17T22:26:17Z

Data hierarchy understanding is almost right! It is useful to see the confusing points here, because they're definitely confusing parts of the UX.

A "project" vs. "study area" are synonymous (or one-to-one).

A study area is a place like Edinburgh or a section of Seattle. A project is meant to be like a file somebody makes (and later shares) to use the tool. The project is tied to one study area, but a study area can have multiple files.

Right now, the project has just one set of edits (changes to one-ways, modal filters, turn restrictions, speeds and a definition of neighbourhood boundaries). But in the old tool, there were multiple "proposals" per file/project, and a way to switch quickly between them. I'm not sure we need all that complexity here yet. Something closer to what I'm picturing is https://nptscot.github.io/npw/ (only a few places here have working files online -- Edinburgh or Glasgow, if you're going to try this). First the user restores a project file they were working on previously, or makes a new one by first picking the study area.

When you are placing filters, you are drilled down to "editing a neighborhood", not editing the entire study area.

Correct, but this is probably confusing: #77

But because edits in one neighborhood might affect traffic in another, impact measurements must be done at the scale of the entire project/study-area.

Exactly. The flow a user might take:

Start with a neighbourhood boundary they have in mind already, or use the future neighbourhood prioritization module to pick one.
Within the edit neighbourhood view, edit stuff to achieve low shortcuts and good cell splits. There's pretty instant feedback for this. The problem being solved is traffic cutting through this one area.
Afterwards, someone needs to understand if there could be unexpected spillover effects to nearby neighbourhoods. Plug the leak in one area, but just push traffic to a residential street in the adjacent neighbourhood? No good, need to think through that too. Or push traffic onto the main road (the expected short-term consequence, before "traffic dissipation" can occur) -- maybe there are bus routes there or other issues, and actually a filter on a main perimeter road could be worth thinking about.

--

Thanks for the review! I'll wait for feedback from upstream about "predict impact" vs "predict areawide traffic impacts" or something else before renaming.

dabreegster added 6 commits January 17, 2025 10:43

Start a basic predict impact mode. Get everything wired up e2e in a

915e2a2

simple way.

Show relative change with color and baseline traffic with thickness

20e92ad

Show impact on one particular road

4bda256

Switch to deterministic random OD data, so enormous maps don't have too

9d22095

many requests

More consistent var names

c9c24ec

Clarifying for impact mode

3b993b5

dabreegster commented Jan 17, 2025

View reviewed changes

dabreegster requested a review from michaelkirk January 17, 2025 16:28

michaelkirk reviewed Jan 17, 2025

View reviewed changes

michaelkirk approved these changes Jan 17, 2025

View reviewed changes

PR feedback: don't need to say 'mode' in breadcrumbs

197bcac

dabreegster merged commit 502d6f3 into main Jan 17, 2025

dabreegster deleted the predict_impact branch January 17, 2025 22:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Start impact prediction mode #83

Start impact prediction mode #83

dabreegster commented Jan 17, 2025

dabreegster Jan 17, 2025

dabreegster Jan 17, 2025

michaelkirk Jan 17, 2025

dabreegster Jan 17, 2025

michaelkirk Jan 17, 2025

dabreegster Jan 17, 2025

dabreegster Jan 17, 2025

dabreegster Jan 17, 2025

dabreegster Jan 17, 2025

dabreegster Jan 17, 2025

dabreegster Jan 17, 2025

dabreegster Jan 17, 2025

michaelkirk Jan 17, 2025

dabreegster Jan 17, 2025

michaelkirk left a comment

michaelkirk Jan 17, 2025

michaelkirk Jan 17, 2025

dabreegster Jan 17, 2025

dabreegster commented Jan 17, 2025

michaelkirk left a comment

michaelkirk Jan 17, 2025

dabreegster Jan 17, 2025

dabreegster commented Jan 17, 2025


		use crate::{IntersectionID, MapModel, RoadID};

		// TODO Rename?

Start impact prediction mode #83

Start impact prediction mode #83

Conversation

dabreegster commented Jan 17, 2025

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

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

Choose a reason for hiding this comment

dabreegster commented Jan 17, 2025

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 Jan 17, 2025