What's wrong
GeometricDistributionProvider (Essentials.DistributionProviders.Geometric/GeometricDistributionProvider.cs) works with 1 − p directly:
- ~L54–55:
failureProbability = 1.0 - probability; logFailureProbability = Math.Log(failureProbability);
- ~L79:
Cdf = 1.0 - Math.Pow(failureProbability, value + 1.0)
- ~L109:
raw = Math.Ceiling(Math.Log(1.0 - probability) / logFailureProbability) - 1.0
- ~L121: the upward correction loop
while (count < int.MaxValue && Cdf(count) < probability) count++;
For p ≲ 1.1e-16, 1.0 - p rounds to exactly 1:
logFailureProbability becomes 0.
Cdf is 0 and SurvivalFunction is 1 for every k.
- The quantile estimate is
-inf, so count starts at 0.
- The correction loop, whose comment says it "can't run more than once", walks all the way to
int.MaxValue, making about 2.1e9 Math.Pow calls.
For larger small p, Cdf still carries relative error of about eps/p. For example, Geometric(1e-12).Cdf(1e9) returns 9.99478e-4 against a true 9.995002e-4.
Repro
IDistribution<int> g = new GeometricDistributionProvider(1e-17);
g.Cdf(100_000_000); // 0 true ≈ 1e-9
g.Quantile(1e-9); // 2147483647 after ~21.9 s; true ≈ 1e8
_ = g.Median; // ~22.8 s
The constructor accepts any p > 0, so nothing stops this.
Why it matters
Median and Sample (which goes through the inverse CDF) effectively hang a thread for tens of seconds and then return a wrong answer. The CDF is silently wrong for small success probabilities, which is a normal use of a geometric model (waiting for a rare event).
Suggested fix
- Store
log1p(-p) instead of log(1 - p).
Cdf = -expm1((k + 1) * log1p(-p)) and SurvivalFunction = exp((k + 1) * log1p(-p)).
- In
Quantile, use log1p(-probability) / log1p(-p).
- Add accurate
Log1P/ExpM1 helpers to SpecialFunctions, because .NET's double.LogP1/double.ExpM1 are implemented naively as Log(x + 1) / Exp(x) - 1.
- Bound the correction loops, for example to a few steps, falling back to bisection, so an estimate that is off can never turn into a 2e9-step walk.
Acceptance: Geometric(1e-17).Quantile(1e-9) returns ≈ 1e8 in microseconds, and Cdf matches -expm1((k+1)·log1p(-p)) to ~1e-15 relative for p down to 1e-300.
What's wrong
GeometricDistributionProvider(Essentials.DistributionProviders.Geometric/GeometricDistributionProvider.cs) works with1 − pdirectly:failureProbability = 1.0 - probability; logFailureProbability = Math.Log(failureProbability);Cdf = 1.0 - Math.Pow(failureProbability, value + 1.0)raw = Math.Ceiling(Math.Log(1.0 - probability) / logFailureProbability) - 1.0while (count < int.MaxValue && Cdf(count) < probability) count++;For p ≲ 1.1e-16,
1.0 - prounds to exactly 1:logFailureProbabilitybecomes 0.Cdfis 0 andSurvivalFunctionis 1 for every k.-inf, socountstarts at 0.int.MaxValue, making about 2.1e9Math.Powcalls.For larger small p,
Cdfstill carries relative error of about eps/p. For example,Geometric(1e-12).Cdf(1e9)returns 9.99478e-4 against a true 9.995002e-4.Repro
The constructor accepts any p > 0, so nothing stops this.
Why it matters
MedianandSample(which goes through the inverse CDF) effectively hang a thread for tens of seconds and then return a wrong answer. The CDF is silently wrong for small success probabilities, which is a normal use of a geometric model (waiting for a rare event).Suggested fix
log1p(-p)instead oflog(1 - p).Cdf = -expm1((k + 1) * log1p(-p))andSurvivalFunction = exp((k + 1) * log1p(-p)).Quantile, uselog1p(-probability) / log1p(-p).Log1P/ExpM1helpers toSpecialFunctions, because .NET'sdouble.LogP1/double.ExpM1are implemented naively asLog(x + 1)/Exp(x) - 1.Acceptance:
Geometric(1e-17).Quantile(1e-9)returns ≈ 1e8 in microseconds, andCdfmatches-expm1((k+1)·log1p(-p))to ~1e-15 relative for p down to 1e-300.