Open quildtide opened 3 months ago
Upon further reflection, I believe that the least-breaking change is to:
NotDegenerate
wrapper or singleton to dispatch to methods that follow current behaviorThis way, the default behavior becomes what users think the current behavior is, while users who really want performance and can guarantee that they aren't using degenerate cases can explicitly ask for current-ish behavior.
Long rambling introduction
Distributions.jl currently allows many weird edge-cases which result in Dirac-type degenerate point mass distributions.
There are numerous bugs related to many of these cases, e.g.
Some time ago, I looked into fixing the edge cases where
Beta
can turn degenerate. The problem is that many methods which were previously arbitrarily simple (likemean(d::Beta) = ((α, β) = params(d); α / (α + β))
) become much longer, like:Even worse is when methods with currently-constant outputs stop being constants as a result. For example,
minimum(Normal(0,0))
returns-Inf
; it dispatches to a method defined by a macro,@distr_support Normal -Inf Inf
. Effectively,minimum(::Normal)
always returns-Inf
, and the compiler is able to make all sorts of optimizations because of this.For example, compare:
Fixing these methods to recognize that
Bernoulli(0.0)
andBinomial(1, 0.0)
have a maximum at 0 would prevent these kinds of compiler optimizations.The Problem
Distributions.jl currently "supports" a massive amount of edge-cases which result in degenerate distributions. Many methods return incorrect results on these degenerate cases, BUT fixing these rare edge cases would have tangible performance impact on normal usage for people who aren't leveraging the degenerate distributions.
For some functions like
rand
, fixing edge-cases sorand(d::Normal)
and friends don't cause infinite loops seems like a totally reasonable thing to do, sincerand
is already such a costly function. For smaller functions likeminimum
, I think there is a legitimate discussion to be had.Potential Solutions
Honestly, I have no clue. I would have submitted a pull req first if I had any strong opinion on how these issues should be resolved.
There is a part of me that thinks that most Distributions should stop supporting their degenerate cases, and we should implement some kind of wrapper like
Degenerable{D} = Union{D, Dirac} where D <: Distribution
. In this case, we would probably want to give deprecation warnings on things likeNormal(0, 0)
and note that accuracy is non-guaranteed.Alternatively, it might also make sense to have default behavior support degenerate cases and give correct results, even when this yields performance penalties. Prior behavior (with its performance improvements) could be reenabled via alternative methods like
minimum(d::Distribution, ::NotDegenerate)
with a singleton typeNotDegenerate
or something. Alternatively, it should theoretically be possible to implementNotDegenerate{D} where D <: Distribution
as a wrapper instead.