David Widmann

Results 1463 comments of David Widmann

I don't want to approve and merge my own changes. This does not feel right.

I think the second case, and generally such issues with unbounded discrete distributions where `Int` is not sufficient, would be fixed by https://github.com/JuliaStats/Distributions.jl/pull/1433.

I think we should add tests. I don't want to merge any changes that are untested and reduce coverage. Even if they are correct initially it would go unnoticed if...

How did you compute these values?

Just replacing `Real` with `Number` is not sufficient for `Normal` as the example above shows - `pdf`, `logpdf`, `cdf`, `logcdf`, etc. should all return unitless numbers. Thus unfortunately I assume...

> pdf does have units, namely the reciprocal of the independent variable's. Not in general but only for distributions without atoms - ie. eg. not for `DiscreteDistribution`, `censored` distributions (open...

Another - but probably non-standard - approach would be to choose different base measures (they're all implicit anyway currently) that ensure that `pdf` is always unitless. That shifts the unit...

> It's unfortunate we use pdf to refer to the PDF and the PMF. For a discrete distribution, it's returning probabilities so it should be unit less It's a density...

Since the sufficient statistics are internal unexported implementation details of the `fit!` pipeline, I think the correct approach is rather to add fast paths for `loglikelihood(dist, x)` whenever possible such...

In the Turing example you would then just write something like ```julia x ~ product_distribution(Fill(MvNormal(m, S), n)) ``` and it would use the fast path automatically.