Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add precision argument with default value #13

Closed
Robinlovelace opened this issue Mar 20, 2021 · 12 comments
Closed

Add precision argument with default value #13

Robinlovelace opened this issue Mar 20, 2021 · 12 comments

Comments

@Robinlovelace
Copy link
Contributor

As stated by @mvl22 here cyipt/actdev#10 5 decimal places ~1m accuracy which is probably enough for many purposes assuming 1 km starting size (1/1000 accuracy).

Could be a new arg in the new Params struct, concept I've just discovered!

@Robinlovelace Robinlovelace changed the title Add precision argument Add precision argument with default value Mar 20, 2021
@Robinlovelace
Copy link
Contributor Author

Looking at this with reference to the cheatsheet here: https://floating-point-gui.de/languages/rust/

@Robinlovelace
Copy link
Contributor Author

@Robinlovelace
Copy link
Contributor Author

OK maybe not, only has 4 stars on GitHub: https://github.com/0x022b/libmath-rs Any advice @dabreegster ? I'm still looking and learning.

@Robinlovelace
Copy link
Contributor Author

Latest thinking: fmt lines only works for printing. Planning to try this approach: https://stackoverflow.com/questions/63214346/how-to-truncate-f64-to-2-decimal-places

@dabreegster
Copy link
Contributor

https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=608a9849b07c0916a651d18b7e1a744c
Indeed, the problem is that all of the geo types implement std::fmt::Display, and they don't do any rounding.

The stackoverflow approach is good. I would advise creating a new utility method fn round(poly: &mut Polygon<f64>, precision: usize) and call this at the very end of clockboard. Don't sprinkle the rounding logic everywhere in the code; just post-process the final result. https://docs.rs/geo/0.17.1/geo/struct.Polygon.html#method.exterior_mut should help mutate a Polygon.

@Robinlovelace
Copy link
Contributor Author

fn round(poly: &mut Polygon, precision: usize)

Sounds like a good idea. Do I need another for loop to iterate over every element? So far I have this but cargo run tells me that this is not good:

fn round(poly: &mut Polygon<f64>, precision: usize) {
    poly.exterior_mut(f64::trunc(before  * 100.0) / 100.0;)
}

@dabreegster
Copy link
Contributor

https://docs.rs/geo/0.17.1/geo/struct.Polygon.html#method.exterior_mut
exterior_mut takes a closure as an argument; that's like a little function without a name. https://doc.rust-lang.org/rust-by-example/fn/closures.html talks about how to use them.

In this case, the closure is given a mutable LineString. If you click through the docs to https://docs.rs/geo/0.17.1/geo/struct.LineString.html, you can see how to work on it. You could figure out how to iterate over its points mutably and then do the truncate.

... But actually as I look at the docs, I spotted https://docs.rs/geo/0.17.1/geo/algorithm/map_coords/trait.MapCoordsInplace.html. This is implemented for Polygon, so all you have to do is follow that example and do the truncate.

@Robinlovelace
Copy link
Contributor Author

... But actually as I look at the docs, I spotted https://docs.rs/geo/0.17.1/geo/algorithm/map_coords/trait.MapCoordsInplace.html. This is implemented for Polygon, so all you have to do is follow that example and do the truncate.

Awesome, guessed there would be a more direct way. Will give it a spin!

@Robinlovelace
Copy link
Contributor Author

OK follow-up question, I now have this which seems to be working:

fn round(poly: &mut Polygon<f64>, precision: usize) {
    poly.map_coords_inplace(|&(x, y)| (x + 1000., y * 2.))    
}

pub fn clockboard(
    centerpoint: Point<f64>,
    params: Params,
    boundary: Option<Polygon<f64>>,
) -> Vec<Polygon<f64>> {
    let mut polygons = Vec::new();
    let circle = makecircle(centerpoint, params.distances[0], params.num_vertices); 
    polygons.push(circle);
    polygons
}

How do apply to round function to polygon option? Like this in the penultimate line would seem reasonable to me:

    polygons.round(precision)

But that fails with

cargo run
   Compiling zonebuilder v0.1.0 (/home/robin/github-orgs/zonebuilders/zonebuilder-rust)
error[E0425]: cannot find value `precision` in this scope
  --> src/lib.rs:37:20
   |
37 |     polygons.round(precision)
   |                    ^^^^^^^^^ not found in this scope

Robinlovelace added a commit that referenced this issue Mar 27, 2021
@dabreegster
Copy link
Contributor

The error says precision isn't defined. Search through the code for it -- it's right, where's that come from? Remember it's inside params.

Your next issue is that you can't do polygons.round. polygons is a Vec<Polygon<f64>>. If https://doc.rust-lang.org/std/vec/struct.Vec.html doesn't say there's a round method, it's not going to work. The round method you've defined operates on a single Polygon<f64>. So, you need to iterate through the list of polygons and apply the function to each one. See if any of the methods in https://doc.rust-lang.org/std/vec/struct.Vec.html look appropriate

@Robinlovelace
Copy link
Contributor Author

Think this is good to go. Please review/merge Dustin when you get a chance. Many thanks!

dabreegster pushed a commit that referenced this issue Mar 27, 2021
dabreegster pushed a commit that referenced this issue Mar 27, 2021
@Robinlovelace
Copy link
Contributor Author

Closed in #16

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants