NegativeBinomial distribution sample generator wrapper

[fearghuskeeble]

Per #3864 I added a wrapper for the generating samples in NegativeBinomial. This is already used in ZeroInflatedNegativeBinomial and other distributions which require operations on the scipy arguments.

Also fixed an error typo in sampling.

[codecov]

Codecov Report

Merging #3866 into master will decrease coverage by 0.05%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #3866      +/-   ##
==========================================
- Coverage   90.73%   90.68%   -0.06%     
==========================================
  Files         135      135              
  Lines       21190    21190              
==========================================
- Hits        19227    19216      -11     
- Misses       1963     1974      +11     

Impacted Files	Coverage Δ
pymc3/distributions/discrete.py	74.65% <100.00%> (+0.14%)	⬆️
pymc3/sampling.py	84.52% <100.00%> (-0.10%)	⬇️
pymc3/tests/test_distributions_random.py	98.51% <100.00%> (+<0.01%)	⬆️
pymc3/tests/test_sampling.py	99.64% <100.00%> (ø)
pymc3/tuning/scaling.py	57.40% <0.00%> (-12.97%)	⬇️
pymc3/backends/report.py	90.62% <0.00%> (-2.35%)	⬇️
pymc3/backends/base.py	87.25% <0.00%> (-0.39%)	⬇️

[twiecki]

Thanks! Why wasn't this caught by a test? Can we add one that would have caught it?

[ColCarroll]

Posterior predictive tests are not at all comprehensive. Adding a test would be nice if @fearghuskeeble has time.

[rpgoldman]

Please test on both sample_posterior_predictive and fast_sample_posterior_predictive. You'll see examples of testing both in the existing tests (I think in test_sampling).

[fearghuskeeble]

I'm not clear as to the relevance of posterior predictive sampling here.

In terms of the prior sampling, the testing is unfortunately a little opaque to me but I will have a look...

[lucianopaz]

@twiecki, it wasn't caught because there are no tests that try to sample from the prior when one of the distribution parameters is a scalar and the other is a vector.

@fearghuskeeble, take a look at test_distributions_random.py. The tests there are structured in a relatively obscure way, so I recommend you just look at how we added a single extra test for the categorical distribution and follow that pattern to add test in the negative binomial's test class.
Oh, and don't forget to add a line in the release notes.md fine at the project's root under maintenance.

[fearghuskeeble]

@lucianopaz I looked back at #3509 and added a test to your nested sampling class.

[lucianopaz]

Great work! I can't merge this from my phone but I'll do it once I get a chance to log into my computer

[twiecki]

Thanks @fearghuskeeble!