From b3e07a773aeb9adb6505fd7c37a5ce082f045921 Mon Sep 17 00:00:00 2001 From: Doug Rinckes Date: Tue, 28 May 2019 10:49:45 +0200 Subject: [PATCH 1/2] format, benchmark --- .travis.yml | 7 +++- rust/Cargo.toml | 1 + rust/README.md | 16 ++++++++ rust/src/codearea.rs | 12 +++--- rust/src/consts.rs | 10 ++--- rust/src/interface.rs | 47 +++++++++++++---------- rust/src/lib.rs | 3 +- rust/src/private.rs | 12 +++--- rust/tests/all_test.rs | 82 ++++++++++++++++++++++++++++++++++++---- rust/tests/csv_reader.rs | 3 +- 10 files changed, 142 insertions(+), 51 deletions(-) create mode 100644 rust/README.md diff --git a/.travis.yml b/.travis.yml index 675a9634..43cbbba9 100644 --- a/.travis.yml +++ b/.travis.yml @@ -119,8 +119,11 @@ matrix: # Rust implementation. Lives in rust/ - language: rust env: OLC_PATH=rust + before_script: + - rustup component add rustfmt # Adding caching of .cargo doesn't improve the build/test times. script: - cd rust/ - - cargo build --verbose --all - - cargo test --verbose --all + - cargo fmt --all -- --check + - cargo build + - cargo test -- --nocapture diff --git a/rust/Cargo.toml b/rust/Cargo.toml index fa7e1ffc..cd138983 100644 --- a/rust/Cargo.toml +++ b/rust/Cargo.toml @@ -10,3 +10,4 @@ exclude = ["rust.iml"] [dependencies] geo = "^0.4.3" +rand = "^0.6.5" diff --git a/rust/README.md b/rust/README.md new file mode 100644 index 00000000..2ab7b6e3 --- /dev/null +++ b/rust/README.md @@ -0,0 +1,16 @@ +This is the Rust implementation of the Open Location Code library. + +# Contributing + +## Code Formatting + +Code must be formatted with `rustfmt`. You can do this by running `cargo fmt`. + +The formatting will be checked in the TravisCI integration tests. If the files +need formatting the tests will fail. + +## Testing + +Test code by running `cargo test -- --nocapture`. This will run the tests +including the benchmark loops. + diff --git a/rust/src/codearea.rs b/rust/src/codearea.rs index d1dc3bcc..62582921 100644 --- a/rust/src/codearea.rs +++ b/rust/src/codearea.rs @@ -1,4 +1,4 @@ -use geo::{Point}; +use geo::Point; pub struct CodeArea { pub south: f64, @@ -12,9 +12,12 @@ pub struct CodeArea { impl CodeArea { pub fn new(south: f64, west: f64, north: f64, east: f64, code_length: usize) -> CodeArea { CodeArea { - south: south, west: west, north: north, east: east, + south: south, + west: west, + north: north, + east: east, center: Point::new((west + east) / 2f64, (south + north) / 2f64), - code_length: code_length + code_length: code_length, } } @@ -24,8 +27,7 @@ impl CodeArea { self.west + other.west, self.north + other.north, self.east + other.east, - self.code_length + other.code_length + self.code_length + other.code_length, ) } } - diff --git a/rust/src/consts.rs b/rust/src/consts.rs index ff16e57e..cc1b01ec 100644 --- a/rust/src/consts.rs +++ b/rust/src/consts.rs @@ -10,10 +10,8 @@ pub const PADDING_CHAR_STR: &'static str = "0"; // The character set used to encode the values. pub const CODE_ALPHABET: [char; 20] = [ -'2', '3', '4', '5', '6', -'7', '8', '9', 'C', 'F', -'G', 'H', 'J', 'M', 'P', -'Q', 'R', 'V', 'W', 'X', + '2', '3', '4', '5', '6', '7', '8', '9', 'C', 'F', 'G', 'H', 'J', 'M', 'P', 'Q', 'R', 'V', 'W', + 'X', ]; // The base to use to convert numbers to/from. @@ -36,9 +34,7 @@ pub const MAX_CODE_LENGTH: usize = 15; // The resolution values in degrees for each position in the lat/lng pair // encoding. These give the place value of each position, and therefore the // dimensions of the resulting area. -pub const PAIR_RESOLUTIONS: [f64; 5] = [ - 20.0f64, 1.0f64, 0.05f64, 0.0025f64, 0.000125f64 -]; +pub const PAIR_RESOLUTIONS: [f64; 5] = [20.0f64, 1.0f64, 0.05f64, 0.0025f64, 0.000125f64]; // Number of columns in the grid refinement method. pub const GRID_COLUMNS: f64 = 4f64; diff --git a/rust/src/interface.rs b/rust/src/interface.rs index 9dd6de6a..0ebbd8aa 100644 --- a/rust/src/interface.rs +++ b/rust/src/interface.rs @@ -3,14 +3,14 @@ use geo::Point; use codearea::CodeArea; use consts::{ - SEPARATOR, SEPARATOR_POSITION, PADDING_CHAR, PADDING_CHAR_STR, CODE_ALPHABET, ENCODING_BASE, - LATITUDE_MAX, LONGITUDE_MAX, PAIR_CODE_LENGTH, MAX_CODE_LENGTH, PAIR_RESOLUTIONS, GRID_COLUMNS, GRID_ROWS, - MIN_TRIMMABLE_CODE_LEN, + CODE_ALPHABET, ENCODING_BASE, GRID_COLUMNS, GRID_ROWS, LATITUDE_MAX, LONGITUDE_MAX, + MAX_CODE_LENGTH, MIN_TRIMMABLE_CODE_LEN, PADDING_CHAR, PADDING_CHAR_STR, PAIR_CODE_LENGTH, + PAIR_RESOLUTIONS, SEPARATOR, SEPARATOR_POSITION, }; use private::{ - code_value, normalize_longitude, clip_latitude, compute_latitude_precision, prefix_by_reference, - narrow_region, + clip_latitude, code_value, compute_latitude_precision, narrow_region, normalize_longitude, + prefix_by_reference, }; /// Determines if a code is a valid Open Location Code. @@ -62,7 +62,7 @@ pub fn is_valid(_code: &str) -> bool { return false; } // Extract the padding from the code (mutates code) - let padding: String = code.drain(ppos..eppos+1).collect(); + let padding: String = code.drain(ppos..eppos + 1).collect(); if padding.chars().any(|c| c != PADDING_CHAR) { // Padding must be one, contiguous block of padding chars return false; @@ -80,8 +80,7 @@ pub fn is_valid(_code: &str) -> bool { /// A short Open Location Code is a sequence created by removing four or more /// digits from an Open Location Code. It must include a separator character. pub fn is_short(_code: &str) -> bool { - is_valid(_code) && - _code.find(SEPARATOR).unwrap() < SEPARATOR_POSITION + is_valid(_code) && _code.find(SEPARATOR).unwrap() < SEPARATOR_POSITION } /// Determines if a code is a valid full Open Location Code. @@ -143,9 +142,7 @@ pub fn encode(pt: Point, code_length: usize) -> String { } } if digit < SEPARATOR_POSITION { - code.push_str( - PADDING_CHAR_STR.repeat(SEPARATOR_POSITION - digit).as_str() - ); + code.push_str(PADDING_CHAR_STR.repeat(SEPARATOR_POSITION - digit).as_str()); code.push(SEPARATOR); } code @@ -159,7 +156,8 @@ pub fn decode(_code: &str) -> Result { if !is_full(_code) { return Err(format!("Code must be a valid full code: {}", _code)); } - let mut code = _code.to_string() + let mut code = _code + .to_string() .replace(SEPARATOR, "") .replace(PADDING_CHAR_STR, "") .to_uppercase(); @@ -189,7 +187,13 @@ pub fn decode(_code: &str) -> Result { lng += lng_res * (code_value(chr) as f64 % GRID_COLUMNS); } } - Ok(CodeArea::new(lat, lng, lat + lat_res, lng + lng_res, code.len())) + Ok(CodeArea::new( + lat, + lng, + lat + lat_res, + lng + lng_res, + code.len(), + )) } /// Remove characters from the start of an OLC code. @@ -218,13 +222,16 @@ pub fn shorten(_code: &str, ref_pt: Point) -> Result { let codearea: CodeArea = decode(_code).unwrap(); if codearea.code_length < MIN_TRIMMABLE_CODE_LEN { - return Err(format!("Code length must be at least {}", MIN_TRIMMABLE_CODE_LEN)); + return Err(format!( + "Code length must be at least {}", + MIN_TRIMMABLE_CODE_LEN + )); } // How close are the latitude and longitude to the code center. - let range = (codearea.center.lat() - clip_latitude(ref_pt.lat())).abs().max( - (codearea.center.lng() - normalize_longitude(ref_pt.lng())).abs() - ); + let range = (codearea.center.lat() - clip_latitude(ref_pt.lat())) + .abs() + .max((codearea.center.lng() - normalize_longitude(ref_pt.lng())).abs()); for i in 0..PAIR_RESOLUTIONS.len() - 2 { // Check if we're close enough to shorten. The range must be less than 1/2 @@ -299,6 +306,8 @@ pub fn recover_nearest(_code: &str, ref_pt: Point) -> Result longitude { longitude += resolution; } - Ok(encode(Point::new(longitude, latitude), code_area.code_length)) + Ok(encode( + Point::new(longitude, latitude), + code_area.code_length, + )) } - diff --git a/rust/src/lib.rs b/rust/src/lib.rs index f4efe8ee..2364f414 100644 --- a/rust/src/lib.rs +++ b/rust/src/lib.rs @@ -7,5 +7,4 @@ mod codearea; pub use codearea::CodeArea; mod interface; -pub use interface::{is_valid, is_short, is_full, encode, decode, shorten, recover_nearest}; - +pub use interface::{decode, encode, is_full, is_short, is_valid, recover_nearest, shorten}; diff --git a/rust/src/private.rs b/rust/src/private.rs index bdb88cd0..f031cd24 100644 --- a/rust/src/private.rs +++ b/rust/src/private.rs @@ -1,6 +1,6 @@ use consts::{ - CODE_ALPHABET, ENCODING_BASE, LATITUDE_MAX, LONGITUDE_MAX, PAIR_CODE_LENGTH, GRID_ROWS, - GRID_COLUMNS, NARROW_REGION_PRECISION, + CODE_ALPHABET, ENCODING_BASE, GRID_COLUMNS, GRID_ROWS, LATITUDE_MAX, LONGITUDE_MAX, + NARROW_REGION_PRECISION, PAIR_CODE_LENGTH, }; use interface::encode; @@ -19,7 +19,7 @@ pub fn normalize_longitude(value: f64) -> f64 { while result >= LONGITUDE_MAX { result -= LONGITUDE_MAX * 2f64; } - while result < -LONGITUDE_MAX{ + while result < -LONGITUDE_MAX { result += LONGITUDE_MAX * 2f64; } result @@ -31,7 +31,7 @@ pub fn clip_latitude(latitude_degrees: f64) -> f64 { pub fn compute_latitude_precision(code_length: usize) -> f64 { if code_length <= PAIR_CODE_LENGTH { - return ENCODING_BASE.powf((code_length as f64 / -2f64 + 2f64).floor()) + return ENCODING_BASE.powf((code_length as f64 / -2f64 + 2f64).floor()); } ENCODING_BASE.powi(-3i32) / GRID_ROWS.powf(code_length as f64 - PAIR_CODE_LENGTH as f64) } @@ -41,9 +41,9 @@ pub fn prefix_by_reference(pt: Point, code_length: usize) -> String { let mut code = encode( Point::new( (pt.lng() / precision).floor() * precision, - (pt.lat() / precision).floor() * precision + (pt.lat() / precision).floor() * precision, ), - PAIR_CODE_LENGTH + PAIR_CODE_LENGTH, ); code.drain(code_length..); code diff --git a/rust/tests/all_test.rs b/rust/tests/all_test.rs index cd8c61c5..e21a18a9 100644 --- a/rust/tests/all_test.rs +++ b/rust/tests/all_test.rs @@ -1,11 +1,14 @@ -extern crate open_location_code; extern crate geo; +extern crate open_location_code; +extern crate rand; +use rand::Rng; +use std::time::Instant; use std::vec::Vec; -use open_location_code::{is_valid, is_short, is_full}; -use open_location_code::{encode, decode}; -use open_location_code::{shorten, recover_nearest}; +use open_location_code::{decode, encode}; +use open_location_code::{is_full, is_short, is_valid}; +use open_location_code::{recover_nearest, shorten}; use geo::Point; @@ -77,9 +80,12 @@ fn encode_test() { let code = cols[3]; assert_eq!( - encode(Point::new(lng, lat), len), code, + encode(Point::new(lng, lat), len), + code, "encoding lat={},lng={},len={}", - lat, lng, len + lat, + lng, + len ); tested += 1; @@ -99,7 +105,11 @@ fn shorten_recovery_test() { let test_type = cols[4]; if test_type == "B" || test_type == "S" { - assert_eq!(shorten(full_code, Point::new(lng, lat)).unwrap(), short_code, "shorten"); + assert_eq!( + shorten(full_code, Point::new(lng, lat)).unwrap(), + short_code, + "shorten" + ); } if test_type == "B" || test_type == "R" { assert_eq!( @@ -111,7 +121,63 @@ fn shorten_recovery_test() { tested += 1; } - assert!(tested > 0); } +#[test] +fn benchmark_test() { + struct BenchmarkData { + lat: f64, + lng: f64, + len: usize, + } + let mut rng = rand::thread_rng(); + // Create the benchmark data - coordinates and lengths for encoding, codes for decoding. + let loops = 100000; + let mut bd: Vec = Vec::new(); + for _i in 0..loops { + let lat = rng.gen_range(-90.0, 90.0); + let lng = rng.gen_range(-180.0, 180.0); + let mut len = rng.gen_range(2, 15); + // Make sure the length is even if it's less than 10. + if len < 10 && len % 2 == 1 { + len += 1; + } + let b = BenchmarkData { + lat: lat, + lng: lng, + len: len, + }; + bd.push(b); + } + + // Do the encode benchmark. + // Get the current time, loop through the benchmark data, print the time. + let mut codes: Vec = Vec::new(); + let mut now = Instant::now(); + for b in &bd { + codes.push(encode(Point::new(b.lng, b.lat), b.len)); + } + let enc_duration = now.elapsed().as_secs() * 1000000 + now.elapsed().subsec_micros() as u64; + + // Do the encode benchmark. + // Get the current time, loop through the benchmark data, print the time. + now = Instant::now(); + for c in codes { + let _c = decode(&c); + } + let dec_duration = now.elapsed().as_secs() * 1000000 + now.elapsed().subsec_micros() as u64; + // Output. + println!( + "Encoding benchmark: {} loops, total time {} usec, {} usec per encode", + loops, + enc_duration, + enc_duration as f64 / loops as f64 + ); + println!( + "Decoding benchmark: {} loops, total time {} usec, {} usec per decode", + loops, + dec_duration, + dec_duration as f64 / loops as f64 + ); +} diff --git a/rust/tests/csv_reader.rs b/rust/tests/csv_reader.rs index 1ffefeee..e1871654 100644 --- a/rust/tests/csv_reader.rs +++ b/rust/tests/csv_reader.rs @@ -3,7 +3,7 @@ use std::fs::File; use std::io::{BufRead, BufReader, Lines}; pub struct CSVReader { - iter: Lines> + iter: Lines>, } impl CSVReader { @@ -32,4 +32,3 @@ impl Iterator for CSVReader { None } } - From aeba786e4a1912d89fe4d2d35104b11198ab5cef Mon Sep 17 00:00:00 2001 From: Doug Rinckes Date: Tue, 28 May 2019 10:56:39 +0200 Subject: [PATCH 2/2] move rand into dev dependency --- rust/Cargo.toml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/rust/Cargo.toml b/rust/Cargo.toml index cd138983..633b5e07 100644 --- a/rust/Cargo.toml +++ b/rust/Cargo.toml @@ -10,4 +10,6 @@ exclude = ["rust.iml"] [dependencies] geo = "^0.4.3" + +[dev-dependencies] rand = "^0.6.5"