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

very minor change in internals #2

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

MichaelChirico
Copy link

noticed you declared nr but then didn't use it.

also, another suggestion (not included here):

instead of the iterative version of .decimate.core used here, why not just sequence along the edge? this approach is faster in my benchmarks for small levels of precision (it's more or less stable w.r.t. precision, whereas .decimate.core had marked increases in computation time for precision <.1. further, it is very consistent in speed across precision levels.

it is slower than the .decimate.core version for moderately large levels of precision (relative to the initial distance), which is why I wanted to ping you before including it.

here's what I came up with this morning:

.decimate.core<-function(x,pr) {
  if (.lengths(x)<pr)x
  else{
    nn<-ceiling(.lengths(x)/pr)
    cbind(seq(x[1,1],x[2,1],by=(x[2,1]-x[1,1])/nn),
          seq(x[1,2],x[2,2],by=(x[2,2]-x[1,2])/nn))
  }
}

this may be able to be sped up. the idea is to divide the line between the pair of points into just enough equal-length segments to make the proximal distances below pr.

let me know what you think

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

Successfully merging this pull request may close these issues.

1 participant