Prevent rejection sampling if truncated normal variance is 0 #1273
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix #1264
Upon initially investigating this, I discovered that this was not a crash, but instead a process that either does not resolve or a process that takes an extremely long time to resolve. My initial suspicion was that this was an infinite loop. Furthermore, it can be triggered with
rand(truncate(Normal(0,0), x, y))
wherex
andy
are any numbers that are 0 or of different sign, along with some other cases that I won't immediately delve into. I suspect that the mean of the Normal distribution is also irrelevant, but I did not bother testing this.We have a method for
rand(d::Truncated{Normal{T}, Continus})
defined at line 140 oftruncated/normal.jl
(Distributions.jl/src/truncated/normal.jl
Line 140 in 863844c
isfinite(mean(truncated(Normal(0,0), 0, 1)))
correctly evaluates totrue
, bringing us to therandnt
function (although it's worth noting that we're passing exclusivelyNaN
,Inf
, and-Inf
to therandnt
function. This is because we are dividing by the standard deviation, which is 0, in order to standardize the bounds.Within the 2nd part of
randnt
, the first two cases, when triggered, yield acceptable results even for Normal(0,0), e.g. they always yield0.0
within a reasonable amount of time.When neither of these cases are triggered, a third case is triggered which, under a wide variety of
x,y
where our underlying distribution is Normal(0,0) and our bounds arex,y
, never exits. Under these circumstances,rho
is alwaysNaN
and we can only exit the loop whenr < rho
(wherer
is randomly generated).r < NaN
will never evaluate totrue
, so we never exit the loop.However, there's no need to actually delve deep into
randnt
to fix this. We should probably just avoid trying to feedrandnt
the "standardized" values from a 0-variance distribution in the first place.I propose fixing this at the level of
rand(rng::AbstractRNG, d::Truncated{Normal{T},Continuous}) where T <: Real
:We return the mean when a user attempts to sample from a 0-variance truncated normal distribution.