Serialize trimmed f64's in the geom crate as i32, since the f64's are #686
Conversation
trimmed to 4 decimal places anyway. Do this for Pt2D, Distance, Duration, Speed, Time. Regenerating everything...
| /// Serializes a trimmed `f64` as an `i32` to save space. | ||
| fn serialize_f64<S: Serializer>(x: &f64, s: S) -> Result<S::Ok, S::Error> { | ||
| // 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. |
There was a problem hiding this comment.
We don't need to round() here; trim_f64 already handles that.
Is there a reason to keep trim_64 as something that happens as a precondition to this method? It's not obvious to me that trim_f64 is being called in all the places that use this method.
It's a minor point - so feel free to take it or leave it.
There was a problem hiding this comment.
To clarify my suggestion: would it make sense to delete trim_f64 and instead put that logic in this method.
There was a problem hiding this comment.
It's not obvious to me that trim_f64 is being called in all the places that use this method.
Unless I missed one with grep, I think all of the trim_f64 and serialize_f64 cases match up in geom. We could be more type-safe here and make trim_f64 return some TrimmedF64(f64) newtype and make the serialize methods work off of that. But it feels like a bit overkill for a small API...
would it make sense to delete trim_f64 and instead put that logic in this method.
I don't think so.. we don't want a behavioral difference in using an object that we create in-memory, vs one we load from serde. The f64 needs to be exactly the same in both cases. If we did the rounding only when serializing, then we'd get slightly different results if we
- import a
Map, immediately run a simulation with it - load a
Mapfrom disk, then run a simulation with it
since the rounding would only happen during serialization
|
Regenerating maps and running all the tests revealed some other stuff that has to change, like preserving old map edits. Nearly all of that should be fixed with this commit. Finishing up a last few pieces and then will merge |
- offstreet_parking_length in importer config - backwards compatibility for map edits - fixing up the baked-in proposals - working around a few PolyLine bugs that happen with the new rounding - Don't regenerate actdev scenarios yet -- the upstream JSON format is out of date, will have to fix separately.
dabreegster
force-pushed
the
plane_pt2d_integer
branch
from
047a1b1 to
f3e0b3a
Compare
trimmed to 4 decimal places anyway. Do this for Pt2D, Distance,
Duration, Speed, Time.
Regenerating everything...
Initial results:
Map(or anything else) and working with it in-memory, and working with one loaded from a file