Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pass size to HalfNormal random method #3686

Merged
merged 2 commits into from
Nov 20, 2019

Conversation

ColCarroll
Copy link
Member

This fixes #3685 -- it was just propagating the size argument to draw_values. I'll do a quick check of other RVs to make sure this is taken care of elsewhere too.

@ColCarroll
Copy link
Member Author

Yep. All other uses include point= and size=, and HalfNormal was just special. I decided not to make that a required argument because Transforms use draw_value without a size argument.

@ColCarroll
Copy link
Member Author

Also, this does not fix the Bound case:

with pm.Model() as model:
    P = pm.Uniform('P', 1, 100)
    sigma_e = pm.Deterministic('sigma_e', 1/P)
    e = pm.Bound(pm.Normal, 0, 1)('e', mu=0, sigma=sigma_e)
    prior = pm.sample_prior_predictive(100_000)

    
values = pm.distributions.draw_values([P, e], size=100_000)

both prior and values are drawn independently. I'll keep thinking about that, but right now I am in favor of merging this so @adrn has a workaround, and I can open a separate issue.

@adrn
Copy link
Contributor

adrn commented Nov 19, 2019

Wow, fast turnaround :)

Thanks @ColCarroll, I'll give this a try tomorrow!

@codecov
Copy link

codecov bot commented Nov 19, 2019

Codecov Report

Merging #3686 into master will decrease coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3686      +/-   ##
==========================================
- Coverage    89.9%   89.89%   -0.01%     
==========================================
  Files         134      134              
  Lines       20166    20176      +10     
==========================================
+ Hits        18130    18137       +7     
- Misses       2036     2039       +3
Impacted Files Coverage Δ
pymc3/distributions/continuous.py 80.85% <100%> (ø) ⬆️
pymc3/exceptions.py 84.21% <0%> (-15.79%) ⬇️
pymc3/ode/utils.py 100% <0%> (ø) ⬆️
pymc3/tests/test_ode.py 100% <0%> (ø) ⬆️
pymc3/ode/ode.py 94.05% <0%> (+0.3%) ⬆️

@lucianopaz
Copy link
Contributor

Looks good to me. Thanks @colcarrol! Could you just add a line to the release notes, under maintenance?

@ColCarroll ColCarroll merged commit a886abc into pymc-devs:master Nov 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

draw_values not generating samples from joint distribution with HalfNormal or Bound(Normal)
3 participants