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

weighted quantile bug fix #134

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

Conversation

iii-org-tw
Copy link

Original code has two problems:

  1. When normwt = FALSE and weight sum < 1.
    image

This line goes wrong. The indexing becomes descending.

  1. It sees the lowest index as 1.
    image

For example:

> data <- 1:3
> weights<- c(0.1, 0.1, 0.8)
> probs=0.09
 
 wtd.quantile(x=data, weights=weights, probs=probs, normwt=TRUE)
9% 
3 

It is because the cumsum is 0.3, 0.6, 3.0 and the approx function try to locate lower index 1 and upper index 2.
It turns both q take the 3 value and the interpolation crush.

I find one formula to define weighted quantile.
https://stats.stackexchange.com/questions/13169/defining-quantiles-over-a-weighted-sample
It is well defined in boundary and the result equals to numpy quantile when weights are equal.
I implement the code accordingly and this one doesn't have the above mentioned problem.

Any review is welcome.

@iii-org-tw
Copy link
Author

Hi @harrelfe

Do you have time to review this PR?

@harrelfe
Copy link
Owner

Would you mind doing some tests on how this changes behavior in the non-breaking case? Also I would need changes in the help file to go along with these changes. Thanks for working on this.

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.

3 participants