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

Documentation mistake?: implementation of varcorrection is different from the comment in weights.jl #784

Closed
phasetr opened this issue Apr 6, 2022 · 4 comments · Fixed by #785

Comments

@phasetr
Copy link

phasetr commented Apr 6, 2022

The problem is here.

"""
    varcorrection(w::UnitWeights, corrected=false)
* `corrected=true`: ``\\frac{n}{n - 1}``, where ``n`` is the length of the weight vector
* `corrected=false`: ``\\frac{1}{n}``, where ``n`` is the length of the weight vector
This definition is equivalent to the correction applied to unweighted data.
"""
@inline function varcorrection(w::UnitWeights, corrected::Bool=false)
    corrected ? (1 / (w.len - 1)) : (1 / w.len)

The comment say that we get \frac{n}{n - 1} if corrected=true, but the implementation is (1 / (w.len - 1)). Which is true? Implementation is right, I think.

@nalimilan
Copy link
Member

The docstring says "where n is the length of the weight vector", so these are the same, right? This docstring only applies to UnitWeights.

@phasetr
Copy link
Author

phasetr commented Apr 6, 2022

The number n is w.len, I think.

nalimilan added a commit that referenced this issue Apr 7, 2022
This didn't match the implementation. Fixes #784.
@nalimilan
Copy link
Member

Ah sorry I had misunderstood your comment. This is indeed a typo (probably a copy/paste). See #785.

@phasetr
Copy link
Author

phasetr commented Apr 7, 2022

Thank you for your quick responce! I close this issue.

@phasetr phasetr closed this as completed Apr 7, 2022
nalimilan added a commit that referenced this issue Apr 7, 2022
This didn't match the implementation. Fixes #784.
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.

2 participants