Skip to content

Prior predictions constant data #5723

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

Merged

Conversation

TimOliverMaier
Copy link
Contributor

This is my suggestion for issue #5722
With this minor changes, pm.sample_prior_predictive() returns an InferenceData object, that
contains the constant_data group, useful for e.g. plotting purposes of prior predictions before sampling.

@codecov
Copy link

codecov bot commented Apr 21, 2022

Codecov Report

Merging #5723 (1588e1f) into main (d322f75) will decrease coverage by 0.02%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #5723      +/-   ##
==========================================
- Coverage   88.92%   88.90%   -0.03%     
==========================================
  Files          75       75              
  Lines       13739    13738       -1     
==========================================
- Hits        12218    12214       -4     
- Misses       1521     1524       +3     
Impacted Files Coverage Δ
pymc/backends/arviz.py 90.28% <ø> (-0.04%) ⬇️
pymc/parallel_sampling.py 86.71% <0.00%> (-1.00%) ⬇️

@TimOliverMaier
Copy link
Contributor Author

I don't know why the test is failing on windows. The failing test test_step_continuous passes on my system (Ubuntu 20.04). Can somebody tell me if the test is failing because of my additions or if there is a problem with the test?

@@ -464,7 +468,7 @@ def observed_data_to_xarray(self):
default_dims=[],
)

@requires(["trace", "predictions"])
@requires(["trace", "predictions", "prior_predictions"])
Copy link
Member

Choose a reason for hiding this comment

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

I think this prior predictions attribute is not needed, generating the constant data group here should be enough. IIUC, removing this line completely and leaving everything else untouched should be enough

@@ -1042,6 +1042,28 @@ def point_list_arg_bug_fixture() -> Tuple[pm.Model, pm.backends.base.MultiTrace]


class TestSamplePriorPredictive(SeededTest):
def test_idata_output(self):
Copy link
Member

Choose a reason for hiding this comment

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

I think this test should be in idata_conversion. And I would try to avoid having a new test but instead checking for constant data in existing tests where it is currently missing

@TimOliverMaier
Copy link
Contributor Author

Yes, you are right. Thank you for the review. I pushed the changes.

@OriolAbril OriolAbril merged commit 162ad6c into pymc-devs:main Apr 22, 2022
@OriolAbril
Copy link
Member

Thanks @TimOliverMaier !

fonnesbeck added a commit that referenced this pull request May 6, 2022
* Refactor get_tau_sigma to handle lists

* Fixed syntax error

* Black formatted

* Change assertions to ValueError raises

* Prior predictions constant data (#5723)

* support return constant_data with prior pred

* pre-commit format

* added test

* code review

* Fix issue probably-meant-fstring found at https://codereview.doctor (#5726)

* Add coords argument to pymc.set_data (#5588)


Co-authored-by: Oriol (ZBook) <[email protected]>

* remove MultinomialRV override

* ⬆️ UPGRADE: Autoupdate pre-commit config

* Group GaussianRandomWalk tests in single class

* Infer steps from shape in GaussianRandomWalk

* Unpin upper limit on numpydoc

Closes #5401

* ⬆️ UPGRADE: Autoupdate pre-commit config

* Standardize docstrings of input dists arguments and add warning about cloning

* Remove unnecessary tag in Simulator logp

* Replace deprecated tag.ignore_logprob

* Group compile_pymc tests in own class

* Remove remaining uses of default_updates in codebase

* Remove redundant/wrong docstrings from GaussianRandomWalk logp

* Add moment to GaussianRandomWalk and fix mu/sigma broadcasting bug

* Do not create temporary SymbolicDistribution just to retrieve number of RNGs needed

Reordered methods for consistency

* Move SymbolicDistribution docstring to body of class

* Refactor AR distribution

* Deprecates AR1 distribution
* Implements random and moment methods
* Batching works on the left as with other distributions

* Rename `pandas_to_array` to `convert_observed_data`

* Obtain step information from dims and observed

* Make AR steps extend shape beyond initial_dist

This is consistent with the meaning of steps in the GaussianRandomWalk and translates directly to the number of scan steps taken

Co-authored-by: TimOliverMaier <[email protected]>
Co-authored-by: code-review-doctor <[email protected]>
Co-authored-by: Somasree Majumder <[email protected]>
Co-authored-by: Oriol (ZBook) <[email protected]>
Co-authored-by: danhphan <[email protected]>
Co-authored-by: twiecki <[email protected]>
Co-authored-by: Ricardo <[email protected]>
Co-authored-by: Ravin Kumar <[email protected]>
Co-authored-by: Michael Osthege <[email protected]>
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.

2 participants