Update docstrings of pm.set_data and model.Data #6087

bwengals · 2022-08-31T20:53:35Z

Adds docstrings describing how to track changing shape of MutableData in the likelihood. Meant to address issue #5987. The recommended solution is then to set the shape=x.shape, like this:

import pymc as pm                                                                       
with pm.Model() as model:                                                               
    x = pm.MutableData('x', [1., 2., 3.])                                               
    y = pm.MutableData('y', [1., 2., 3.])                                               
    beta = pm.Normal('beta', 0, 1)                                                      
    obs = pm.Normal('obs', x * beta, 1, observed=y, shape=x.shape) # <--- here                      
    idata = pm.sample(1000, tune=1000)   

with model:                                                                             
    pm.set_data({'x': [5., 6., 9., 12., 15.]})                                               
    y_test = pm.sample_posterior_predictive(idata)                                      
y_test.posterior_predictive['obs'].mean(('chain', 'draw'))

Major / Breaking Changes

None

Bugfixes / New features

None

Docs / Maintenance

Add recommendation to docstrings relevant to prediction and posterior predictive sampling.

…ing posterior predictive sampling

canyon289 · 2022-08-31T21:14:05Z

pymc/model.py

@@ -1849,6 +1849,13 @@ def point_logps(self, point=None, round_vals=2):
 def set_data(new_data, model=None, *, coords=None):
    """Sets the value of one or more data container variables.

+    When doing prediction or posterior predictive sampling, make sure that


whats the difference between prediction and posterior predictive sampling?

Nothing.. wanted to catch the eye of someone skimming with either word. Maybe better phrasing here?

I would just drop the first prediction mention.

"make sure" is also a bit strong, as it's not required, it's more of a convenience. And maybe also say at the end that the benefit of this is that you only have to set_data() x, but not both.

I would also add an example of resizing y alone first. MutableData is not useful only in posterior predictive sampling with GLMs where this concern for having to resize both variables arises.

It's also useful to fit the same model to different datasets.

Would be strange for me as user seeing this being discussed in the only example.

@twiecki that makes sense, I'll change it.

@ricardoV94 For an example with resizing y alone first, would y be a pm.MutableData object, and observed? Makes sense RE fitting the same model to different datasets. In the past I've just remade the model object but you could save on compiling time.

codecov · 2022-08-31T21:16:35Z

Codecov Report

Merging #6087 (163e2fa) into main (0b191ad) will increase coverage by 2.59%.
The diff coverage is n/a.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6087      +/-   ##
==========================================
+ Coverage   89.54%   92.14%   +2.59%     
==========================================
  Files          72       92      +20     
  Lines       12929    21061    +8132     
==========================================
+ Hits        11577    19406    +7829     
- Misses       1352     1655     +303

Impacted Files	Coverage Δ
pymc/data.py	`80.08% <ø> (ø)`
pymc/model.py	`88.23% <ø> (ø)`
pymc/variational/inference.py	`84.97% <0.00%> (-1.13%)`	⬇️
pymc/printing.py	`86.08% <0.00%> (-0.58%)`	⬇️
pymc/distributions/logprob.py	`97.31% <0.00%> (-0.42%)`	⬇️
pymc/distributions/shape_utils.py	`98.95% <0.00%> (-0.33%)`	⬇️
pymc/distributions/mixture.py	`95.40% <0.00%> (-0.32%)`	⬇️
pymc/distributions/simulator.py	`87.23% <0.00%> (-0.27%)`	⬇️
pymc/distributions/continuous.py	`97.50% <0.00%> (-0.01%)`	⬇️
... and 55 more

junpenglao · 2022-09-14T09:54:16Z

Is this ready to merge?

OriolAbril · 2022-09-23T05:50:08Z

pymc/data.py

@@ -594,6 +594,11 @@ def Data(
    To set the value of the data container variable, check out
    :func:`pymc.Model.set_data`.


Suggested change

:func:`pymc.Model.set_data`.

:meth:`pymc.Model.set_data`.

OriolAbril · 2022-09-23T05:51:09Z

pymc/data.py

+    When making predictions or doing posterior predictive sampling, the shape of the
+    registered data variable will most likely need to be changed.  If you encounter an
+    Aesara shape mismatch error, refer to the documentation for
+    :func:`pymc.model.set_data`.


Suggested change

:func:`pymc.model.set_data`.

:func:`pymc.set_data`.

The function is imported as pymc.set_data and since 4.0 we document objects in the same place they should be imported.

OriolAbril · 2022-09-23T05:51:30Z

pymc/model.py

+    the random variable acting as the likelihood (`observed` is True) is dynamically
+    resized when the data is updated.  Set the `shape` argument of the random
+    variable acting as the likelihood to the shape of the relevant
+    :class:`pymc.data.MutableData` object, as shown in the example below


Suggested change

:class:`pymc.data.MutableData` object, as shown in the example below

:class:`pymc.MutableData` object, as shown in the example below

bwengals · 2022-09-26T21:23:36Z

Took another crack at it, basically made the docstring more minimal and added an example as per your suggestion @ricardoV94.

Would it be nice to also have the syntax allow pm.set_data(y=y) as well as pm.set_data({'y': y})? a la xarray?

ricardoV94

Looks great

ricardoV94 · 2022-09-27T04:54:14Z

Would it be nice to also have the syntax allow pm.set_data(y=y) as well as pm.set_data({'y': y})? a la xarray?

I guess so ;)

bwengals · 2022-09-28T18:48:25Z

One last question, if you don't mind, I can't seem to figure out why pre-commit isn't happy. Since this PR just changes docstrings, what's it saying about numpy, positional arguments?

ricardoV94 · 2022-09-28T19:38:04Z

That was fixed today. If you rebase from main it should go away

update docstrings to explain how to avoid aesara shape errors when do…

dec1c3f

…ing posterior predictive sampling

bwengals mentioned this pull request Aug 31, 2022

Prediction could be better if shape of observed would not need to be considered #5987

Closed

canyon289 reviewed Aug 31, 2022

View reviewed changes

canyon289 approved these changes Aug 31, 2022

View reviewed changes

OriolAbril reviewed Sep 23, 2022

View reviewed changes

Rewrite docstring for pm.set_data, fix other comments.

163e2fa

ricardoV94 approved these changes Sep 27, 2022

View reviewed changes

michaelosthege merged commit faebc60 into pymc-devs:main Oct 7, 2022

		@@ -594,6 +594,11 @@ def Data(
		To set the value of the data container variable, check out
		:func:`pymc.Model.set_data`.

	:class:`pymc.data.MutableData` object, as shown in the example below
	:class:`pymc.MutableData` object, as shown in the example below

Uh oh!

Update docstrings of pm.set_data and model.Data #6087

Update docstrings of pm.set_data and model.Data #6087

Uh oh!

Conversation

bwengals commented Aug 31, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Major / Breaking Changes

Bugfixes / New features

Docs / Maintenance

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ricardoV94 Sep 1, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Aug 31, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

junpenglao commented Sep 14, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bwengals commented Sep 26, 2022

Uh oh!

ricardoV94 left a comment

Choose a reason for hiding this comment

Uh oh!

ricardoV94 commented Sep 27, 2022

Uh oh!

bwengals commented Sep 28, 2022

Uh oh!

ricardoV94 commented Sep 28, 2022

Uh oh!

Uh oh!

bwengals commented Aug 31, 2022 •

edited

Loading

ricardoV94 Sep 1, 2022 •

edited

Loading

codecov bot commented Aug 31, 2022 •

edited

Loading