mathnet / mathnet-numerics

Math.NET Numerics
http://numerics.mathdotnet.com
MIT License
3.51k stars 898 forks source link

Negative Binomial parameter wrong #263

Open colinfang opened 10 years ago

colinfang commented 10 years ago
        /// <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 commented 10 years ago

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

cdrnet commented 10 years ago

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 commented 9 years ago

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 commented 7 years ago

Related: #455

Arlofin commented 2 years ago

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