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

Negative Binomial parameter wrong #263

Open
colinfang opened this issue Nov 4, 2014 · 5 comments
Open

Negative Binomial parameter wrong #263

colinfang opened this issue Nov 4, 2014 · 5 comments

Comments

@colinfang
Copy link

        /// <summary>
        /// Initializes a new instance of the <see cref="NegativeBinomial"/> class.
        /// </summary>
        /// <param name="r">The number of failures (r) until the experiment stopped. Range: r ≥ 0.</param>
        /// <param name="p">The probability (p) of a trial resulting in success. Range: 0 ≤ p ≤ 1.</param>
        public NegativeBinomial(double r, double p)
        {
            if (!IsValidParameterSet(r, p))
            {
                throw new ArgumentException(Resources.InvalidDistributionParameters);
            }

            _random = SystemRandomSource.Default;
            _p = p;
            _trials = r;
        }

So here p is said to be success chance.

However, here:

 /// <summary>
        /// Gets the mean of the distribution.
        /// </summary>
        public double Mean
        {
            get { return _trials*(1.0 - _p)/_p; }
        }

According to wiki, it should be _trials * _p / (1 - _p) if we follow the description of the parameter.

It seems throughout the code we interpret _p as 1 - _p

@colinfang
Copy link
Author

Also could someone check the Sample method? It produces non-sense as well.

@cdrnet
Copy link
Member

cdrnet commented Nov 6, 2014

Thanks for pointing this out, will have a look at it.

Sample: the actual sampling distribution shape is indeed not currently verified in the unit tests (commented out, also Poisson and ConwayMaxwellPoisson). We must cover all of them properly.

@cdrnet
Copy link
Member

cdrnet commented Dec 25, 2014

I finally had a quick first look at this - the talk page at that wikipedia article is quite interesting, there seems to be a lot of confusion, mostly on slightly different definitions of the parameters and the support. And yes, there seems to be a mismatch between our implementation and its own inline documentation.

@cdrnet
Copy link
Member

cdrnet commented Dec 7, 2016

Related: #455

@Arlofin
Copy link
Contributor

Arlofin commented Sep 29, 2022

The description was partially fixed in 9fbd27f, however the description in the F# interface is still wrong.

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

3 participants