You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
I now fear that there might be more issues in the implementation of Empirical, rendering it unusable in contexts where I require independency between distributions.
Also the shaping of the output seems strange, which is (n, k), while I would expect it to be (k, n).
Maybe I also completely misunderstood the documentation.
The text was updated successfully, but these errors were encountered:
Wow, yeah, that's a pretty egregious bug. Thank you for flagging. There's pretty good test coverage of this distribution, including, eg, statistical tests of mean and variance, even in the presence of batch shapes. But nothing checks independence of the samples across batches! Should be a straightforward-ish fix (will need to sample indices for each batch dimension and change the gather to a gather_nd, which is always fun :)).
Are you interested in trying to submit a patch? No pressure, just don't want to duplicate work.
While the documentation states that
this is not the case for the implementation.
As can be seen from this MWE, the samples for every distribution are always the same:
The root cause seems to be at this location, where indices are sampled only once but re-used for every distribution:
probability/tensorflow_probability/python/distributions/empirical.py
Lines 236 to 238 in fbc5ebe
I now fear that there might be more issues in the implementation of Empirical, rendering it unusable in contexts where I require independency between distributions.
Also the shaping of the output seems strange, which is
(n, k)
, while I would expect it to be(k, n)
.Maybe I also completely misunderstood the documentation.
The text was updated successfully, but these errors were encountered: