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

Fix need for dummy data in sample_posterior_predictive #7265

Merged
merged 1 commit into from
May 6, 2024

Conversation

ricardoV94
Copy link
Member

@ricardoV94 ricardoV94 commented Apr 17, 2024

Description

Fix arviz backend find_constants

This also fixes a bug when doing sample_posterior_predictive with resized X data, where the old unused Y data would be retrieved and a dimension mismatch would crop up, unless a dummy Y was set prior to sampling.

Related Issue

  • Closes #
  • Related to #

Checklist

Type of change

  • New feature / enhancement
  • Bug fix
  • Documentation
  • Maintenance
  • Other (please specify):

📚 Documentation preview 📚: https://pymc--7265.org.readthedocs.build/en/7265/

@ricardoV94 ricardoV94 force-pushed the fix_dummy_data branch 2 times, most recently from 94cc61d to 309139b Compare May 3, 2024 08:07
@ricardoV94 ricardoV94 marked this pull request as ready for review May 3, 2024 08:07
@ricardoV94 ricardoV94 added trace-backend Traces and ArviZ stuff samplers labels May 3, 2024
@ricardoV94 ricardoV94 requested a review from OriolAbril May 3, 2024 08:09
@ricardoV94 ricardoV94 added the bug label May 3, 2024
@ricardoV94 ricardoV94 changed the title Fix need for dummy data in sample_posterior_predictive Fix need for dummy data in sample_posterior_predictive May 3, 2024
@@ -454,7 +454,7 @@ def test_constant_data(self, use_context):
test_dict = {
"posterior": ["beta"],
"observed_data": ["obs"],
"constant_data": ["x", "y", "beta_sigma"],
"constant_data": ["x", "beta_sigma"],
Copy link
Member Author

Choose a reason for hiding this comment

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

This y was in fact observed data... Now nothing stops the user from using y both as observed data and constant data, so I am not sure we want to truly remove it.

We can check if y shows up in the generative graph.

Copy link
Member Author

Choose a reason for hiding this comment

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

doing it now

Copy link
Contributor

@zaxtax zaxtax left a comment

Choose a reason for hiding this comment

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

LGTM

This avoids needing to set dummy observed data when doing sample_posterior_predictive when that is not part of the generative graph.
@jessegrabowski
Copy link
Member

Let's get this merged? The CI failure looks unrelated.

@ricardoV94 ricardoV94 merged commit 3a884dd into pymc-devs:main May 6, 2024
18 of 20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug samplers trace-backend Traces and ArviZ stuff
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants