From e8eea997e7d1a081a29ead45553a92d660d8d43a Mon Sep 17 00:00:00 2001 From: Dustin Carlino Date: Mon, 13 Feb 2023 16:10:31 +0000 Subject: [PATCH 1/3] Add a test demonstrating a large class of geometry problems with serialized maps. --- tests/src/main.rs | 31 ++++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/tests/src/main.rs b/tests/src/main.rs index a4d45a0efd..c458f84eed 100644 --- a/tests/src/main.rs +++ b/tests/src/main.rs @@ -8,7 +8,7 @@ use rand::seq::SliceRandom; use abstio::{CityName, MapName}; use abstutil::Timer; -use geom::{Duration, Time}; +use geom::{Distance, Duration, Time}; use map_model::{IntersectionID, LaneType, Map, Perimeter, RoadID}; use sim::{AlertHandler, PrebakeSummary, Sim, SimFlags, SimOptions}; use synthpop::{IndividTrip, PersonSpec, Scenario, TripEndpoint, TripMode, TripPurpose}; @@ -31,6 +31,9 @@ fn main() -> Result<()> { if false { smoke_test()?; } + if true { + geometry_test()?; + } Ok(()) } @@ -389,3 +392,29 @@ fn bus_test() -> Result<()> { } Ok(()) } + +/// Check serialized geometry on every map. +fn geometry_test() -> Result<()> { + let mut timer = Timer::new("check geometry for all maps"); + for name in MapName::list_all_maps_locally() { + let map = map_model::Map::load_synchronously(name.path(), &mut timer); + + for l in map.all_lanes() { + let mut sum = Distance::ZERO; + // This'll crash if duplicate adjacent points snuck in. The sum check is mostly just to + // make sure the lines actually get iterated. But also, if it fails, we're discovering + // the same sort of serialization problem! + for line in l.lane_center_pts.lines() { + sum += line.length(); + } + assert_eq!(sum, l.lane_center_pts.length()); + } + + for i in map.all_intersections() { + for pair in i.polygon.get_outer_ring().points().windows(2) { + assert_ne!(pair[0], pair[1]); + } + } + } + Ok(()) +} From 93d976c6da341c8c1eb02dc43e3cd0b6cd6d3359 Mon Sep 17 00:00:00 2001 From: Dustin Carlino Date: Mon, 13 Feb 2023 16:14:04 +0000 Subject: [PATCH 2/3] Make the f64_trimming unit test in geom ACTUALLY test the correct thing ... revealing the root of most evil. --- geom/src/lib.rs | 47 +++++++++++++++++++++++++++++------------------ 1 file changed, 29 insertions(+), 18 deletions(-) diff --git a/geom/src/lib.rs b/geom/src/lib.rs index c025f0984b..a28af71601 100644 --- a/geom/src/lib.rs +++ b/geom/src/lib.rs @@ -180,28 +180,39 @@ mod tests { #[test] fn f64_trimming() { - // Roundtrip a bunch of random f64's let mut rng = rand_xorshift::XorShiftRng::seed_from_u64(42); for _ in 0..1_000 { - let input = rng.gen_range(-214_000.00..214_000.0); - let trimmed = trim_f64(input); - let json_roundtrip: f64 = - serde_json::from_slice(serde_json::to_string(&trimmed).unwrap().as_bytes()) - .unwrap(); - let bincode_roundtrip: f64 = - bincode::deserialize(&bincode::serialize(&trimmed).unwrap()).unwrap(); - assert_eq!(json_roundtrip, trimmed); - assert_eq!(bincode_roundtrip, trimmed); + // Generate a random point + let input = Pt2D::new( + rng.gen_range(-214_000.00..214_000.0), + rng.gen_range(-214_000.00..214_000.0), + ); + // Round-trip to JSON and bincode + let json_roundtrip: Pt2D = + serde_json::from_slice(serde_json::to_string(&input).unwrap().as_bytes()).unwrap(); + let bincode_roundtrip: Pt2D = + bincode::deserialize(&bincode::serialize(&input).unwrap()).unwrap(); + + // Make sure everything is EXACTLY equal + if !exactly_eq(input, json_roundtrip) || !exactly_eq(input, bincode_roundtrip) { + panic!("Round-tripping mismatch!\ninput= {:?}\njson_roundtrip= {:?}\nbincode_roundtrip={:?}", input,json_roundtrip, bincode_roundtrip); + } } // Hardcode a particular case, where we can hand-verify that it trims to 4 decimal places - let input = 1.2345678; - let trimmed = trim_f64(input); - let json_roundtrip: f64 = - serde_json::from_slice(serde_json::to_string(&trimmed).unwrap().as_bytes()).unwrap(); - let bincode_roundtrip: f64 = - bincode::deserialize(&bincode::serialize(&trimmed).unwrap()).unwrap(); - assert_eq!(json_roundtrip, 1.2346); - assert_eq!(bincode_roundtrip, 1.2346); + let input = Pt2D::new(1.2345678, 9.876543); + let json_roundtrip: Pt2D = + serde_json::from_slice(serde_json::to_string(&input).unwrap().as_bytes()).unwrap(); + let bincode_roundtrip: Pt2D = + bincode::deserialize(&bincode::serialize(&input).unwrap()).unwrap(); + assert_eq!(input.x(), 1.2346); + assert_eq!(input.y(), 9.8765); + assert!(exactly_eq(input, json_roundtrip)); + assert!(exactly_eq(input, bincode_roundtrip)); + } + + // Don't use the PartialEq implementation, which does an epsilon check + fn exactly_eq(pt1: Pt2D, pt2: Pt2D) -> bool { + pt1.x() == pt2.x() && pt1.y() == pt2.y() } } From abdf21c227cc1420597ccb53f8cffdf2ffd39043 Mon Sep 17 00:00:00 2001 From: Dustin Carlino Date: Mon, 13 Feb 2023 16:16:05 +0000 Subject: [PATCH 3/3] Fix the f64 round-tripping bug --- geom/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/geom/src/lib.rs b/geom/src/lib.rs index a28af71601..da8b751f8e 100644 --- a/geom/src/lib.rs +++ b/geom/src/lib.rs @@ -56,8 +56,8 @@ pub fn trim_f64(x: f64) -> f64 { /// Serializes a trimmed `f64` as an `i32` to save space. fn serialize_f64(x: &f64, s: S) -> Result { // So a trimmed f64's range becomes 2**31 / 10,000 =~ 214,000, which is plenty - // We don't need to round() here; trim_f64 already handles that. - let int = (x * 10_000.0) as i32; + // We MUST round here, the same as trim_f64. The unit test demonstrates why. + let int = (x * 10_000.0).round() as i32; int.serialize(s) }