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

StatsBase.sample should warn about weights containing Inf/NaN #671

Closed
xiruizhao opened this issue Mar 16, 2021 · 4 comments
Closed

StatsBase.sample should warn about weights containing Inf/NaN #671

xiruizhao opened this issue Mar 16, 2021 · 4 comments

Comments

@xiruizhao
Copy link

xiruizhao commented Mar 16, 2021

function sample(rng::AbstractRNG, wv::AbstractWeights)
t = rand(rng) * sum(wv)

Perhaps add something like

swv = sum(wv)
isfinite(swv) || throw(DomainError(swv, "weights sum to $swv"))
@nalimilan
Copy link
Member

Good idea. Would you make a pull request?

@ruairidhs
Copy link
Contributor

I recently ran into a bug due to this issue. A warning about Inf weights would have helped me to find the problem faster. I see the PR wasn't accepted though. Are there still plans to implement this?

@nalimilan
Copy link
Member

Actually the PR wasn't rejected, it was closed by its author (I don't know why). Would you be willing to make a new PR with its contents?

@ruairidhs
Copy link
Contributor

Actually the PR wasn't rejected, it was closed by its author (I don't know why). Would you be willing to make a new PR with its contents?

#814

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 a pull request may close this issue.

3 participants