Skip to content

Update pymc.TruncatedNormal docstring #5546

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.

Sign up for GitHub

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

Merged
OriolAbril merged 7 commits into from
Mar 12, 2022
+11 −8
Merged

Update pymc.TruncatedNormal docstring #5546

Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
df1850c
fixed TruncateNormal docsting
purna135 Mar 4, 2022
4f6fff4
Merge remote-tracking branch 'upstream/main' into doc_continuous_Trun…
purna135 Mar 6, 2022
4a12878
fixed TruncateNormal docsting
purna135 Mar 6, 2022
c972983
removed sd from dist
purna135 Mar 10, 2022
016b88e
Merge remote-tracking branch 'upstream/main' into doc_continuous_Trun…
purna135 Mar 10, 2022
d36d34c
undo the sd change
purna135 Mar 12, 2022
fbf1190
Merge remote-tracking branch 'upstream/main' into doc_continuous_Trun…
purna135 Mar 12, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions pymc/distributions/continuous.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -673,14 +673,14 @@ class TruncatedNormal(BoundedContinuous):

Parameters
----------
mu: float
mu : tensor_like of float, optional
Mean.
Comment thread
purna135 marked this conversation as resolved.
sigma: float
sigma : tensor_like of float, optional
Standard deviation (sigma > 0).
lower: float (optional)
Left bound.
upper: float (optional)
Right bound.
lower : tensor_like of float, optional
Left bound. Defaults to 0.

@OriolAbril OriolAbril Mar 5, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the defaults are wrong (same for upper). I'd bet the default is no bound or infinity or somehting of the sort

@purna135 purna135 Mar 5, 2022

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am unable to find out the default value for mu and sigma.
could you please guide me, how to identify these dafult value so that it can help me for other methords.

@OriolAbril OriolAbril Mar 5, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First check the description in the docstring, sometimes the defaults are incorrectly explained in the extended summary section. If they are not you should read the .dist method and follow the variables defined there and/or function calls to see what the defaults are. Here are some examples, I hope they are illustrative and you can follow the pattern for the rest.

mu

pymc/pymc/distributions/continuous.py

Line 720 in 4223b6e

mu = at.as_tensor_variable(floatX(mu))

it is defined straight away here, so it seems like it is not optional actually. @ricardoV94 is this intended? If the value in the function call were 0 instead of None then you would use the default 0 instead of optional for example, however, if some function is called to get the default, it probably means you should use optional and explain the default (or combinations of defaults depending on other parameters) in the description

lower

pymc/pymc/distributions/continuous.py

Line 724 in 4223b6e

lower = at.as_tensor_variable(floatX(lower)) if lower is not None else at.constant(-np.inf)

the default in the function call is None, and when it is None, what is actually used is - infinity. When a value is given, that value is used. So lower should be lower : tensor_like of float, default -numpy.inf (and check in the readthedocs preview that it gets rendered correctly).

@ricardoV94 ricardoV94 Mar 5, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like the default of mu should indeed be zero, not None

@ricardoV94 ricardoV94 Mar 5, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

transform as a positional argument is outdated an should be removed from the dist signature

upper : tensor_like of float, optional
Right bound. Defaults to 1.

Examples
--------
Expand Down Expand Up @@ -762,9 +762,9 @@ def logp(

Parameters
----------
value: numeric
value : tensor_like of float
Value(s) for which log-probability is calculated. If the log probabilities for multiple
values are desired the values must be provided in a numpy array or Aesara tensor
values are desired the values must be provided in a numpy array or Aesara tensor.

Returns
-------
Expand Down