diff --git a/docs/TECHNICAL_DESIGN.md b/docs/TECHNICAL_DESIGN.md index 9c7a008b..6889ceac 100644 --- a/docs/TECHNICAL_DESIGN.md +++ b/docs/TECHNICAL_DESIGN.md @@ -398,8 +398,9 @@ fetch away. `suggest_edits` below) exactly once, writing all four layers both of `conflated.pmtiles`' passes need: - the coarse **overview** `matched`/`unmatched` layers — one - deliberately minimal feature per row: `spider`, `matched`, and - `osm:type`/`osm:id` when matched, but **no tags and no `fid`**. At + deliberately minimal feature per row: `spider`, `matched`, + `importance` (rounded to two decimals), and `osm:type`/`osm:id` + when matched, but **no tags and no `fid`**. At z0 a single tile holds the whole planet, and every per-feature byte counts: tags push the tile so far past tippecanoe’s size limit that `--drop-densest-as-needed` guts it to a few hundred features @@ -477,7 +478,16 @@ fetch away. — runs tippecanoe over those layers to build `diffed-places.pmtiles` for visual review. **~2m48s** at full-planet scale (cpx42 run). The same function now builds both this and `conflated.pmtiles` above, - parameterized by output filename. + parameterized by output filename. For this archive and the + `conflated.pmtiles` overview, a tippecanoe feature filter (`-j`) on + the rounded `importance` decides what coarse zooms show: the top 1% + at z0, 4% at z1, 16% at z2, 64% at z3 and everything from z4 on + (four times as many per zoom, since each tile splits into four), + with no random dot-dropping (`-r 1`) and `--drop-densest-as-needed` + only as the safety net for oversized tiles. A per-feature + `"tippecanoe": {"minzoom": N}` would be the obvious way, but with + tippecanoe v2.49 it made tippecanoe drop nearly all features at + every zoom. - **`upload_conflated` / `upload_conflated_tiles` / `upload_tiles`** ([`src/pipeline/upload.rs`](../src/pipeline/upload.rs)) — push the data output and both sets of tiles to the public (`PUBLIC_S3_*`) @@ -652,7 +662,7 @@ until the full pipeline was proven out: conflating municipal tree datasets against OSM by spatial distance and species looks particularly tractable). See [#708](https://github.com/brawer/osmdiffs/issues/708). -- There’s no per-row match-confidence or cartographic-importance - signal yet, so both PMTiles archives rely entirely on tippecanoe’s - own density-based dropping at low zoom — see - [#713](https://github.com/brawer/osmdiffs/issues/713). +- There’s no per-row match-confidence signal yet. Low-zoom tiles + select features by `importance` (see + [#713](https://github.com/brawer/osmdiffs/issues/713)), but not by + how sure we are of a match. diff --git a/docs/outputs/CONFLATED_TILES.md b/docs/outputs/CONFLATED_TILES.md index 44b2f669..61555629 100644 --- a/docs/outputs/CONFLATED_TILES.md +++ b/docs/outputs/CONFLATED_TILES.md @@ -48,8 +48,17 @@ against a full planet: full tags → ~370 features visible at z0; `+fid` Geometry is `osm_geometry` (the matched OpenStreetMap shape — point, line, or polygon) for `matched` rows, `atp_geometry` for `unmatched`. -Properties: `spider`, `matched`, and — `matched` rows only — `osm:type` -/ `osm:id`. To see a row's tags, click through to the detail layer +Properties: `spider`, `matched`, `importance`, and — `matched` rows +only — `osm:type` / `osm:id`. + +Coarse zooms show the most important rows, by their +[`importance`](CONFLATED_PARQUET.md#importance): the top 1% at z0 +(`importance >= 0.99`), 4% at z1 (`>= 0.96`), 16% at z2 (`>= 0.84`), +64% at z3 (`>= 0.36`), and every row from z4 on. Each zoom shows four +times as many as the one before, since a tile splits into four, so a +tile holds about as many features at every zoom. Where a tile would +still be too large, tippecanoe's `--drop-densest-as-needed` thins it +out further. To see a row's tags, click through to the detail layer (below): `matched` rows join on `osm:type`+`osm:id`, `unmatched` rows on nearest `part: "atp"` feature of the same `spider`. @@ -95,6 +104,8 @@ for the full investigation. `atp.fetched.spider` in `conflated.parquet`). On every feature. - `matched` (`true` | `false`) — overview features only; the detail layer conveys the same thing through its layer name and `part`. +- `importance` — the row's `importance` from `conflated.parquet`, + rounded to two decimals. Overview features only. - `osm:type`, `osm:id` — the matched OpenStreetMap object. On the `matched` overview feature (where they double as the join key to the detail layer) and the z13–16 `osm`/`link` detail features. diff --git a/src/pipeline/conflated_tiles.rs b/src/pipeline/conflated_tiles.rs index b18ed40e..fd9f4af1 100644 --- a/src/pipeline/conflated_tiles.rs +++ b/src/pipeline/conflated_tiles.rs @@ -44,7 +44,8 @@ //! [#775](https://github.com/brawer/osmdiffs/issues/775) for //! the full design discussion. -use crate::utils::parquet::{get_binary, get_string, get_struct, get_tags, get_u64}; +use super::tiles::round_importance; +use crate::utils::parquet::{get_binary, get_f32, get_string, get_struct, get_tags, get_u64}; use crate::{TileLayer, make_progress_bar}; use anyhow::{Context, Result}; use arrow::array::{Array, RecordBatch}; @@ -153,6 +154,8 @@ pub struct ConflatedLayers { /// skipped entirely, the same defensive handling `edits.rs` applies. struct ConflatedTileRow { spider: String, + /// See `pipeline::importance`. + importance: f32, atp_tags: Vec<(String, String)>, atp_geometry_wkb: Vec, osm: Option, @@ -179,12 +182,10 @@ impl ConflatedTileRow { /// the z13+ detail layer ([`Self::to_detail_geojson_lines`] / /// [`Self::to_unmatched_detail_geojson_line`]). /// - /// TODO(#713): once conflated.parquet carries a per-row rank/ - /// importance signal, this is the natural place to also emit it - /// as a feature property (or otherwise feed it to tippecanoe), - /// so --drop-densest-as-needed can prefer important places over - /// low-priority ones at low zoom instead of today's - /// density-only heuristic. + /// Also carries the row's `importance`, rounded to two decimals so + /// that it costs the tile at most 101 distinct values, which the + /// overview's tippecanoe filter uses to show the most important + /// places at low zooms (see `tiles::importance_filter`). fn to_overview_geojson_line(&self) -> Result { let geometry_wkb: &[u8] = match &self.osm { Some(osm) => &osm.osm_geometry_wkb, @@ -199,6 +200,10 @@ impl ConflatedTileRow { properties.insert("osm:type".to_string(), Value::String(osm.osm_type.clone())); properties.insert("osm:id".to_string(), Value::from(osm.osm_id)); } + properties.insert( + "importance".to_string(), + Value::from(round_importance(self.importance)), + ); geometry_to_geojson_line(&geometry, properties) } @@ -561,6 +566,7 @@ fn extract_conflated_tile_row(batch: &RecordBatch, row: usize) -> Result, osm_version: u32, osm_geometry_wkb: Vec, + importance: f32, } fn produce_edits( @@ -211,6 +223,7 @@ fn produce_edits( osm_version: row.osm_version, tags, centroid: decode_centroid(&row.osm_geometry_wkb)?, + importance: row.importance, }; num_edits.fetch_add(1, Ordering::Relaxed); // TODO: Dispatch to one of {shops, infrastructure, trees} @@ -283,6 +296,7 @@ fn extract_conflated_row(batch: &RecordBatch, row: usize) -> Result, path: &Path, workdir: &Path) -> R Ok(num_edits.load(Ordering::SeqCst)) } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn to_geojson_carries_rounded_importance() { + let edit = SuggestedEdit { + osm_id: 42, + osm_type: "node".to_string(), + osm_version: 3, + tags: vec![("opening_hours".to_string(), "Mo-Fr 08:00-18:00".to_string())], + centroid: (8.5, 47.2), + importance: 0.4567, + }; + let value = serde_json::to_value(edit.to_geojson()).expect("json"); + assert_eq!(value["properties"]["@importance"], 0.46); + assert_eq!(value["properties"]["@osm_version"], 3); + } +} diff --git a/src/pipeline/mod.rs b/src/pipeline/mod.rs index bf54749a..5b3b5e9f 100644 --- a/src/pipeline/mod.rs +++ b/src/pipeline/mod.rs @@ -311,6 +311,7 @@ fn run_pipeline_steps( min: 0, max: conflated_tiles::DETAIL_MIN_ZOOM - 1, }, + Some("importance"), ) })?; let conflated_detail = run_step("render_conflated_detail", || { @@ -326,6 +327,7 @@ fn run_pipeline_steps( min: conflated_tiles::DETAIL_MIN_ZOOM, max: conflated_tiles::DETAIL_MAX_ZOOM, }, + None, ) })?; let conflated_tiles_out = run_step("join_conflated_tiles", || { @@ -349,6 +351,7 @@ fn run_pipeline_steps( workdir, "diffed-places.pmtiles", tiles::ZoomRange::Auto, + Some("@importance"), ) })?; published.push(run_step("upload_tiles", || { diff --git a/src/pipeline/tiles.rs b/src/pipeline/tiles.rs index ff5a5807..1ba883cb 100644 --- a/src/pipeline/tiles.rs +++ b/src/pipeline/tiles.rs @@ -24,12 +24,60 @@ pub enum ZoomRange { Bounded { min: u8, max: u8 }, } +/// Share of features, by `importance` (see `pipeline::importance`), +/// that tiles show at z0. Each zoom level above that shows four times +/// as many (a tile splits into four, so features per tile stay about +/// the same), until every feature is shown from [`ALL_FEATURES_ZOOM`] +/// on. On a planet of ~4M conflated rows, 1% is ~40k features, about +/// what fits into z0 today. +const TOP_SHARE_AT_Z0: f64 = 0.01; + +/// From this zoom level on, tiles show features of any importance. +const ALL_FEATURES_ZOOM: u8 = 4; + +/// Rounds `importance` to two decimals for writing it into tiles, so a +/// tile holds at most 101 distinct values of it. +pub(crate) fn round_importance(importance: f32) -> f64 { + (f64::from(importance) * 100.0).round() / 100.0 +} + +/// The lowest (rounded) importance that tiles show at `zoom`: 0.99 at +/// z0, 0.96 at z1, 0.84 at z2, 0.36 at z3. +fn min_importance_at_zoom(zoom: u8) -> f64 { + let share = TOP_SHARE_AT_Z0 * 4f64.powi(i32::from(zoom)); + ((1.0 - share) * 100.0).round() / 100.0 +} + +/// A tippecanoe feature filter (`-j`) so that coarse zooms show the +/// most important places, rather than whatever survives density-based +/// dropping ([#713](https://github.com/brawer/osmdiffs/issues/713)). +/// `attr` names the feature property holding the rounded importance. +/// +/// A filter rather than a per-feature `"tippecanoe": {"minzoom": N}`, +/// because with tippecanoe v2.49 the latter made it drop nearly all +/// features at every zoom (2 of 1,436 at z0 in a Swiss test run). +fn importance_filter(attr: &str) -> String { + let mut any = vec![ + serde_json::json!("any"), + serde_json::json!([">=", "$zoom", ALL_FEATURES_ZOOM]), + ]; + for zoom in 0..ALL_FEATURES_ZOOM { + any.push(serde_json::json!([ + "all", + [">=", "$zoom", zoom], + [">=", attr, min_importance_at_zoom(zoom)] + ])); + } + serde_json::json!({ "*": any }).to_string() +} + pub fn render_tiles( layers: &[TileLayer], progress: &MultiProgress, workdir: &Path, out_filename: &str, zoom_range: ZoomRange, + importance_attr: Option<&str>, ) -> Result { let out_path = workdir.join(out_filename); if out_path.exists() { @@ -45,7 +93,7 @@ pub fn render_tiles( let progress_bar = make_progress_bar(progress, "tiles.render", 100, "percent"); - let mut cmd = make_tippecanoe_command(layers, workdir, &tmp_path, zoom_range)?; + let mut cmd = make_tippecanoe_command(layers, workdir, &tmp_path, zoom_range, importance_attr)?; let mut child = cmd.stderr(Stdio::piped()).spawn()?; let stderr = child.stderr.take().expect("Failed to capture stderr"); let stderr_reader = BufReader::new(stderr); @@ -72,6 +120,7 @@ fn make_tippecanoe_command( workdir: &Path, out_path: &Path, zoom_range: ZoomRange, + importance_attr: Option<&str>, ) -> Result { let mut cmd = Command::new("tippecanoe"); // Clear all environment variables, so we don't leak secrets to an untrusted subprocess. @@ -90,7 +139,21 @@ fn make_tippecanoe_command( cmd.arg(format!("-Z{min}")).arg(format!("-z{max}")); } } - cmd.arg("-r").arg("1.2").arg("--drop-densest-as-needed"); + match importance_attr { + // Importance decides what coarse zooms show, so no additional + // random dot-dropping; `--drop-densest-as-needed` still thins + // out any tile that would otherwise be too large. + Some(attr) => { + cmd.arg("-r") + .arg("1") + .arg("-j") + .arg(importance_filter(attr)); + } + None => { + cmd.arg("-r").arg("1.2"); + } + } + cmd.arg("--drop-densest-as-needed"); for layer in layers.iter() { // Suppress empty layers; Tippecanoe fails if any input file is empty. let metadata = std::fs::metadata(&layer.path)?; @@ -174,6 +237,59 @@ fn parse_progress(line: &str) -> Option { mod tests { use super::*; + #[test] + fn test_round_importance() { + assert_eq!(round_importance(0.98765), 0.99); + assert_eq!(round_importance(0.4567), 0.46); + assert_eq!(round_importance(0.0), 0.0); + assert_eq!(round_importance(1.0), 1.0); + } + + #[test] + fn test_min_importance_at_zoom() { + let got: Vec = (0..ALL_FEATURES_ZOOM).map(min_importance_at_zoom).collect(); + assert_eq!(got, [0.99, 0.96, 0.84, 0.36]); + } + + #[test] + fn test_importance_filter() { + let filter: serde_json::Value = + serde_json::from_str(&importance_filter("imp")).expect("valid JSON"); + assert_eq!( + filter, + serde_json::json!({"*": [ + "any", + [">=", "$zoom", 4], + ["all", [">=", "$zoom", 0], [">=", "imp", 0.99]], + ["all", [">=", "$zoom", 1], [">=", "imp", 0.96]], + ["all", [">=", "$zoom", 2], [">=", "imp", 0.84]], + ["all", [">=", "$zoom", 3], [">=", "imp", 0.36]], + ]}) + ); + } + + #[test] + fn test_make_tippecanoe_command_importance_filter() { + let layers = [make_test_tile_layer("Shops", "diffed-shops.jsonl")]; + let cmd = make_tippecanoe_command( + &layers, + &PathBuf::from("test-workdir"), + &PathBuf::from("test-output.pmtiles"), + ZoomRange::Auto, + Some("@importance"), + ) + .expect("should not fail"); + let args: Vec = cmd + .get_args() + .map(|s| s.to_string_lossy().into_owned()) + .collect(); + let r = args.iter().position(|a| a == "-r").expect("-r"); + assert_eq!(args[r + 1], "1"); + let j = args.iter().position(|a| a == "-j").expect("-j"); + assert_eq!(args[j + 1], importance_filter("@importance")); + assert!(args.iter().any(|a| a == "--drop-densest-as-needed")); + } + #[test] fn test_make_tippecanoe_command_auto_zoom() { let layers = [ @@ -183,7 +299,7 @@ mod tests { ]; let workdir = PathBuf::from("test-workdir"); let out = PathBuf::from("test-output.pmtiles"); - let cmd = make_tippecanoe_command(&layers, &workdir, &out, ZoomRange::Auto) + let cmd = make_tippecanoe_command(&layers, &workdir, &out, ZoomRange::Auto, None) .expect("should not fail"); assert_eq!(cmd.get_program(), "tippecanoe"); let args: Vec = cmd @@ -222,6 +338,7 @@ mod tests { &workdir, &out, ZoomRange::Bounded { min: 13, max: 16 }, + None, ) .expect("should not fail"); let args: Vec = cmd diff --git a/src/utils/parquet.rs b/src/utils/parquet.rs index 17033bb0..8754be49 100644 --- a/src/utils/parquet.rs +++ b/src/utils/parquet.rs @@ -14,8 +14,8 @@ use anyhow::{Context, Result}; use arrow::array::{ - Array, ArrayRef, BinaryArray, MapArray, RecordBatch, StringArray, StructArray, UInt16Array, - UInt32Array, UInt64Array, + Array, ArrayRef, BinaryArray, Float32Array, MapArray, RecordBatch, StringArray, StructArray, + UInt16Array, UInt32Array, UInt64Array, }; /// Something a named column can be looked up in -- a [`RecordBatch`] @@ -86,6 +86,14 @@ pub(crate) fn get_u16(src: &impl ColumnSource, name: &str, row: usize) -> Result .value(row)) } +pub(crate) fn get_f32(src: &impl ColumnSource, name: &str, row: usize) -> Result { + Ok(column(src, name)? + .as_any() + .downcast_ref::() + .with_context(|| format!("column '{name}' is not Float32"))? + .value(row)) +} + pub(crate) fn get_binary(src: &impl ColumnSource, name: &str, row: usize) -> Result> { Ok(column(src, name)? .as_any() diff --git a/tests/integration_test.rs b/tests/integration_test.rs index 97877e5c..a6d4ea30 100644 --- a/tests/integration_test.rs +++ b/tests/integration_test.rs @@ -515,6 +515,10 @@ fn assert_shops_jsonl(path: &Path) -> Result<()> { feature["properties"].get("@osm_changeset").is_none(), "feature should not carry @osm_changeset: {feature}" ); + let importance = feature["properties"]["@importance"] + .as_f64() + .unwrap_or_else(|| panic!("feature has no @importance: {feature}")); + assert!((0.0..=1.0).contains(&importance), "{feature}"); } // One specific, known edit: this OSM feature's opening_hours and