search

csgillespie / poweRlaw

This package implements both the discrete and continuous maximum likelihood estimators for fitting the power-law distribution to data. Additionally, a goodness-of-fit based approach is used to estimate the lower cutoff for the scaling region.
109 stars 24 forks source link

ensure input and output of dpldis have same length #73

Closed mpadge closed 6 years ago

mpadge commented 6 years ago

Heya Colin, i'm finally getting back around to my convolution ms. This PR solves the problem that the following lines work:

f <- function (x) dpldis (x, xmin = 1, alpha = 2)
integrate (f, 1, Inf)

while the following fail:

f2 <- function (x, y) f (y - x) * f (x)
integrate (f2, 1, Inf, y = 10)

The reason is simply because the offending line changes the length of the output of dpldis. The PR ensures that both cases work, and so allows dpldis to be passed to any other integration or convolution functions. This passes all tests, and I think has only one consequence that it no longer ensures that the log(x) call does not receive any negative numbers - i'll leave that to you if that's okay. - that's now taken care of as well.

Note another way to look at the problem: xmin may often be derived from some other routine, and not necessarily be explicitly known. The following will not work, and it will be difficult to reconcile the known x with the shorter result of dpldis. The commit also avoids that problem.

x <- 0:n
plot (x, dpldis (x, xmin = xmin, alpha = alpha))
csgillespie commented 6 years ago

I think the PR looks OK. But I'm having an odd issue with travis and tests. Once I figure out, I'll merge.

mpadge commented 6 years ago

yeah i saw the travis output, but it was clearly not related to my PR so i ignored it. Hope you can figure it out

csgillespie commented 6 years ago

Thanks for the bug fix (it was a bug).

Also fixed travis. It seems that testthat and set.seed have suddenly stopped playing together. Odd.