Check that concentration parameters of Dirichlet distribution are all > 0

[AlexAndorra]

As dicussed with @junpenglao, the Dirichlet distribution doesn't check that the concentration parameters (a vector) are all strictly positive when the distribution is initialized.

Currently, the user can define pm.Dirichlet("p", a=np.array([-1, 2, 3]), shape=3) in a model and PyMC won't complain before the user tries to sample from the model (BadInitialEnergy Error).

As the concentration parameters have to be > 0, this PR just checks that it is the case before the user can do anything else.
Thanks in advance for the review, and I'm there for any comment/change. PyMCheers ✌️

[codecov]

Codecov Report

Merging #3853 into master will decrease coverage by 0.30%.
The diff coverage is 93.33%.

@@            Coverage Diff             @@
##           master    #3853      +/-   ##
==========================================
- Coverage   90.69%   90.39%   -0.31%     
==========================================
  Files         135      135              
  Lines       21190    21202      +12     
==========================================
- Hits        19219    19165      -54     
- Misses       1971     2037      +66     

Impacted Files	Coverage Δ
pymc3/distributions/multivariate.py	78.96% <83.33%> (+0.03%)	⬆️
pymc3/tests/test_distributions.py	96.45% <100.00%> (-1.70%)	⬇️
pymc3/tests/models.py	70.24% <0.00%> (-15.71%)	⬇️
pymc3/tests/test_step.py	93.20% <0.00%> (-6.80%)	⬇️
pymc3/step_methods/hmc/base_hmc.py	93.45% <0.00%> (-1.87%)	⬇️
pymc3/tests/test_transforms.py	98.52% <0.00%> (-1.48%)	⬇️
pymc3/tests/test_mixture.py	98.95% <0.00%> (-0.70%)	⬇️
pymc3/distributions/continuous.py	79.69% <0.00%> (-0.41%)	⬇️

[AlexAndorra]

I'm not sure I understand CodeCov output here: does this mean I have to modify something in all of the mentioned "Impacted files"?

[twiecki]

Does it work if the input is a RV?

[ColCarroll]

This looks like just a buggy report. I wouldn't worry.

[ColCarroll]

(note: thomas' question is still a good one)

[AlexAndorra]

Thanks @ColCarroll -- kind of a relief 😅
@twiecki, do you mean if the a vector is a probability distribution?

[twiecki]

Yes, exactly.

[AlexAndorra]

The condition I added to check strict positiveness is only for arrays -- so no; I should add this case. I suppose positiveness is already taken care of; I just have to figure out how to add the "strict" 😉

[AlexAndorra]

Still getting esoteric CodeCov warnings, but the tests passed 🎉
Turns out the strict positiveness was a problem only when a was not an RV, so I restricted the check to only those cases.
I'll also enabled the possibility to specify a as a list in addition to a numpy array, and I added some checks on a's type.
Happy to hear what you guys think 🖖

[twiecki]

@AlexAndorra A test would be good.

[AlexAndorra]

Is think there already are tests, aren't there? In test_distributions.py and test_distributions_random.py

[twiecki]

@AlexAndorra Apparently not of passing in negative values, otherwise that test would have failed I think.

[AlexAndorra]

Ow ok -- sorry I'm not well versed in testing.
Very busy this week but I'll take a look ASAP!

[twiecki]

=================================== FAILURES ===================================
_________________ TestMatchesScipy.test_dirichlet_init_fail[3] _________________
self = <pymc3.tests.test_distributions.TestMatchesScipy object at 0x7f441e645c88>
n = 3
    @pytest.mark.parametrize('n', [3, 4])
    def test_dirichlet_init_fail(self, n):
        with Model():
            with pytest.raises(ValueError) as err:
                _ = Dirichlet('x', a=np.zeros(n), shape=n)
>           err.match("All concentration parameters (a) must be > 0.")
E           AssertionError: Pattern 'All concentration parameters (a) must be > 0.' does not match 'All concentration parameters (a) must be > 0.'
pymc3/tests/test_distributions.py:959: AssertionError
_________________ TestMatchesScipy.test_dirichlet_init_fail[4] _________________
self = <pymc3.tests.test_distributions.TestMatchesScipy object at 0x7f4415c37f28>
n = 4
    @pytest.mark.parametrize('n', [3, 4])
    def test_dirichlet_init_fail(self, n):
        with Model():
            with pytest.raises(ValueError) as err:
                _ = Dirichlet('x', a=np.zeros(n), shape=n)
>           err.match("All concentration parameters (a) must be > 0.")
E           AssertionError: Pattern 'All concentration parameters (a) must be > 0.' does not match 'All concentration parameters (a) must be > 0.'
pymc3/tests/test_distributions.py:959: AssertionError

[AlexAndorra]

Thanks guys, this was very helpful!
Tests passed this time (notwithstanding the buggy codecov) 🍾

[twiecki]

Thanks!