Skip to content

Commit aa2b528

Browse files
brawerclaude
andauthored
Add OsmCandidate + score_osm_candidate, additive (#661)
PR 3 of the staged sequence tracked in #655. New OsmCandidate<'a> (matchers/mod.rs): pairs a tables::Feature with the StringPool needed to resolve its tag ids -- kept as its own type (rather than threading &Feature/&StringPool through Matcher separately) so a future hydration step (e.g. a pre-decoded geo::Geometry for Hausdorff-distance scoring, or a pre-tokenized name for Token Sort Ratio matching) can attach fields here without another trait-signature change. tags() is cheap; geometry() decodes WKB and is not, so callers should only reach for it when they actually need the shape. trait Matcher gains a new score_osm_candidate(&self, &OsmCandidate) method, alongside the existing Place-based score/suggest_edit -- not replacing them yet, so conflate.rs/edits.rs don't need to change in this PR at all (they still go through Place/PlaceIndex). Needs to be a trait method, not just an inherent method on PoiMatcher: conflate/edits call through Box<dyn Matcher>, so a later PR that wires this in can't reach an inherent method that isn't on the trait object. PoiMatcher::score_osm_candidate mirrors score's brand:wikidata-based logic, but checks the (cheap) tag match before decoding the (not cheap) geometry, unlike score's current unconditional-then-check order -- Place's distance was already free to compute, Feature's isn't. Since Feature has no precomputed position (unlike Place's s2_cell_id), distance is computed from the decoded geometry's centroid; added geo_point_distance/geo_point_distance_score alongside the existing Place-based distance/distance_score for this. Also extracted find_brand_wikidata(), shared by for_place/score/score_osm_candidate instead of duplicating the same tag scan three times, and deduplicated a stray inline EARTH_RADIUS_METERS constant into one module-level one. Tests: ported test_poi_matcher's CH_CLOTHES/CH_KIOSK fixtures to Feature/StringPool (reusing the same real-world coordinates via their s2_cell_id, so this exercises the WKB-decode-then-centroid path against genuine positions), plus direct unit tests for OsmCandidate::tags/geometry and geo_point_distance_score. Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
1 parent c62afd9 commit aa2b528

2 files changed

Lines changed: 295 additions & 33 deletions

File tree

src/matchers/mod.rs

Lines changed: 175 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,22 @@
11
//! Logic for matching AllThePlaces with OpenStreetMap features.
22
33
use crate::places::Place;
4+
use crate::tables::{Feature, StringPool};
5+
use anyhow::{Context, Result};
46
use deepsize::DeepSizeOf;
7+
use geo_traits::to_geo::ToGeoGeometry;
58
use s2::{cellid::CellID, point::Point, s1::ChordAngle};
69
use serde::{Deserialize, Serialize};
710
use std::sync::LazyLock;
11+
use wkb::reader::read_wkb;
812

913
mod poi_matcher;
1014

15+
/// Mean Earth radius, in meters -- used throughout this module to turn
16+
/// S2 `Angle`s (dimensionless, on the unit sphere) into real-world
17+
/// distances.
18+
const EARTH_RADIUS_METERS: f64 = 6_371_000.0;
19+
1120
/// A bitmask to speed up the matching of AllThePlaces with OpenStreetMap.
1221
///
1322
/// The mask provides a first, very rough way to exclude matches. For example,
@@ -233,17 +242,62 @@ static LARGE_DISTANCE: LazyLock<ChordAngle> = LazyLock::new(|| meters_to_chord_a
233242

234243
fn meters_to_chord_angle(radius_meters: f64) -> ChordAngle {
235244
use s2::s1::angle::{Angle, Rad};
236-
const EARTH_RADIUS_METERS: f64 = 6_371_000.0;
237245
ChordAngle::from(Angle::from(Rad(radius_meters / EARTH_RADIUS_METERS)))
238246
}
239247

240-
/// Trait for objects that can score a `Place`.
248+
/// An OpenStreetMap feature under consideration as a match candidate.
249+
/// Kept as its own type (rather than threading `&Feature`/`&StringPool`
250+
/// through `Matcher` separately) so a future hydration step -- for
251+
/// derived data that's *not* cheap to recompute on every access, e.g. a
252+
/// decoded `geo::Geometry` for Hausdorff-distance scoring, or a
253+
/// pre-tokenized name for Token Sort Ratio matching -- can attach fields
254+
/// here without another trait-signature change.
255+
#[allow(unused)]
256+
pub struct OsmCandidate<'a> {
257+
pub feature: &'a Feature,
258+
pub strings: &'a StringPool<'a>,
259+
}
260+
261+
#[allow(unused)]
262+
impl<'a> OsmCandidate<'a> {
263+
/// Decodes `feature.tags` -- flat `[key_id, value_id, ...]` pairs of
264+
/// `StringPool` indices -- into `(key, value)` string pairs. Cheap: a
265+
/// handful of string-pool lookups.
266+
pub fn tags(&self) -> impl Iterator<Item = (&'a str, &'a str)> + '_ {
267+
self.feature.tags.chunks_exact(2).map(move |kv| {
268+
(
269+
self.strings.get(kv[0] as usize),
270+
self.strings.get(kv[1] as usize),
271+
)
272+
})
273+
}
274+
275+
/// Decodes `feature.geometry_wkb` into a `geo::Geometry`. Not cheap
276+
/// -- allocates real nested structures, non-trivially so for a
277+
/// complex polygon -- so callers should only do this when they
278+
/// actually need the shape (e.g. to compute a centroid for distance
279+
/// scoring), not on every candidate unconditionally.
280+
pub fn geometry(&self) -> Result<geo::Geometry<f64>> {
281+
Ok(read_wkb(&self.feature.geometry_wkb)
282+
.context("failed to decode feature geometry")?
283+
.to_geometry())
284+
}
285+
}
286+
287+
/// Trait for objects that can score a match candidate.
241288
pub trait Matcher {
242289
/// Returns a score between 0.0 and 1.0 indicating how well the place matches.
243290
/// A high score means a good match; 0.0 means the place is clearly not a match.
244291
fn score(&self, place: &Place) -> f64;
245292

246293
fn suggest_edit(&self, osm_feature: &Place) -> Option<Place>;
294+
295+
/// Like `score`, but against an [OsmCandidate] backed by the new,
296+
/// geometry-aware `OsmFeatureIndex` path instead of `Place`. Landing
297+
/// alongside `score`/`suggest_edit` rather than replacing them, as
298+
/// part of a staged migration of OSM-side matching off `Place`.
299+
#[allow(unused)]
300+
fn score_osm_candidate(&self, candidate: &OsmCandidate) -> f64;
247301
}
248302

249303
/// Construct a matcher for a given AllThePlaces feature.
@@ -265,7 +319,7 @@ pub fn create_matcher(place: &Place) -> Option<Box<dyn Matcher + '_>> {
265319

266320
fn distance(pt: &Point, place: &Place) -> f64 {
267321
let pt2 = Point(CellID(place.s2_cell_id).raw_point().normalize());
268-
pt.distance(&pt2).rad() * 6_371_000.0
322+
pt.distance(&pt2).rad() * EARTH_RADIUS_METERS
269323
}
270324

271325
fn distance_score(pt: &Point, place: &Place, max_meters: f64) -> f64 {
@@ -277,6 +331,29 @@ fn distance_score(pt: &Point, place: &Place, max_meters: f64) -> f64 {
277331
}
278332
}
279333

334+
/// Like `distance`, but against a plain geographic point (longitude,
335+
/// latitude) instead of a `Place`'s stored S2 cell -- `Feature` (unlike
336+
/// `Place`) doesn't carry a precomputed position, so callers scoring an
337+
/// [OsmCandidate] compute one themselves (e.g. a decoded geometry's
338+
/// centroid) and pass it in here.
339+
#[allow(unused)]
340+
fn geo_point_distance(pt: &Point, geo_pt: &geo::Point<f64>) -> f64 {
341+
let ll = s2::latlng::LatLng::from_degrees(geo_pt.y(), geo_pt.x());
342+
let pt2 = Point::from(ll);
343+
pt.distance(&pt2).rad() * EARTH_RADIUS_METERS
344+
}
345+
346+
/// Like `distance_score`, but for [geo_point_distance].
347+
#[allow(unused)]
348+
fn geo_point_distance_score(pt: &Point, geo_pt: &geo::Point<f64>, max_meters: f64) -> f64 {
349+
let dist = geo_point_distance(pt, geo_pt);
350+
if dist <= max_meters {
351+
(max_meters - dist) / max_meters
352+
} else {
353+
0.0
354+
}
355+
}
356+
280357
fn parse_wikidata_id(s: &str) -> Option<u64> {
281358
let trimmed = s.trim();
282359
let digits = trimmed
@@ -288,6 +365,9 @@ fn parse_wikidata_id(s: &str) -> Option<u64> {
288365
#[cfg(test)]
289366
mod tests {
290367
use super::*;
368+
use s2::{cell::Cell, latlng::LatLng};
369+
use tempfile::TempDir;
370+
use wkb::writer::{WriteOptions, write_point};
291371

292372
#[test]
293373
fn test_match_distance() {
@@ -298,4 +378,96 @@ mod tests {
298378
mask.add_tag("shop", "yes");
299379
assert!(match_distance(&mask) == match_distance(&MatchMask::SHOP));
300380
}
381+
382+
const WKB_OPTS: WriteOptions = WriteOptions {
383+
endianness: wkb::Endianness::LittleEndian,
384+
};
385+
386+
#[test]
387+
fn osm_candidate_tags_decodes_string_pool_indices() {
388+
let dir = TempDir::new().expect("tempdir");
389+
let strings = ["shop", "clothes", "brand", "New Yorker"];
390+
let pool = StringPool::create(
391+
strings.iter().map(|s| s.to_string()),
392+
dir.path(),
393+
&dir.path().join("strings"),
394+
)
395+
.expect("StringPool::create");
396+
397+
let feature = Feature {
398+
id: 1,
399+
tags: vec![
400+
pool.lookup("shop").unwrap() as u32,
401+
pool.lookup("clothes").unwrap() as u32,
402+
pool.lookup("brand").unwrap() as u32,
403+
pool.lookup("New Yorker").unwrap() as u32,
404+
],
405+
..Default::default()
406+
};
407+
let candidate = OsmCandidate {
408+
feature: &feature,
409+
strings: &pool,
410+
};
411+
let tags: Vec<(&str, &str)> = candidate.tags().collect();
412+
assert_eq!(tags, vec![("shop", "clothes"), ("brand", "New Yorker")]);
413+
}
414+
415+
#[test]
416+
fn osm_candidate_geometry_decodes_wkb() {
417+
let dir = TempDir::new().expect("tempdir");
418+
let pool = StringPool::create(std::iter::empty(), dir.path(), &dir.path().join("strings"))
419+
.expect("StringPool::create");
420+
421+
let mut geometry_wkb = Vec::new();
422+
write_point(
423+
&mut geometry_wkb,
424+
&geo::Point::new(7.4478123, 46.9479801),
425+
&WKB_OPTS,
426+
)
427+
.expect("wkb encode");
428+
let feature = Feature {
429+
id: 1,
430+
geometry_wkb,
431+
..Default::default()
432+
};
433+
let candidate = OsmCandidate {
434+
feature: &feature,
435+
strings: &pool,
436+
};
437+
438+
match candidate.geometry().expect("geometry decode") {
439+
geo::Geometry::Point(p) => {
440+
assert!((p.x() - 7.4478123).abs() < 1e-6);
441+
assert!((p.y() - 46.9479801).abs() < 1e-6);
442+
}
443+
other => panic!("expected a point, got {other:?}"),
444+
}
445+
}
446+
447+
#[test]
448+
fn osm_candidate_geometry_rejects_invalid_wkb() {
449+
let dir = TempDir::new().expect("tempdir");
450+
let pool = StringPool::create(std::iter::empty(), dir.path(), &dir.path().join("strings"))
451+
.expect("StringPool::create");
452+
let feature = Feature {
453+
id: 1,
454+
geometry_wkb: vec![0xff, 0x00, 0x01],
455+
..Default::default()
456+
};
457+
let candidate = OsmCandidate {
458+
feature: &feature,
459+
strings: &pool,
460+
};
461+
assert!(candidate.geometry().is_err());
462+
}
463+
464+
#[test]
465+
fn geo_point_distance_score_close_vs_far() {
466+
let center = Cell::from(CellID::from(LatLng::from_degrees(46.9479801, 7.4478123))).center();
467+
let close = geo::Point::new(7.4478123, 46.9479801); // same spot
468+
let far = geo::Point::new(-122.4194, 37.7749); // San Francisco
469+
470+
assert!(geo_point_distance_score(&center, &close, 400.0) > 0.99);
471+
assert_eq!(geo_point_distance_score(&center, &far, 400.0), 0.0);
472+
}
301473
}

0 commit comments

Comments
 (0)