diff --git a/.gitignore b/.gitignore index bc78560..ccdb9a4 100644 --- a/.gitignore +++ b/.gitignore @@ -24,3 +24,5 @@ site/ # OS .DS_Store tags +qa/screenshots/ +qa/results/ diff --git a/CLAUDE.md b/CLAUDE.md index 7e5e37a..4a4c383 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -136,6 +136,24 @@ The app shows: - Background network edges (hover to see edge IDs) - Success/failure status with error messages +### Screenshots of the real UI + +The web app exposes `GET|POST /api/screenshot.png`: it opens its own page in headless +Chromium (Playwright) with `?code=…&config=`, waits for the decode to render, and +returns a PNG — exactly what a user sees. `focus_lrp` + `span_m` zoom on an endpoint, +`map_only=true` crops out the sidebar, `scale=2` gives retina output. The same URL +parameters work in a normal browser for shareable links. `qa/screenshot.py` wraps it +and stitches before/after configs side by side: + +```bash +# server running with the network loaded (see above), then: +~/openlr-web/.venv/bin/python qa/screenshot.py CODE --out qa/screenshots/ \ + --before '{"snap_to_valid_nodes": false}' --after '{"snap_to_valid_nodes": true}' \ + --zoom-lrps 0 1 --span-m 50 +``` + +Use this to attach visual evidence to PRs and to inspect benchmark regressions. + ### Common Failure Modes 1. **"No candidates found for LRP N"** @@ -192,9 +210,29 @@ config = DecoderConfig( max_candidates=10, # More candidates = more path options length_tolerance=0.35, # 35% relative tolerance frc_tolerance=2, # Allow ±2 FRC class difference + snap_to_valid_nodes=True, # Extend path ends to junctions (OpenLR Rule 4) + max_snap_extension_m=25.0, ) ``` +### Terminal-node snap (OpenLR Rule 4) + +Encoders place LRPs on *valid* nodes (junctions); a node with one way in and one +way out is invalid because a route search can step over it. After path selection +the decoder checks whether the path starts/ends on an invalid node and, if so, +follows the forced continuation to the first junction (`RoadNetwork::is_valid_node`, +`forced_continuation`, `forced_predecessor` in `graph.rs`; `finalize_path` in +`decoder.rs`). The added length is folded into `positive_offset`/`negative_offset`, +so the offset-trimmed geometry is unchanged — only the edge set grows. Extension is +capped at `max_snap_extension_m` (25 m; deliberately not scaled by DNP — HERE links are +often only 20–40 m long and still stop a ~7–10 m crossing stub short of the junction), +must keep the segment within the length tolerance, and is only applied when the LRP coordinate is +closer to the new terminal node than to the current one — this guards against HERE +having a junction our OSM car graph lacks (measured on the KC corpus: the guard removed +all ~100 such overshoots while keeping every extension that landed on HERE's endpoint). Because the offset then spans more +than one edge, `*_offset_fraction` (relative to the first/last edge) can exceed 1.0; +the meter offsets are authoritative. + ## Building & Testing Use `uv` for Python environment management. @@ -223,11 +261,14 @@ The `qa/` directory contains tooling for measuring decode quality across a 48K-c ```bash # 1. Build the version you want to test and install into the openlr-web venv -maturin build --release -i python3.12 -cd ~/openlr-web && uv pip install --reinstall ~/openlr-decoder/target/wheels/openlr_decoder-*-cp312-*.whl +# (match the venv's interpreter: `~/openlr-web/.venv/bin/python --version`) +uvx maturin build --release -i python3.11 +cd ~/openlr-web && uv pip install --reinstall ~/openlr-decoder/target/wheels/openlr_decoder-*-cp311-*.whl # 2. Run benchmark (saves results keyed by commit hash) +# --config accepts DecoderConfig overrides (ints, floats, true/false) ~/openlr-web/.venv/bin/python qa/benchmark.py \ + --network ~/openlr-web/openlr_test_edges.parquet \ --save qa/results/$(git rev-parse --short HEAD).parquet # 3. Compare two runs @@ -244,7 +285,13 @@ cd ~/openlr-web && uv pip install --reinstall ~/openlr-decoder/target/wheels/ope ### Test corpus -`qa/test_codes.csv` contains ~48K OpenLR codes with HERE reference geometries (from `bqutils.geo.openlr_to_geography`), randomly sampled from `model-159019.michelin.arity_link_matches_usa_v2` within the KC bounding box (38.53–39.11°N, 95.15–94.47°W). Network: `openlr_test_edges.parquet` (458K edges). +`qa/test_codes.csv` contains ~48K OpenLR codes with HERE reference geometries (from `bqutils.geo.openlr_to_geography`), randomly sampled from `model-159019.michelin.arity_link_matches_usa_v2` within the KC bounding box (38.53–39.11°N, 95.15–94.47°W). Network: `~/openlr-web/openlr_test_edges.parquet` (487K edges, `startOsmNode`/`endOsmNode` +ids). Regenerate it from BigQuery with `qa/regenerate_network.py` (run with the +openlr-web venv python); it pulls the KC bbox from +`model-159019.tomtom.tomtom_segments_updated_street_export_2025_Q2_1_5_0`. + +The benchmark reconstructs geometry from the meter offsets (`positive_offset`, +`negative_offset`) expressed as fractions of the whole edge path. ## Parquet Schema (Network Input) diff --git a/qa/benchmark.py b/qa/benchmark.py index 2041990..821234b 100644 --- a/qa/benchmark.py +++ b/qa/benchmark.py @@ -161,17 +161,14 @@ def run_benchmark( df = batch.to_pandas() cols = set(df.columns) - # Handle schema differences across versions - pos_off_col = ( - "positive_offset_fraction" - if "positive_offset_fraction" in cols - else "positive_offset" - ) - neg_off_col = ( - "negative_offset_fraction" - if "negative_offset_fraction" in cols - else "negative_offset" - ) + # Offsets: prefer meter values and express them as fractions of the *whole* + # edge path (length + both offsets), which is what reconstruct_path_geometry + # expects. The per-edge `*_offset_fraction` columns are relative to the first/ + # last edge only and can exceed 1.0 when a terminal-node snap spans edges, + # so they cannot be applied to the full path directly. + has_meter_offsets = {"positive_offset", "negative_offset"} <= cols + pos_off_col = "positive_offset" if has_meter_offsets else "positive_offset_fraction" + neg_off_col = "negative_offset" if has_meter_offsets else "negative_offset_fraction" has_primary = "primary_edge_id" in cols def safe_float(val): @@ -190,6 +187,13 @@ def safe_float(val): pos_frac = safe_float(row[pos_off_col]) or 0.0 neg_frac = safe_float(row[neg_off_col]) or 0.0 + if has_meter_offsets: + lrp_len = safe_float(row["length"]) or 0.0 + total_m = lrp_len + pos_frac + neg_frac + if total_m > 0: + pos_frac, neg_frac = pos_frac / total_m, neg_frac / total_m + else: + pos_frac, neg_frac = 0.0, 0.0 result = { "openlr_code": tc["openlr_code"], @@ -426,7 +430,11 @@ def main(): config_kwargs = {} for kv in args.config: k, v = kv.split("=", 1) - # Auto-cast numeric values + # Auto-cast boolean / numeric values + if v.lower() in ("true", "false"): + v = v.lower() == "true" + config_kwargs[k] = v + continue try: v = int(v) except ValueError: diff --git a/qa/regenerate_network.py b/qa/regenerate_network.py new file mode 100644 index 0000000..d85b1b5 --- /dev/null +++ b/qa/regenerate_network.py @@ -0,0 +1,38 @@ +"""Regenerate ~/openlr-web/openlr_test_edges.parquet (KC metro test network). + +Pulls ALLOWS_CAR edges in the KC bounding box from the Replica street export in +BigQuery via the Storage Read API, with startOsmNode/endOsmNode as the node ids +(required since #25). Reuses the web app's loader with the node columns swapped. + +Usage: + cd ~/openlr-web && .venv/bin/python ~/openlr-decoder/qa/regenerate_network.py [TABLE] +""" +import inspect +import sys +from pathlib import Path + +import pyarrow.parquet as pq + +WEB = Path.home() / "openlr-web" +sys.path.insert(0, str(WEB)) +import app # noqa: E402 + +TABLE = sys.argv[1] if len(sys.argv) > 1 else app.DEFAULT_BQ_TABLE +# KC bbox used for qa/test_codes.csv +BBOX = dict(min_lat=38.53, max_lat=39.11, min_lon=-95.15, max_lon=-94.47) +OUT = WEB / "openlr_test_edges.parquet" + +code = inspect.getsource(app.load_from_bigquery).replace( + '"startVertex",\n "endVertex",', '"startOsmNode",\n "endOsmNode",' +) +assert '"startOsmNode"' in code, "app.load_from_bigquery column list changed; update this script" +ns = dict(vars(app)) +exec(code, ns) + +table = ns["load_from_bigquery"](TABLE, **BBOX) +if OUT.exists(): + backup = OUT.with_suffix(".bak.parquet") + OUT.rename(backup) + print(f"Previous network moved to {backup}") +pq.write_table(table, OUT) +print(f"Wrote {table.num_rows} edges to {OUT}") diff --git a/qa/screenshot.py b/qa/screenshot.py new file mode 100644 index 0000000..bb5c3a3 --- /dev/null +++ b/qa/screenshot.py @@ -0,0 +1,97 @@ +"""Capture before/after screenshots of the real openlr-web UI for OpenLR codes. + +Uses the app's /api/screenshot.png endpoint (headless Chromium via Playwright), so +the images are exactly what the web app shows. Requires a running server with the +network loaded, e.g.: + + cd ~/openlr-web && .venv/bin/uvicorn app:app --port 8000 + curl -X POST localhost:8000/api/network/load -H 'content-type: application/json' \ + -d '{"path":"openlr_test_edges.parquet"}' + +Usage: + python qa/screenshot.py CODE [CODE ...] --out DIR \ + --before '{"snap_to_valid_nodes": false}' --after '{"snap_to_valid_nodes": true}' \ + [--zoom-lrps 0 1] [--span-m 60] [--map-only] [--server http://localhost:8000] + +For each code this writes `_overview.jpg` plus `_lrp.jpg` for each +requested zoom LRP (zoom shots are cropped to the map). With --after, each image is the before and after shots stitched +side by side (before on the left). Pass only --before for single shots. +""" +import argparse +import io +import json +import re +import urllib.request +from pathlib import Path + + +def fetch(server: str, body: dict) -> bytes: + req = urllib.request.Request( + f"{server}/api/screenshot.png", + data=json.dumps(body).encode(), + headers={"content-type": "application/json"}, + ) + with urllib.request.urlopen(req, timeout=120) as r: + return r.read() + + +def combine(pngs: list[bytes], jpeg_quality: int | None, gap: int = 16) -> bytes: + """Stitch shots side by side (before on the left) and encode as JPEG or PNG.""" + from PIL import Image + + imgs = [Image.open(io.BytesIO(p)).convert("RGB") for p in pngs] + h = max(i.height for i in imgs) + w = sum(i.width for i in imgs) + gap * (len(imgs) - 1) + out = Image.new("RGB", (w, h), "white") + x = 0 + for im in imgs: + out.paste(im, (x, 0)) + x += im.width + gap + buf = io.BytesIO() + if jpeg_quality: + out.save(buf, format="JPEG", quality=jpeg_quality, optimize=True) + else: + out.save(buf, format="PNG", optimize=True) + return buf.getvalue() + + +def main(): + ap = argparse.ArgumentParser(description=__doc__, formatter_class=argparse.RawDescriptionHelpFormatter) + ap.add_argument("codes", nargs="+") + ap.add_argument("--out", type=Path, required=True) + ap.add_argument("--before", type=json.loads, default=None, help="DecoderConfig JSON for the left/only shot") + ap.add_argument("--after", type=json.loads, default=None, help="DecoderConfig JSON for the right shot") + ap.add_argument("--zoom-lrps", type=int, nargs="*", default=[], help="LRP indices to render zoomed views for") + ap.add_argument("--span-m", type=float, default=60.0, help="Width of zoomed views in meters") + ap.add_argument("--width", type=int, default=1100) + ap.add_argument("--height", type=int, default=750) + ap.add_argument("--map-only", action="store_true", help="Crop overview shots to the map too (zoom shots always are)") + ap.add_argument("--scale", type=int, default=1, help="Device pixel ratio (2 = retina)") + ap.add_argument("--server", default="http://localhost:8000") + ap.add_argument("--png", action="store_true", help="Write lossless PNG instead of JPEG (much larger with satellite imagery)") + ap.add_argument("--jpeg-quality", type=int, default=85) + args = ap.parse_args() + + configs = [args.before] + ([args.after] if args.after is not None else []) + args.out.mkdir(parents=True, exist_ok=True) + + for code in args.codes: + slug = re.sub(r"[^A-Za-z0-9]", "", code)[:16] + views = [("overview", {"map_only": args.map_only})] + [ + (f"lrp{i}", {"focus_lrp": i, "span_m": args.span_m, "map_only": True}) for i in args.zoom_lrps + ] + for name, view in views: + shots = [ + fetch( + args.server, + {"code": code, "config": cfg, "width": args.width, "height": args.height, "scale": args.scale, **view}, + ) + for cfg in configs + ] + out = args.out / f"{slug}_{name}.{'png' if args.png else 'jpg'}" + out.write_bytes(combine(shots, None if args.png else args.jpeg_quality)) + print(out) + + +if __name__ == "__main__": + main() diff --git a/src/decoder.rs b/src/decoder.rs index 0947c07..11e1640 100644 --- a/src/decoder.rs +++ b/src/decoder.rs @@ -220,9 +220,12 @@ pub struct DecodedPath { pub positive_offset_m: f64, /// Distance from last LRP projection to end of last edge (trim from end) in meters pub negative_offset_m: f64, - /// Fraction along first edge where first LRP projects (0.0-1.0) + /// `positive_offset_m` as a fraction of the first edge's length. Normally 0.0-1.0; + /// may exceed 1.0 when a terminal-node snap prepended edges (the offset then + /// spans more than the first edge — use `positive_offset_m` as authoritative). pub positive_offset_fraction: f64, - /// Fraction along last edge where last LRP projects (0.0-1.0, measured from end) + /// `negative_offset_m` as a fraction of the last edge's length, measured from its + /// end. Normally 0.0-1.0; may exceed 1.0 when a terminal-node snap appended edges. pub negative_offset_fraction: f64, /// The edge ID that covers the most distance in the decoded path pub primary_edge_id: u64, @@ -245,6 +248,24 @@ struct PathResult { total_length: f64, } +/// Length bookkeeping for one LRP-to-LRP segment, used to bound terminal snaps. +#[derive(Debug, Clone, Copy, Default)] +struct SegmentInfo { + /// Decoded path length of the segment (meters) + length_m: f64, + /// Encoded DNP for the segment (meters) + expected_m: f64, +} + +/// Offsets computed for a finalized path. +#[derive(Debug, Clone, Copy, Default)] +struct PathOffsets { + positive_offset_m: f64, + negative_offset_m: f64, + positive_offset_fraction: f64, + negative_offset_fraction: f64, +} + /// Configuration for the decoder #[derive(Debug, Clone)] pub struct DecoderConfig { @@ -263,6 +284,17 @@ pub struct DecoderConfig { /// service, track) in A* search. This makes the decoder prefer tertiary/unclassified /// roads over residential when paths are similar in length. Default 10m. pub access_road_cost_penalty: f64, + /// Extend decoded paths so their start and end lie on *valid* nodes (junctions), + /// per OpenLR whitepaper Rule 4. When the selected path terminates on a node + /// with no routing choice (1-in/1-out), the forced continuation is followed to + /// the first junction and the added length is absorbed into the offsets, so + /// the LRP-to-LRP geometry is unchanged. Default true. + pub snap_to_valid_nodes: bool, + /// Cap (meters) on how far a terminal-node snap may extend a path at each end. + /// The extension must also keep the segment within the length tolerance. + /// Default 25m. (Not scaled by DNP: HERE links are often only 20–40m long and + /// still stop short of a junction by a ~7–10m crossing stub.) + pub max_snap_extension_m: f64, } impl Default for DecoderConfig { @@ -274,6 +306,8 @@ impl Default for DecoderConfig { max_search_distance_factor: 2.0, // Search up to 2x the expected distance slip_road_cost_penalty: 20.0, // 20m penalty gently prefers main roads over slip roads access_road_cost_penalty: 10.0, // 10m penalty gently prefers tertiary over residential + snap_to_valid_nodes: true, + max_snap_extension_m: 25.0, } } } @@ -299,6 +333,181 @@ impl<'a> Decoder<'a> { self } + /// Upper bound on acceptable decoded length for a segment with the given DNP: + /// whichever of the relative and absolute tolerances is more generous. + fn max_valid_distance(&self, expected_distance: f64) -> f64 { + let rel_max = expected_distance * (1.0 + self.config.length_tolerance); + let abs_max = expected_distance + self.config.absolute_length_tolerance; + rel_max.max(abs_max) + } + + /// Maximum length a terminal-node snap may add to one end of the path, given + /// the adjacent segment. Returns 0 when snapping is disabled or the segment has + /// no room left under `max_valid_distance`. + fn snap_cap_m(&self, seg: SegmentInfo) -> f64 { + if !self.config.snap_to_valid_nodes { + return 0.0; + } + let cap = self.config.max_snap_extension_m; + let headroom = self.max_valid_distance(seg.expected_m) - seg.length_m; + cap.min(headroom).max(0.0) + } + + /// Walk forward from the end of `path` along forced continuations (OpenLR Rule 4: + /// nodes with no routing choice) until a valid node is reached. Returns the edges + /// to append and their total length, or `None` if no valid node is reachable + /// within `cap_m` (a partial extension would still end on an invalid node, so + /// it is not worth applying). + fn forward_extension(&self, path: &[EdgeIndex], cap_m: f64) -> Option<(Vec, f64)> { + let mut incoming = *path.last()?; + let mut node = self.network.edge_target(incoming)?; + let mut added = Vec::new(); + let mut added_m = 0.0; + while let Some(next) = self.network.forced_continuation(node, incoming) { + // Don't loop back onto edges we already traverse. + if path.contains(&next) || added.contains(&next) { + return None; + } + let len = self.network.edge(next)?.length_m; + if added_m + len > cap_m { + return None; + } + added_m += len; + added.push(next); + incoming = next; + node = self.network.edge_target(next)?; + } + if added.is_empty() { + None + } else { + Some((added, added_m)) + } + } + + /// Mirror of `forward_extension`: walk backward from the start of `path` along + /// forced predecessors to the nearest valid node. Returned edges are in path + /// order (ready to be prepended). + fn backward_extension(&self, path: &[EdgeIndex], cap_m: f64) -> Option<(Vec, f64)> { + let mut outgoing = *path.first()?; + let mut node = self.network.edge_source(outgoing)?; + let mut added = Vec::new(); + let mut added_m = 0.0; + while let Some(prev) = self.network.forced_predecessor(node, outgoing) { + if path.contains(&prev) || added.contains(&prev) { + return None; + } + let len = self.network.edge(prev)?.length_m; + if added_m + len > cap_m { + return None; + } + added_m += len; + added.push(prev); + outgoing = prev; + node = self.network.edge_source(prev)?; + } + if added.is_empty() { + None + } else { + added.reverse(); + Some((added, added_m)) + } + } + + /// Compute LRP-projection offsets for `path`, then snap its ends to valid nodes. + /// + /// Spec deviation: OpenLR Section 12.10 says offsets come from the binary-encoded + /// fractions, but HERE data always encodes them as 0. Instead we project the LRP + /// coordinates onto the first/last edges in our network, giving edge-relative + /// offsets that are meaningful for consumers working with OSM edges. + /// + /// If `snap_to_valid_nodes` is enabled and a terminal edge ends on an invalid + /// node (Rule 4), the path is extended along the forced continuation to the first + /// junction and the added length is folded into the corresponding offset, so the + /// offset-trimmed geometry is unchanged while the edge set covers the whole link. + /// The extension is only applied if the LRP coordinate is closer to the new + /// terminal node than to the current one (see the guard below). + fn finalize_path( + &self, + path: &mut Vec, + first_lrp_coord: Point, + last_lrp_coord: Point, + first_segment: SegmentInfo, + last_segment: SegmentInfo, + ) -> PathOffsets { + let (Some(&first_edge_idx), Some(&last_edge_idx)) = (path.first(), path.last()) else { + return PathOffsets::default(); + }; + + let mut positive_offset_m = self + .network + .edge(first_edge_idx) + .map(|e| { + e.length_m + * crate::spatial::project_point_to_line_fraction(first_lrp_coord, &e.geometry) + }) + .unwrap_or(0.0); + let mut negative_offset_m = self + .network + .edge(last_edge_idx) + .map(|e| { + e.length_m + * (1.0 + - crate::spatial::project_point_to_line_fraction( + last_lrp_coord, + &e.geometry, + )) + }) + .unwrap_or(0.0); + + // Guard against topology asymmetry (HERE has a junction our OSM car graph + // lacks, so a real LRP node looks like a through-node to us): only extend if + // the LRP itself is closer to the new terminal node than to the current one. + // In genuine stop-short cases the LRP sits at the junction we extend to. + let lrp_prefers = + |lrp: Point, old_node: Option, new_node: Option| match ( + old_node.and_then(|n| self.network.node(n)), + new_node.and_then(|n| self.network.node(n)), + ) { + (Some(old), Some(new)) => { + lrp.haversine_distance(&new.coord) < lrp.haversine_distance(&old.coord) + } + _ => false, + }; + + if let Some((tail, added_m)) = self.forward_extension(path, self.snap_cap_m(last_segment)) { + let old_node = path.last().and_then(|&e| self.network.edge_target(e)); + let new_node = tail.last().and_then(|&e| self.network.edge_target(e)); + if lrp_prefers(last_lrp_coord, old_node, new_node) { + path.extend(tail); + negative_offset_m += added_m; + } + } + if let Some((head, added_m)) = self.backward_extension(path, self.snap_cap_m(first_segment)) + { + let old_node = path.first().and_then(|&e| self.network.edge_source(e)); + let new_node = head.first().and_then(|&e| self.network.edge_source(e)); + if lrp_prefers(first_lrp_coord, old_node, new_node) { + path.splice(0..0, head); + positive_offset_m += added_m; + } + } + + let frac = |offset_m: f64, edge_idx: Option<&EdgeIndex>| { + edge_idx + .and_then(|&idx| self.network.edge(idx)) + .filter(|e| e.length_m > 0.0) + .map(|e| offset_m / e.length_m) + .unwrap_or(0.0) + }; + + PathOffsets { + positive_offset_m, + negative_offset_m, + positive_offset_fraction: frac(positive_offset_m, path.first()), + negative_offset_fraction: frac(negative_offset_m, path.last()), + } + } + /// Convert edge indices to stable edge IDs fn edge_indices_to_ids(&self, indices: &[EdgeIndex]) -> Vec { indices @@ -400,6 +609,10 @@ impl<'a> Decoder<'a> { let mut total_length = 0.0; // Track coverage per edge (edge may appear multiple times, so we aggregate) let mut edge_coverage_map: HashMap = HashMap::new(); + // (path length, expected DNP) of the first and last LRP segments, used to + // bound the terminal-node snap. + let mut first_segment = SegmentInfo::default(); + let mut last_segment = SegmentInfo::default(); for i in 0..points.len() - 1 { // Get distance to next point (path attributes are optional for last point) @@ -425,6 +638,14 @@ impl<'a> Decoder<'a> { // Add the actual traversed length (accounts for partial edge traversals) total_length += path_result.total_length; + let seg = SegmentInfo { + length_m: path_result.total_length, + expected_m: expected_distance, + }; + if i == 0 { + first_segment = seg; + } + last_segment = seg; // Accumulate edge coverages (aggregate if edge appears multiple times) for coverage in &path_result.coverages { @@ -456,59 +677,26 @@ impl<'a> Decoder<'a> { .map(|e| e.id) .unwrap_or(0); - // Spec deviation: OpenLR Section 12.10 says offsets come from the binary-encoded - // fractions, but HERE data always encodes them as 0. Instead we project the LRP - // coordinates onto the first/last edges in our network, giving edge-relative offsets - // that are meaningful for consumers working with OSM edges. - let ( - positive_offset_m, - negative_offset_m, - positive_offset_fraction, - negative_offset_fraction, - ) = if !full_path.is_empty() { - let first_lrp_coord = Point::new(points[0].coordinate.lon, points[0].coordinate.lat); - let last_lrp_coord = Point::new( - points[points.len() - 1].coordinate.lon, - points[points.len() - 1].coordinate.lat, - ); - - let first_edge_idx = full_path[0]; - let last_edge_idx = full_path[full_path.len() - 1]; - - let (pos_offset_m, pos_frac) = - if let Some(first_edge) = self.network.edge(first_edge_idx) { - let frac = crate::spatial::project_point_to_line_fraction( - first_lrp_coord, - &first_edge.geometry, - ); - (first_edge.length_m * frac, frac) - } else { - (0.0, 0.0) - }; - - let (neg_offset_m, neg_frac) = if let Some(last_edge) = self.network.edge(last_edge_idx) - { - let frac = crate::spatial::project_point_to_line_fraction( - last_lrp_coord, - &last_edge.geometry, - ); - (last_edge.length_m * (1.0 - frac), 1.0 - frac) - } else { - (0.0, 0.0) - }; - - (pos_offset_m, neg_offset_m, pos_frac, neg_frac) - } else { - (0.0, 0.0, 0.0, 0.0) - }; + let first_lrp_coord = Point::new(points[0].coordinate.lon, points[0].coordinate.lat); + let last_lrp_coord = Point::new( + points[points.len() - 1].coordinate.lon, + points[points.len() - 1].coordinate.lat, + ); + let offsets = self.finalize_path( + &mut full_path, + first_lrp_coord, + last_lrp_coord, + first_segment, + last_segment, + ); Ok(DecodedPath { edge_ids: self.edge_indices_to_ids(&full_path), length_m: total_length, - positive_offset_m, - negative_offset_m, - positive_offset_fraction, - negative_offset_fraction, + positive_offset_m: offsets.positive_offset_m, + negative_offset_m: offsets.negative_offset_m, + positive_offset_fraction: offsets.positive_offset_fraction, + negative_offset_fraction: offsets.negative_offset_fraction, primary_edge_id, primary_edge_coverage_m: primary_coverage, }) @@ -610,53 +798,22 @@ impl<'a> Decoder<'a> { .map(|e| e.id) .unwrap_or(0); - // Spec deviation: same as decode_line — projection-based offsets instead of binary. - let ( - positive_offset_m, - negative_offset_m, - positive_offset_fraction, - negative_offset_fraction, - ) = if !path_result.edges.is_empty() { - let first_lrp_coord = Point::new(points[0].coordinate.lon, points[0].coordinate.lat); - let last_lrp_coord = Point::new(points[1].coordinate.lon, points[1].coordinate.lat); - - let first_edge_idx = path_result.edges[0]; - let last_edge_idx = path_result.edges[path_result.edges.len() - 1]; - - let (pos_offset_m, pos_frac) = - if let Some(first_edge) = self.network.edge(first_edge_idx) { - let frac = crate::spatial::project_point_to_line_fraction( - first_lrp_coord, - &first_edge.geometry, - ); - (first_edge.length_m * frac, frac) - } else { - (0.0, 0.0) - }; - - let (neg_offset_m, neg_frac) = if let Some(last_edge) = self.network.edge(last_edge_idx) - { - let frac = crate::spatial::project_point_to_line_fraction( - last_lrp_coord, - &last_edge.geometry, - ); - (last_edge.length_m * (1.0 - frac), 1.0 - frac) - } else { - (0.0, 0.0) - }; - - (pos_offset_m, neg_offset_m, pos_frac, neg_frac) - } else { - (0.0, 0.0, 0.0, 0.0) + let mut full_path = path_result.edges.clone(); + let first_lrp_coord = Point::new(points[0].coordinate.lon, points[0].coordinate.lat); + let last_lrp_coord = Point::new(points[1].coordinate.lon, points[1].coordinate.lat); + let seg = SegmentInfo { + length_m: path_result.total_length, + expected_m: expected_distance, }; + let offsets = self.finalize_path(&mut full_path, first_lrp_coord, last_lrp_coord, seg, seg); Ok(DecodedPath { - edge_ids: self.edge_indices_to_ids(&path_result.edges), + edge_ids: self.edge_indices_to_ids(&full_path), length_m: path_result.total_length, - positive_offset_m, - negative_offset_m, - positive_offset_fraction, - negative_offset_fraction, + positive_offset_m: offsets.positive_offset_m, + negative_offset_m: offsets.negative_offset_m, + positive_offset_fraction: offsets.positive_offset_fraction, + negative_offset_fraction: offsets.negative_offset_fraction, primary_edge_id, primary_edge_coverage_m: primary_coverage, }) @@ -784,8 +941,8 @@ impl<'a> Decoder<'a> { // For maximum: use whichever is MORE GENEROUS // For short segments, absolute tolerance provides necessary slack for cross-provider // geometry differences; for long segments, relative tolerance is more appropriate - let abs_max = expected_distance + self.config.absolute_length_tolerance; - let max_valid_distance = rel_max.max(abs_max); + let max_valid_distance = self.max_valid_distance(expected_distance); + debug_assert!(max_valid_distance >= rel_max); // Maximum distance for A* search - don't explore beyond this // Use max_search_distance_factor to bound the search, with a minimum of 500m @@ -1840,4 +1997,238 @@ mod tests { assert_eq!(path.edges[0], edge_idx); assert!((path.total_length - 4.0).abs() < 0.1); } + + /// Network for terminal-snap tests (all edges one-way, west→east): + /// + /// ```text + /// N1 --e1(100m)--> N2 --e2(100m)--> N3 --e3(15m)--> N4 --e4(100m)--> N5 + /// ^ | + /// | v + /// N6 --e6(20m) N7 (e7, 100m) + /// ``` + /// + /// N3 is 1-in/1-out (invalid); N2 and N4 are junctions (valid). + fn snap_network() -> (RoadNetwork, SpatialIndex) { + TestNetworkBuilder::new() + .add_node(1, 0.0, 0.0) + .add_node(2, 0.0, 0.001) + .add_node(3, 0.0, 0.002) + .add_node(4, 0.0, 0.0021) + .add_node(5, 0.0, 0.003) + .add_node(6, -0.0002, 0.001) + .add_node(7, -0.001, 0.0021) + .add_edge(1, 1, 2, 100.0, Frc::Frc4, Fow::SingleCarriageway) + .add_edge(2, 2, 3, 100.0, Frc::Frc4, Fow::SingleCarriageway) + .add_edge(3, 3, 4, 15.0, Frc::Frc4, Fow::SingleCarriageway) + .add_edge(4, 4, 5, 100.0, Frc::Frc4, Fow::SingleCarriageway) + .add_edge(6, 6, 2, 20.0, Frc::Frc4, Fow::SingleCarriageway) + .add_edge(7, 4, 7, 100.0, Frc::Frc4, Fow::SingleCarriageway) + .build() + } + + fn edge_by_id(network: &RoadNetwork, id: u64) -> EdgeIndex { + *network.edge_id_to_index.as_ref().unwrap().get(&id).unwrap() + } + + fn ids(network: &RoadNetwork, path: &[EdgeIndex]) -> Vec { + path.iter().map(|&i| network.edge(i).unwrap().id).collect() + } + + #[test] + fn test_snap_extends_end_to_valid_node_and_folds_into_offset() { + let (network, spatial) = snap_network(); + let decoder = Decoder::new(&network, &spatial); + + // Path e1,e2 ends at N3 (invalid). Per Rule 4 the encoder placed the last + // LRP at the junction N4, 15m further on — our path stopped short. + let mut path = vec![edge_by_id(&network, 1), edge_by_id(&network, 2)]; + let n1 = network + .node(network.edge_source(path[0]).unwrap()) + .unwrap() + .coord; + let n4 = network + .node(network.edge_target(edge_by_id(&network, 3)).unwrap()) + .unwrap() + .coord; + let seg = SegmentInfo { + length_m: 200.0, + expected_m: 200.0, + }; + + let offsets = decoder.finalize_path(&mut path, n1, n4, seg, seg); + + assert_eq!( + ids(&network, &path), + vec![1, 2, 3], + "e3 (15m) appended to reach N4" + ); + assert!((offsets.negative_offset_m - 15.0).abs() < 1e-6); + assert!((offsets.negative_offset_fraction - 1.0).abs() < 1e-6); + // Start (N1, a source node) is valid — untouched. + assert!(offsets.positive_offset_m.abs() < 1e-6); + } + + #[test] + fn test_snap_respects_absolute_cap() { + let (network, spatial) = snap_network(); + let config = DecoderConfig { + max_snap_extension_m: 10.0, // e3 is 15m + ..DecoderConfig::default() + }; + let decoder = Decoder::new(&network, &spatial).with_config(config); + + let mut path = vec![edge_by_id(&network, 1), edge_by_id(&network, 2)]; + let n1 = network + .node(network.edge_source(path[0]).unwrap()) + .unwrap() + .coord; + let n4 = network + .node(network.edge_target(edge_by_id(&network, 3)).unwrap()) + .unwrap() + .coord; + let seg = SegmentInfo { + length_m: 200.0, + expected_m: 200.0, + }; + + let offsets = decoder.finalize_path(&mut path, n1, n4, seg, seg); + assert_eq!(ids(&network, &path), vec![1, 2]); + assert!(offsets.negative_offset_m.abs() < 1e-6); + } + + #[test] + fn test_snap_respects_length_headroom() { + let (network, spatial) = snap_network(); + let decoder = Decoder::new(&network, &spatial); + let mut path = vec![edge_by_id(&network, 1), edge_by_id(&network, 2)]; + let n1 = network + .node(network.edge_source(path[0]).unwrap()) + .unwrap() + .coord; + let n4 = network + .node(network.edge_target(edge_by_id(&network, 3)).unwrap()) + .unwrap() + .coord; + + // Segment already at max_valid_distance (DNP 200 → max 300) → no headroom + let no_headroom = SegmentInfo { + length_m: 295.0, + expected_m: 200.0, + }; + decoder.finalize_path(&mut path, n1, n4, no_headroom, no_headroom); + assert_eq!(ids(&network, &path), vec![1, 2]); + } + + #[test] + fn test_snap_disabled_by_config() { + let (network, spatial) = snap_network(); + let config = DecoderConfig { + snap_to_valid_nodes: false, + ..DecoderConfig::default() + }; + let decoder = Decoder::new(&network, &spatial).with_config(config); + let mut path = vec![edge_by_id(&network, 1), edge_by_id(&network, 2)]; + let n1 = network + .node(network.edge_source(path[0]).unwrap()) + .unwrap() + .coord; + let n4 = network + .node(network.edge_target(edge_by_id(&network, 3)).unwrap()) + .unwrap() + .coord; + let seg = SegmentInfo { + length_m: 200.0, + expected_m: 200.0, + }; + decoder.finalize_path(&mut path, n1, n4, seg, seg); + assert_eq!(ids(&network, &path), vec![1, 2]); + } + + #[test] + fn test_snap_extends_start_to_valid_node() { + let (network, spatial) = snap_network(); + let decoder = Decoder::new(&network, &spatial); + + // Path e3,e4 starts at N3 (invalid); its forced predecessor e2 (100m) leads + // back to the junction N2, where the first LRP sits. Needs a generous cap. + let mut path = vec![edge_by_id(&network, 3), edge_by_id(&network, 4)]; + let n2 = network + .node(network.edge_source(edge_by_id(&network, 2)).unwrap()) + .unwrap() + .coord; + let n5 = network + .node(network.edge_target(path[1]).unwrap()) + .unwrap() + .coord; + let config = DecoderConfig { + max_snap_extension_m: 150.0, + ..DecoderConfig::default() + }; + let decoder = decoder.with_config(config); + let seg = SegmentInfo { + length_m: 115.0, + expected_m: 1500.0, + }; + + let offsets = decoder.finalize_path(&mut path, n2, n5, seg, seg); + assert_eq!( + ids(&network, &path), + vec![2, 3, 4], + "e2 prepended to reach junction N2" + ); + assert!((offsets.positive_offset_m - 100.0).abs() < 1e-6); + assert!((offsets.positive_offset_fraction - 1.0).abs() < 1e-6); + // End N5 is a sink (valid) — untouched. + assert!(offsets.negative_offset_m.abs() < 1e-6); + } + + #[test] + fn test_snap_skipped_when_lrp_sits_on_through_node() { + // Same as test_snap_extends_end_to_valid_node..., but the last LRP is placed + // at N3 (the through-node) rather than at the junction N4. This models HERE + // having a junction at N3 that our OSM car graph lacks: extending would + // overshoot the LRP, so the guard must leave the path alone. + let (network, spatial) = snap_network(); + let decoder = Decoder::new(&network, &spatial); + let mut path = vec![edge_by_id(&network, 1), edge_by_id(&network, 2)]; + let n1 = network + .node(network.edge_source(path[0]).unwrap()) + .unwrap() + .coord; + let n3 = network + .node(network.edge_target(path[1]).unwrap()) + .unwrap() + .coord; + let seg = SegmentInfo { + length_m: 200.0, + expected_m: 200.0, + }; + let offsets = decoder.finalize_path(&mut path, n1, n3, seg, seg); + assert_eq!(ids(&network, &path), vec![1, 2], "LRP at N3: no extension"); + assert!(offsets.negative_offset_m.abs() < 1e-6); + } + + #[test] + fn test_snap_noop_when_terminal_nodes_already_valid() { + let (network, spatial) = snap_network(); + let decoder = Decoder::new(&network, &spatial); + // e1 runs N1 (source) → N2 (junction): both valid. + let mut path = vec![edge_by_id(&network, 1)]; + let n1 = network + .node(network.edge_source(path[0]).unwrap()) + .unwrap() + .coord; + let n2 = network + .node(network.edge_target(path[0]).unwrap()) + .unwrap() + .coord; + let seg = SegmentInfo { + length_m: 100.0, + expected_m: 100.0, + }; + let offsets = decoder.finalize_path(&mut path, n1, n2, seg, seg); + assert_eq!(ids(&network, &path), vec![1]); + assert!(offsets.positive_offset_m.abs() < 1e-6); + assert!(offsets.negative_offset_m.abs() < 1e-6); + } } diff --git a/src/graph.rs b/src/graph.rs index 8c79f34..c3184a2 100644 --- a/src/graph.rs +++ b/src/graph.rs @@ -331,6 +331,112 @@ impl RoadNetwork { self.graph.edge_endpoints(edge_idx).map(|(s, _)| s) } + /// Check whether a node is a *valid* node per OpenLR whitepaper Rule 4 (§6). + /// + /// A node is **invalid** when a route search can step over it without making + /// a choice: it connects exactly two distinct neighbours, with at most one + /// incoming and one outgoing edge per neighbour and equal in/out degree + /// (i.e. 1-in/1-out, or 2-in/2-out on a bidirectional through-road). + /// Everything else — junctions, dead ends, one-way merges/splits — is valid. + /// + /// Encoders place LRPs on valid nodes, so a decoded path terminating on an + /// invalid node has most likely stopped short of the intended endpoint. + pub fn is_valid_node(&self, node_idx: NodeIndex) -> bool { + use petgraph::Direction; + + let mut in_deg = 0usize; + let mut out_deg = 0usize; + // (neighbour, incoming count, outgoing count) — at most 2 tracked before bailing + let mut neighbours: Vec<(NodeIndex, usize, usize)> = Vec::with_capacity(3); + + let mut record = |other: NodeIndex, incoming: bool| -> bool { + if other == node_idx { + return false; // self-loop: not a through-node + } + match neighbours.iter_mut().find(|(n, _, _)| *n == other) { + Some(entry) => { + if incoming { + entry.1 += 1; + } else { + entry.2 += 1; + } + } + None => { + if neighbours.len() >= 2 { + return false; // 3+ distinct neighbours: junction + } + neighbours.push(if incoming { + (other, 1, 0) + } else { + (other, 0, 1) + }); + } + } + true + }; + + for e in self.graph.edges_directed(node_idx, Direction::Incoming) { + in_deg += 1; + if !record(e.source(), true) { + return true; + } + } + for e in self.graph.edges_directed(node_idx, Direction::Outgoing) { + out_deg += 1; + if !record(e.target(), false) { + return true; + } + } + + if neighbours.len() != 2 || in_deg != out_deg { + return true; + } + // Each neighbour may contribute at most one edge in each direction; + // parallel edges to the same neighbour imply a real choice. + if neighbours.iter().any(|(_, i, o)| *i > 1 || *o > 1) { + return true; + } + false + } + + /// If `node_idx` is an invalid node (see [`is_valid_node`](Self::is_valid_node)), + /// return the single outgoing edge that continues travel after arriving via + /// `incoming` — i.e. the only exit that is not a U-turn. Returns `None` at a + /// valid node, or if `incoming` does not actually end at `node_idx`. + pub fn forced_continuation( + &self, + node_idx: NodeIndex, + incoming: EdgeIndex, + ) -> Option { + let (from, to) = self.graph.edge_endpoints(incoming)?; + if to != node_idx || self.is_valid_node(node_idx) { + return None; + } + self.graph + .edges(node_idx) + .find(|e| e.target() != from) + .map(|e| e.id()) + } + + /// Mirror of [`forced_continuation`](Self::forced_continuation) for walking + /// backwards: if `node_idx` is invalid, return the single incoming edge a + /// traveller must have arrived on before leaving via `outgoing`. + pub fn forced_predecessor( + &self, + node_idx: NodeIndex, + outgoing: EdgeIndex, + ) -> Option { + use petgraph::Direction; + let (from, to) = self.graph.edge_endpoints(outgoing)?; + if from != node_idx || self.is_valid_node(node_idx) { + return None; + } + self.graph + .edges_directed(node_idx, Direction::Incoming) + .find(|e| e.source() != to) + .map(|e| e.id()) + } + pub fn node_count(&self) -> usize { self.graph.node_count() } @@ -409,4 +515,91 @@ mod tests { 0.0 ); } + + fn ls(a: (f64, f64), b: (f64, f64)) -> LineString { + LineString::from(vec![a, b]) + } + + /// Build a small network: A→X→B one-way through-road, plus a junction J + /// with three neighbours, and a dead end D hanging off B. + fn topo_network() -> (RoadNetwork, HashMap<&'static str, NodeIndex>) { + let mut net = RoadNetwork::new(); + let pts: [(&str, i64, (f64, f64)); 6] = [ + ("A", 1, (0.0, 0.0)), + ("X", 2, (0.001, 0.0)), + ("B", 3, (0.002, 0.0)), + ("J", 4, (0.003, 0.0)), + ("C", 5, (0.003, 0.001)), + ("D", 6, (0.002, -0.001)), + ]; + let mut idx = HashMap::new(); + for (name, id, (x, y)) in pts { + idx.insert(name, net.get_or_add_node(id, Point::new(x, y))); + } + let p = |n: &str| pts.iter().find(|q| q.0 == n).unwrap().2; + let mut eid = 0; + let mut add = |net: &mut RoadNetwork, a: &str, b: &str| { + eid += 1; + let (ia, ib) = ( + pts.iter().find(|q| q.0 == a).unwrap().1, + pts.iter().find(|q| q.0 == b).unwrap().1, + ); + net.add_edge( + ia, + ib, + Edge::new(eid, ls(p(a), p(b)), Frc::Frc4, Fow::SingleCarriageway), + ) + }; + add(&mut net, "A", "X"); // one-way A→X + add(&mut net, "X", "B"); // one-way X→B + add(&mut net, "B", "J"); + add(&mut net, "J", "B"); + add(&mut net, "J", "C"); + add(&mut net, "C", "J"); + add(&mut net, "B", "D"); // dead-end spur, two-way + add(&mut net, "D", "B"); + (net, idx) + } + + #[test] + fn test_is_valid_node_rule4() { + let (net, n) = topo_network(); + // X: 1-in/1-out through-node → invalid + assert!(!net.is_valid_node(n["X"])); + // A: source only (0-in/1-out) → valid + assert!(net.is_valid_node(n["A"])); + // B: neighbours X, J, D → junction → valid + assert!(net.is_valid_node(n["B"])); + // J: neighbours B, C, bidirectional, 2-in/2-out → invalid (through-node) + assert!(!net.is_valid_node(n["J"])); + // C, D: dead ends (single neighbour) → valid + assert!(net.is_valid_node(n["C"])); + assert!(net.is_valid_node(n["D"])); + } + + #[test] + fn test_forced_continuation_and_predecessor() { + let (net, n) = topo_network(); + let edge_between = |a: NodeIndex, b: NodeIndex| net.graph.find_edge(a, b).unwrap(); + + let ax = edge_between(n["A"], n["X"]); + let xb = edge_between(n["X"], n["B"]); + let bj = edge_between(n["B"], n["J"]); + let jc = edge_between(n["J"], n["C"]); + + // Arriving at X via A→X must continue on X→B + assert_eq!(net.forced_continuation(n["X"], ax), Some(xb)); + // Arriving at J via B→J must continue to C (not U-turn back to B) + assert_eq!(net.forced_continuation(n["J"], bj), Some(jc)); + // B is a valid node: no forced continuation + assert_eq!(net.forced_continuation(n["B"], xb), None); + // Mismatched incoming edge + assert_eq!(net.forced_continuation(n["X"], xb), None); + + // Backwards: leaving X via X→B means we arrived via A→X + assert_eq!(net.forced_predecessor(n["X"], xb), Some(ax)); + // Leaving J via J→C means we arrived via B→J + assert_eq!(net.forced_predecessor(n["J"], jc), Some(bj)); + assert_eq!(net.forced_predecessor(n["B"], bj), None); + } } diff --git a/src/python.rs b/src/python.rs index 0a3c9e3..c7ce2d6 100644 --- a/src/python.rs +++ b/src/python.rs @@ -154,6 +154,13 @@ pub struct PyDecoderConfig { /// service, track) in A* search. Makes decoder prefer tertiary over residential. Default 10m. #[pyo3(get, set)] pub access_road_cost_penalty: f64, + /// Extend decoded paths so they start/end on valid (junction) nodes per OpenLR + /// Rule 4, folding the extra length into the offsets. Default True. + #[pyo3(get, set)] + pub snap_to_valid_nodes: bool, + /// Cap in meters on a terminal-node snap at each end. Default 25m. + #[pyo3(get, set)] + pub max_snap_extension_m: f64, } #[pymethods] @@ -175,7 +182,9 @@ impl PyDecoderConfig { frc_weight = 0.1, fow_weight = 0.1, slip_road_cost_penalty = 20.0, - access_road_cost_penalty = 10.0 + access_road_cost_penalty = 10.0, + snap_to_valid_nodes = true, + max_snap_extension_m = 25.0 ))] fn new( search_radius_m: f64, @@ -192,6 +201,8 @@ impl PyDecoderConfig { fow_weight: f64, slip_road_cost_penalty: f64, access_road_cost_penalty: f64, + snap_to_valid_nodes: bool, + max_snap_extension_m: f64, ) -> Self { PyDecoderConfig { search_radius_m, @@ -208,6 +219,8 @@ impl PyDecoderConfig { fow_weight, slip_road_cost_penalty, access_road_cost_penalty, + snap_to_valid_nodes, + max_snap_extension_m, } } @@ -217,7 +230,8 @@ impl PyDecoderConfig { max_candidates={}, max_candidate_distance_m={}, length_tolerance={}, \ absolute_length_tolerance={}, max_search_distance_factor={}, \ distance_weight={}, bearing_weight={}, frc_weight={}, fow_weight={}, \ - slip_road_cost_penalty={}, access_road_cost_penalty={})", + slip_road_cost_penalty={}, access_road_cost_penalty={}, \ + snap_to_valid_nodes={}, max_snap_extension_m={})", self.search_radius_m, self.max_bearing_diff, self.frc_tolerance, @@ -231,7 +245,9 @@ impl PyDecoderConfig { self.frc_weight, self.fow_weight, self.slip_road_cost_penalty, - self.access_road_cost_penalty + self.access_road_cost_penalty, + self.snap_to_valid_nodes, + self.max_snap_extension_m ) } } @@ -255,6 +271,8 @@ impl From<&PyDecoderConfig> for DecoderConfig { max_search_distance_factor: config.max_search_distance_factor, slip_road_cost_penalty: config.slip_road_cost_penalty, access_road_cost_penalty: config.access_road_cost_penalty, + snap_to_valid_nodes: config.snap_to_valid_nodes, + max_snap_extension_m: config.max_snap_extension_m, } } }