This commit is contained in:
Andras Schmelczer 2026-07-12 21:31:13 +01:00
parent ca771a7edf
commit d7f844d566
12 changed files with 198 additions and 251 deletions

View file

@ -23,6 +23,10 @@ jobs:
- name: Install Python dependencies
run: uv sync
- name: Install finder (scraper) dependencies
working-directory: finder
run: uv sync
- uses: actions/setup-node@v4
with:
node-version: 22

View file

@ -14,6 +14,14 @@ step "Python lint: ruff" uv run ruff check .
step "Python dependency lint: deptry" uv run deptry .
step "Python unit tests" uv run pytest pipeline
(
cd "$ROOT_DIR/finder"
# finder is a separate uv project (the scraper) with its own venv, so the
# root `pytest pipeline` run above never reaches it. pytest is not a declared
# finder dependency, so pull it in ephemerally as its README documents.
step "Finder (scraper) unit tests" uv run --with pytest pytest -q
)
(
cd "$ROOT_DIR/frontend"
step "Frontend lint: ESLint" npm run lint

View file

@ -56,6 +56,22 @@ DYNAMIC_FILTER_ALL_GROUPS = {"Public Transport", "Leisure", "Health"}
DYNAMIC_FILTER_COUNT_THRESHOLD_GROUPS = {"Groceries"}
DYNAMIC_FILTER_EXCLUDED_CATEGORIES = {"Park"}
# Combined "nearest of any rail mode" distance: the distance to the closest of a
# Rail station, Tube station, DLR station, or Tram & Metro stop. Materialized as
# its own real column so the server loads it like any other per-category
# distance. min_distance_per_postcode already supports multi-category groups, so
# this is a genuine nearest-of-the-union query (identical to the per-mode
# minimum). Distance only: a combined "within Xkm" count is not meaningful, so
# it is intentionally omitted from the count groups.
COMBINED_STATION_GROUP_KEY = "poi_any_station"
COMBINED_STATION_DISPLAY_NAME = "Any station"
COMBINED_STATION_SOURCE_CATEGORIES = [
"Rail station",
"Tube station",
"DLR station",
"Tram & Metro stop",
]
def _poi_category_slug(category: str) -> str:
ascii_text = (
@ -227,10 +243,21 @@ def main():
dynamic_counts_5km = count_pois_per_postcode(
postcodes, pois, groups=poi_category_groups, radius_km=5
)
# Distances additionally include the combined "Any station" group (nearest of
# any rail mode). It is distance-only, so it is added here but not to the
# 2km/5km count groups above.
distance_groups = {
**poi_category_groups,
COMBINED_STATION_GROUP_KEY: COMBINED_STATION_SOURCE_CATEGORIES,
}
distance_display_names = {
**poi_display_names,
COMBINED_STATION_GROUP_KEY: COMBINED_STATION_DISPLAY_NAME,
}
dynamic_distances = min_distance_per_postcode(
postcodes, pois, groups=poi_category_groups
postcodes, pois, groups=distance_groups
)
dynamic_renames = _dynamic_poi_metric_renames(poi_display_names)
dynamic_renames = _dynamic_poi_metric_renames(distance_display_names)
dynamic_counts_2km = dynamic_counts_2km.rename(
{k: v for k, v in dynamic_renames.items() if k in dynamic_counts_2km.columns}
)

View file

@ -1,6 +1,9 @@
import polars as pl
from pipeline.transform.poi_proximity import (
COMBINED_STATION_DISPLAY_NAME,
COMBINED_STATION_GROUP_KEY,
COMBINED_STATION_SOURCE_CATEGORIES,
GREENSPACE_PARK_FUNCTIONS,
POI_GROUPS_2KM,
_build_poi_category_groups,
@ -8,7 +11,7 @@ from pipeline.transform.poi_proximity import (
_greenspace_count_frame,
_groceries_categories,
)
from pipeline.utils.poi_counts import count_pois_per_postcode
from pipeline.utils.poi_counts import count_pois_per_postcode, min_distance_per_postcode
def test_groceries_2km_counts_geolytix_brand_categories() -> None:
@ -84,6 +87,42 @@ def test_dynamic_poi_groups_include_requested_categories_only() -> None:
assert "poi_school" not in groups
def test_combined_station_distance_is_nearest_of_any_rail_mode() -> None:
"""The combined "Any station" distance is the nearest of Rail/Tube/DLR/Tram,
materialized as its own column so the server loads it directly (no runtime
synthesis). A far rail station and a near DLR stop: the combined distance
must follow the DLR stop."""
postcodes = pl.DataFrame(
{"postcode": ["E14 5AB"], "lat": [51.5054], "lon": [-0.0235]}
)
pois = pl.DataFrame(
{
"category": ["Rail station", "DLR station"],
"group": ["Public Transport", "Public Transport"],
"lat": [51.6000, 51.5055],
"lng": [-0.2000, -0.0236],
}
)
distance_groups = {
"poi_rail_station": ["Rail station"],
COMBINED_STATION_GROUP_KEY: COMBINED_STATION_SOURCE_CATEGORIES,
}
result = min_distance_per_postcode(postcodes, pois, groups=distance_groups)
renames = _dynamic_poi_metric_renames(
{COMBINED_STATION_GROUP_KEY: COMBINED_STATION_DISPLAY_NAME}
)
result = result.rename({k: v for k, v in renames.items() if k in result.columns})
combined = result["Distance to nearest amenity (Any station) (km)"][0]
rail_only = result["poi_rail_station_nearest_km"][0]
# The near DLR stop is within a few hundred metres; the only rail station is
# kilometres away. The combined distance must follow the DLR stop.
assert combined < 0.2
assert rail_only > 5.0
assert combined < rail_only
def test_dynamic_poi_metric_renames_support_park_count_options() -> None:
assert _dynamic_poi_metric_renames({"parks": "Park"}) == {
"parks_nearest_km": "Distance to nearest amenity (Park) (km)",

View file

@ -14,6 +14,7 @@ import com.conveyal.r5.api.util.LegMode;
import com.conveyal.r5.api.util.TransitModes;
import com.conveyal.r5.kryo.KryoNetworkSerializer;
import com.conveyal.r5.profile.StreetMode;
import com.conveyal.gtfs.model.Stop;
import com.conveyal.r5.transit.TransitLayer;
import com.conveyal.r5.transit.TransportNetwork;
import com.conveyal.r5.transit.path.RouteSequence;
@ -33,6 +34,19 @@ import java.util.List;
/** R5 routing: network loading, spatial filtering, travel time computation. */
public class Router {
// Coarse GB bounding box used to tell "a stop that should have linked to a
// street but didn't" (inside GB) from stops that legitimately never link
// (continental coach termini, neutralised out-of-area stops).
private static final double UK_MIN_LAT = 49.0;
private static final double UK_MAX_LAT = 61.0;
private static final double UK_MIN_LON = -9.0;
private static final double UK_MAX_LON = 2.5;
// If more than this fraction of transit stops fail to link to the street
// network the build is systemically broken (wrong/incomplete OSM extract, or
// corrupt coordinates) rather than the usual tiny residue of unroutable
// stops, so fail loudly instead of silently shipping a half-linked network.
private static final double MAX_UNLINKED_STOP_FRACTION = 0.10;
private static final int ZOOM = 9; // R5 enforces range 9-12
private static final int MAX_GRID_CELLS = 4_900_000; // under R5's 5M limit
// 30-minute peak window: RAPTOR cost is linear in (toTime-fromTime)/60.
@ -168,6 +182,85 @@ public class Router {
System.err.printf(" Transit: %,d stops, %,d routes, %,d patterns, %,d services%n",
stops, routes, patterns, services);
validateStopLinkage(transitLayer);
}
/**
* Verify transit stops are connected to the walkable street network. A stop
* whose coordinate lands nowhere near a street gets streetVertexForStop == -1
* (R5's "unlinked" sentinel); it is then silently unroutable no access,
* egress, or distance table so trains pass through but nobody can board or
* alight (the Tottenham Court Road failure mode, upstream of the coordinate
* fix). A handful of stops legitimately never link (continental coach termini,
* neutralised out-of-area stops); a large fraction signals a systemic break
* (wrong OSM extract, or a whole mode's coordinates corrupt) and fails.
*
* streetVertexForStop is persisted in the network cache, so unlinked stops are
* counted on both fresh and cached loads. Per-stop coordinates come from the
* transient stopForIndex, which is only populated on a fresh fromDirectory
* build; on a cached load coordinates are unavailable and only counts are logged.
*/
private static void validateStopLinkage(TransitLayer transitLayer) {
int n = transitLayer.getStopCount();
// stopForIndex is transient: on a cached (Kryo) load it comes back empty
// (size 0), not null, so require it to be fully populated (fresh build)
// before indexing into it; otherwise fall back to counts-only.
boolean haveCoords = transitLayer.stopForIndex != null
&& transitLayer.stopForIndex.size() == n;
int unlinked = 0;
int unlinkedInGb = 0;
List<String> sample = new ArrayList<>();
for (int i = 0; i < n; i++) {
if (transitLayer.streetVertexForStop.get(i) != -1) {
continue; // linked
}
unlinked++;
if (!haveCoords) {
continue;
}
Stop stop = transitLayer.stopForIndex.get(i);
if (stop == null) {
continue;
}
double lat = stop.stop_lat;
double lon = stop.stop_lon;
boolean inGb = lat >= UK_MIN_LAT && lat <= UK_MAX_LAT
&& lon >= UK_MIN_LON && lon <= UK_MAX_LON;
if (inGb) {
unlinkedInGb++;
if (sample.size() < 25) {
String id = transitLayer.stopIdForIndex.get(i);
String name = transitLayer.stopNames != null && i < transitLayer.stopNames.size()
? transitLayer.stopNames.get(i) : "";
sample.add(String.format("%s '%s' (%.5f, %.5f)", id, name, lat, lon));
}
}
}
double fraction = n == 0 ? 0.0 : (double) unlinked / n;
if (haveCoords) {
System.err.printf(
" Stop linkage: %,d/%,d stops NOT linked to street network (%.2f%%), %,d inside GB%n",
unlinked, n, fraction * 100.0, unlinkedInGb);
for (String s : sample) {
System.err.println(" unlinked (GB): " + s);
}
} else {
System.err.printf(
" Stop linkage: %,d/%,d stops NOT linked to street network (%.2f%%) "
+ "(coords unavailable on cached load)%n",
unlinked, n, fraction * 100.0);
}
if (fraction > MAX_UNLINKED_STOP_FRACTION) {
throw new IllegalStateException(String.format(
"R5 linked only %.1f%% of transit stops to the street network "
+ "(%,d of %,d unlinked). This indicates a systemic linking failure "
+ "(wrong/incomplete OSM extract, or corrupt stop coordinates); "
+ "unlinked stops are silently unroutable.",
(1.0 - fraction) * 100.0, unlinked, n));
}
}
static void validateTransitServices(TransportNetwork network, LocalDate date) {

View file

@ -75,9 +75,7 @@ pub use poi::{resolve_poi_category_filter, POICategoryGroup, POIData, SchoolMeta
pub use postcode_population::PostcodePopulation;
pub use postcodes::{OutcodeData, PostcodeData};
pub use property::{
combine_nearest_distances, combined_station_feature_name,
combined_station_source_feature_names, precompute_h3, FeatureStats, Histogram, HistoricalPrice,
PostcodePoiMetrics, PropertyData, QuantRef, RenovationEvent, TenureEvent,
COMBINED_STATION_CATEGORY,
precompute_h3, FeatureStats, Histogram, HistoricalPrice, PostcodePoiMetrics, PropertyData,
QuantRef, RenovationEvent, TenureEvent,
};
pub use travel_time::{slugify, TravelTimeStore};

View file

@ -7,10 +7,7 @@ use serde::Serialize;
use tracing::info;
use crate::consts::{NAN_U16, QUANT_SCALE};
use crate::data::{
combine_nearest_distances, combined_station_feature_name,
combined_station_source_feature_names, PropertyData, QuantRef,
};
use crate::data::{PropertyData, QuantRef};
use crate::utils::{normalize_postcode, GridIndex, InternedColumn};
const GRID_CELL_SIZE: f32 = 0.01;
@ -321,17 +318,8 @@ fn build_poi_filter_feature_data(
let mut encoded_columns = 0usize;
for (metric_idx, name) in poi_metrics.feature_names.iter().enumerate() {
let values = match extract_optional_feature_f32(df, name)? {
Some(values) => values,
// Some POI columns (the combined-station distance) are synthesized
// in memory by PostcodePoiMetrics and never written to the listings
// parquet. Reconstruct them from their source columns so listing POI
// filters match the map exactly instead of silently rejecting every
// row on an all-NaN column.
None => match synthesize_missing_poi_column(df, name, row_count)? {
Some(values) => values,
None => continue,
},
let Some(values) = extract_optional_feature_f32(df, name)? else {
continue;
};
for (row, value) in values.into_iter().enumerate() {
let dst = row * num_features + metric_idx;
@ -350,45 +338,6 @@ fn build_poi_filter_feature_data(
Ok(feature_data)
}
/// Reconstruct a POI metric column that `PostcodePoiMetrics` synthesizes in
/// memory and therefore is absent from the listings parquet. Currently only the
/// combined-station distance, derived as the elementwise nearest of its per-mode
/// source columns (the same rule the postcode side table uses). Returns `None`
/// if `name` is not a synthesized column, or if none of its sources are present.
fn synthesize_missing_poi_column(
df: &DataFrame,
name: &str,
row_count: usize,
) -> Result<Option<Vec<Option<f32>>>> {
if name != combined_station_feature_name() {
return Ok(None);
}
let mut sources: Vec<Vec<f32>> = Vec::new();
for source_name in combined_station_source_feature_names() {
if let Some(values) = extract_optional_feature_f32(df, &source_name)? {
sources.push(
values
.into_iter()
.map(|value| value.unwrap_or(f32::NAN))
.collect(),
);
}
}
if sources.is_empty() {
return Ok(None);
}
let source_refs: Vec<&[f32]> = sources.iter().map(Vec::as_slice).collect();
let combined = combine_nearest_distances(&source_refs, row_count);
Ok(Some(
combined
.into_iter()
.map(|value| value.is_finite().then_some(value))
.collect(),
))
}
fn feature_index(property_data: &PropertyData, name: &str) -> Option<usize> {
property_data
.feature_names
@ -792,51 +741,6 @@ mod tests {
assert!(!any_listing.listing_url.is_empty());
}
#[test]
fn synthesizes_combined_station_column_from_source_columns() {
// Mirrors the postcode side table: the combined-station distance is the
// finite minimum of the per-mode source columns present in the parquet.
let df = df![
"Distance to nearest amenity (Rail station) (km)" => [2.0f32, f32::NAN, 5.0],
"Distance to nearest amenity (Tube station) (km)" => [1.0f32, f32::NAN, f32::NAN],
"Distance to nearest amenity (DLR station) (km)" => [3.0f32, 4.0, f32::NAN],
]
.unwrap();
let combined = synthesize_missing_poi_column(&df, &combined_station_feature_name(), 3)
.unwrap()
.expect("combined-station column should be synthesized");
assert_eq!(combined[0], Some(1.0)); // min(2, 1, 3)
assert_eq!(combined[1], Some(4.0)); // only DLR present
assert_eq!(combined[2], Some(5.0)); // only Rail present
}
#[test]
fn does_not_synthesize_non_combined_or_sourceless_columns() {
let df = df![
"Distance to nearest amenity (Rail station) (km)" => [1.0f32],
]
.unwrap();
// A real per-mode column is read from the parquet, never synthesized.
assert!(synthesize_missing_poi_column(
&df,
"Distance to nearest amenity (Rail station) (km)",
1
)
.unwrap()
.is_none());
// The combined column cannot be built when no source columns exist.
let empty = df!["Postcode" => ["AB1 2CD"]].unwrap();
assert!(
synthesize_missing_poi_column(&empty, &combined_station_feature_name(), 1)
.unwrap()
.is_none()
);
}
#[test]
fn extracts_price_history_from_parquet() {
let path = PathBuf::from("src/data/testdata/listings_price_history.parquet");

View file

@ -18,10 +18,7 @@ mod quant;
mod stats;
pub use h3::precompute_h3;
pub use poi_metrics::{
combine_nearest_distances, combined_station_feature_name, combined_station_source_feature_names,
PostcodePoiMetrics, COMBINED_STATION_CATEGORY,
};
pub use poi_metrics::PostcodePoiMetrics;
pub use quant::QuantRef;
pub use stats::{FeatureStats, Histogram};

View file

@ -44,7 +44,7 @@ impl PostcodePoiMetrics {
pub(super) fn from_postcode_df(
df: &DataFrame,
mut feature_names: Vec<String>,
feature_names: Vec<String>,
) -> anyhow::Result<Self> {
if feature_names.is_empty() {
return Ok(Self::empty(0));
@ -56,7 +56,7 @@ impl PostcodePoiMetrics {
"Building postcode POI metric side table"
);
let mut col_major: Vec<Vec<f32>> = feature_names
let col_major: Vec<Vec<f32>> = feature_names
.par_iter()
.map(|name| {
let column = df
@ -66,8 +66,6 @@ impl PostcodePoiMetrics {
})
.collect::<anyhow::Result<Vec<_>>>()?;
append_combined_station_distance(&mut feature_names, &mut col_major);
let feature_stats: Vec<FeatureStats> = col_major
.par_iter()
.enumerate()
@ -200,140 +198,3 @@ impl PostcodePoiMetrics {
self.decode_raw(metric_idx, self.raw_for_property_row(row, metric_idx))
}
}
/// Category name of the synthetic "nearest of any rail mode" distance feature.
pub const COMBINED_STATION_CATEGORY: &str = "Any station";
/// Per-mode transport categories folded into [`COMBINED_STATION_CATEGORY`].
const COMBINED_STATION_SOURCE_CATEGORIES: &[&str] = &[
"Rail station",
"Tube station",
"DLR station",
"Tram & Metro stop",
];
fn poi_distance_feature_name(category: &str) -> String {
format!("Distance to nearest amenity ({category}) (km)")
}
/// Append a synthetic combined-station distance column: the elementwise minimum
/// of the per-mode rail/tube/tram/DLR distance columns that are present. This
/// lets users filter on the nearest station of any rail mode without a data
/// rebuild, since the source columns are already loaded. NaN (no station) rows
/// stay NaN only when every source is missing.
fn append_combined_station_distance(
feature_names: &mut Vec<String>,
col_major: &mut Vec<Vec<f32>>,
) {
let combined_name = poi_distance_feature_name(COMBINED_STATION_CATEGORY);
// Guard against re-synthesis or a pipeline that already provides the column.
if feature_names.iter().any(|name| name == &combined_name) {
return;
}
let source_indices: Vec<usize> = COMBINED_STATION_SOURCE_CATEGORIES
.iter()
.filter_map(|category| {
let name = poi_distance_feature_name(category);
feature_names.iter().position(|existing| existing == &name)
})
.collect();
if source_indices.is_empty() {
return;
}
let row_count = col_major.first().map_or(0, Vec::len);
let combined: Vec<f32> = (0..row_count)
.into_par_iter()
.map(|row| {
let nearest = source_indices
.iter()
.map(|&idx| col_major[idx][row])
.filter(|value| value.is_finite())
.fold(f32::INFINITY, f32::min);
if nearest.is_finite() {
nearest
} else {
f32::NAN
}
})
.collect();
tracing::info!(
sources = source_indices.len(),
"Synthesized combined-station POI distance column"
);
feature_names.push(combined_name);
col_major.push(combined);
}
#[cfg(test)]
mod tests {
use super::*;
fn dist_name(category: &str) -> String {
format!("Distance to nearest amenity ({category}) (km)")
}
#[test]
fn combined_station_is_elementwise_min_ignoring_nan() {
let mut names = vec![
dist_name("Rail station"),
dist_name("Tube station"),
dist_name("DLR station"),
dist_name("Tram & Metro stop"),
];
let nan = f32::NAN;
let mut cols = vec![
vec![2.0, nan, 5.0], // Rail
vec![1.0, nan, nan], // Tube
vec![3.0, 4.0, nan], // DLR
vec![nan, nan, nan], // Tram & Metro
];
append_combined_station_distance(&mut names, &mut cols);
assert_eq!(names.last().unwrap(), &dist_name(COMBINED_STATION_CATEGORY));
let combined = cols.last().unwrap();
assert_eq!(combined[0], 1.0); // min(2, 1, 3), tram NaN ignored
assert_eq!(combined[1], 4.0); // only DLR present
assert_eq!(combined[2], 5.0); // only Rail present
}
#[test]
fn combined_station_row_with_no_station_stays_nan() {
let mut names = vec![dist_name("Rail station")];
let mut cols = vec![vec![f32::NAN, 2.0]];
append_combined_station_distance(&mut names, &mut cols);
let combined = cols.last().unwrap();
assert!(combined[0].is_nan());
assert_eq!(combined[1], 2.0);
}
#[test]
fn no_source_categories_appends_nothing() {
let mut names = vec![dist_name("Café"), dist_name("Pub")];
let mut cols = vec![vec![1.0], vec![2.0]];
append_combined_station_distance(&mut names, &mut cols);
assert_eq!(names.len(), 2);
assert_eq!(cols.len(), 2);
}
#[test]
fn combined_station_is_not_duplicated() {
let mut names = vec![
dist_name("Rail station"),
dist_name(COMBINED_STATION_CATEGORY),
];
let mut cols = vec![vec![1.0], vec![1.0]];
append_combined_station_distance(&mut names, &mut cols);
assert_eq!(names.len(), 2);
assert_eq!(cols.len(), 2);
}
}

Binary file not shown.

View file

@ -20,6 +20,10 @@ const FILTER_GROUP_ORDER: &[&str] = &[
const LAST_FILTER_GROUPS: &[&str] = &["Area development"];
const POI_DISTANCE_SLIDER_MIN_KM: f32 = 0.0;
const POI_DISTANCE_SLIDER_MAX_KM: f32 = 5.0;
/// Category name of the pipeline-derived "nearest of any rail mode" distance.
/// Matches `COMBINED_STATION_DISPLAY_NAME` in `pipeline/transform/poi_proximity.py`;
/// used only to give the column friendlier filter copy.
const COMBINED_STATION_CATEGORY: &str = "Any station";
fn is_empty(val: &str) -> bool {
val.is_empty()
@ -169,6 +173,7 @@ pub fn build_features_response(data: &PropertyData) -> FeaturesResponse {
if let Some(category) = features::dynamic_poi_distance_category(name) {
let stats = &data.poi_metrics.feature_stats[feat_idx];
let is_park = category.eq_ignore_ascii_case("park");
let is_any_station = category == COMBINED_STATION_CATEGORY;
dynamic_poi_features.push(FeatureInfo::Numeric {
name: name.clone(),
min: POI_DISTANCE_SLIDER_MIN_KM,
@ -177,11 +182,15 @@ pub fn build_features_response(data: &PropertyData) -> FeaturesResponse {
histogram: stats.histogram.clone(),
description: if is_park {
"Distance to the closest park or green space".to_string()
} else if is_any_station {
"Distance to the closest rail, tube, tram, or DLR station".to_string()
} else {
format!("Distance to the closest {category} amenity")
},
detail: if is_park {
"Straight-line distance in kilometres from the postcode to the nearest park entrance. Covers public parks, gardens, playing fields, and play spaces. Uses access point locations from the OS Open Greenspace dataset, so properties bordering a large park correctly show a short distance.".to_string()
} else if is_any_station {
"Straight-line distance in kilometres from the postcode to the nearest station of any rail mode: National Rail, London Underground, DLR, or tram/metro.".to_string()
} else {
format!(
"Straight-line distance in kilometres from the postcode to the nearest {category} amenity in the amenities dataset."

View file

@ -133,6 +133,11 @@ pub async fn get_postcode_stats(
let price_history =
stats::extract_price_history(&matching_rows, &state.data, &state.feature_name_to_index);
let (sector_price_history, outcode_price_history) = stats::extract_area_price_histories(
Some(postcode_str.as_str()),
&state.data,
&state.feature_name_to_index,
);
let crime_by_year = stats::compute_crime_by_year(
&matching_rows,
@ -220,6 +225,8 @@ pub async fn get_postcode_stats(
numeric_features,
enum_features: enum_features_out,
price_history,
sector_price_history,
outcode_price_history,
crime_by_year,
crime_latest_year,
crime_outcode,