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

Error for default params of NoneType in pm.distributions.distribution._draw_value() #3248

Closed
gilliss opened this issue Nov 9, 2018 · 10 comments

Comments

@gilliss
Copy link
Contributor

gilliss commented Nov 9, 2018

I'm raising this issue following discussion at
https://discourse.pymc.io/t/error-for-default-params-of-nonetype-in-pm-distributions-distribution-draw-value/2183

I came across the following error when using pm.distributions.continuous.TruncatedNormal.dist().random().

ValueError: Unexpected type in draw_value: <class 'NoneType'>

The error seems to be caused by the default values of None for the upper and lower parameters of the BoundedContinuous distribution object. That is,

>>> dist = pm.TruncatedNormal.dist()
>>> type(dist.mu), dist.mu
(<class 'theano.tensor.var.TensorConstant'>, TensorConstant{0})
>>> type(dist.sd), dist.sd
(<class 'theano.tensor.var.TensorConstant'>, TensorConstant{1.0})
>>> type(dist.upper), dist.upper
(<class 'NoneType'>, None)
>>> type(dist.lower), dist.lower
(<class 'NoneType'>, None)
>>> dist.random()
ValueError: ...

If I give a value to just one of 'upper' or 'lower', the error persists. If I give values to both upper and lower, the error is avoided.

>>> dist.lower, dist.upper = 0, np.inf
>>> dist.random()
array(1.15514827)

Am I using the random() method in an unintended way? Perhaps it's just required that the user supply upper and lower bounds to the respective kwargs in TruncatedNormal-- in that case, it may be desirable to make those into positional arguments (matching scipy.stats.truncnorm).

Please provide a minimal, self-contained, and reproducible example.

import pymc3 as pm
pm.TruncatedNormal.dist(lower = 0., upper = 100.).random() # no error
try: pm.TruncatedNormal.dist(lower = 0.).random() # error
except ValueError: print('ValueError')
try: pm.TruncatedNormal.dist(upper = 100.).random() # error
except ValueError: print('ValueError')
try: pm.TruncatedNormal.dist().random() # error
except ValueError: print('ValueError')

Please provide the full traceback.

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Applications/anaconda3/envs/Fit2/lib/python3.6/site-packages/pymc3/distributions/continuous.py", line 578, in random
    [self.mu, self.sd, self.lower, self.upper], point=point, size=size)
  File "/Applications/anaconda3/envs/Fit2/lib/python3.6/site-packages/pymc3/distributions/distribution.py", line 321, in draw_values
    evaluated[param_idx] = _draw_value(param, point=point, givens=givens.values(), size=size)
  File "/Applications/anaconda3/envs/Fit2/lib/python3.6/site-packages/pymc3/distributions/distribution.py", line 418, in _draw_value
    raise ValueError('Unexpected type in draw_value: %s' % type(param))
ValueError: Unexpected type in draw_value: <class 'NoneType'>

Please provide any additional information below.

Versions and main components

  • PyMC3 Version: 3.5
  • Theano Version: 1.0.2
  • Python Version: 3.6.6
  • Operating system: macOS 10.13.2
  • How did you install PyMC3: conda
@junpenglao
Copy link
Member

junpenglao commented Nov 10, 2018

Maybe we can change:
https://github.com/pymc-devs/pymc3/blob/a80440935d96554bf15c92996c3bf6ffb6a43f62/pymc3/distributions/continuous.py#L578-L579
To something like self.lower = tt.as_tensor_variable(lower) if lower is not None else np.inf

@KrUciFieR-Jr
Copy link

I would like to work on this issue

@twiecki
Copy link
Member

twiecki commented Jan 2, 2019

@KrUciFieR-Jr There's a PR that's almost done but seems to have gone stale: #3250 I think it's just missing resolving the last item using a try-except block. You could clone @gilliss's branch, add the commit, and do a separate PR here.

@Harivallabha
Copy link
Contributor

Hi! This issue does seem to have gone stale. I'd like to take it up if no one is currently working on it :) @twiecki

@lucianopaz
Copy link
Contributor

@Harivallabha, sorry, this issue should have been closed after merging #3250.

@lucianopaz lucianopaz reopened this Apr 2, 2019
@lucianopaz
Copy link
Contributor

Oops, actually I got confused. This issue is still open. @Harivallabha, if you're up for the challenge feel free to go ahead.

@Harivallabha
Copy link
Contributor

Yep, thank you!

@gilliss
Copy link
Contributor Author

gilliss commented Apr 2, 2019

Does pull request #3250 not address issue #3248? That's my fault if it doesn't.

@lucianopaz
Copy link
Contributor

@gilliss, sorry, yes. I doubted myself wrongly before. This issue was closed by #3250

@Harivallabha
Copy link
Contributor

Yeah, I just went through #3250 sometime ago, and was wondering about the same xD Seemed to have been resolved :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants