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_variance_impl divides by zero if the first weight is zero #44

Open
jukofyork opened this issue Aug 4, 2020 · 0 comments
Open

Comments

@jukofyork
Copy link

jukofyork commented Aug 4, 2020

Hi,

lazy_weighted_variance_impl works fine if the first weight is zero, but weighted_variance_impl divides by zero:

        if(cnt > 1)
        {
            extractor<MeanFeature> const some_mean = {};

            result_type tmp = args[parameter::keyword<Tag>::get()] - some_mean(args);

            this->weighted_variance =
                numeric::fdiv(this->weighted_variance * (sum_of_weights(args) - args[weight]), sum_of_weights(args))
              + numeric::fdiv(tmp * tmp * args[weight], sum_of_weights(args) - args[weight] );
        }

It looks like "cnt > 1" should instead check "sum_of_weights(args)>0", but I'm not sure as there is some mention of this in cosurgi's answer here:

https://stackoverflow.com/questions/16761965/weighted-variance-and-weighted-standard-deviation-in-c

and this change may end up breaking existing code that takes this into account...

If this is left as is, perhaps the documentation needs updating as currently it just says:

"Note that the sample variance is not defined for n<=1".

https://www.boost.org/doc/libs/1_73_0/doc/html/boost/accumulators/impl/weighted_variance_impl.html

Juk

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

No branches or pull requests

1 participant