Skip to content

Do not propagate dims to observed component of imputed variable #6263

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
merged 4 commits into from
Nov 4, 2022

Conversation

ricardoV94
Copy link
Member

@ricardoV94 ricardoV94 commented Nov 3, 2022

Closes #6177

Also did some refactoring and more informative errors for when imputation is not supported

Major / Breaking Changes

  • ...

Bugfixes / New features

  • Fix bug where dims of partially observed variables would be propagated to the smaller observed sub-component.

Docs / Maintenance

  • ...

@ricardoV94 ricardoV94 changed the title Do not reuse dims for observed component of imputed variable Do not propagate dims to observed component of imputed variable Nov 3, 2022
@ricardoV94 ricardoV94 force-pushed the imputation_dims branch 2 times, most recently from df38c8a to fca51f0 Compare November 3, 2022 14:55
@codecov
Copy link

codecov bot commented Nov 3, 2022

Codecov Report

Merging #6263 (ebcdf0c) into main (48664c0) will decrease coverage by 5.61%.
The diff coverage is 75.00%.

❗ Current head ebcdf0c differs from pull request most recent head f196c0e. Consider uploading reports for the commit f196c0e to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #6263      +/-   ##
==========================================
- Coverage   94.19%   88.57%   -5.62%     
==========================================
  Files         102      102              
  Lines       21477    21480       +3     
==========================================
- Hits        20230    19026    -1204     
- Misses       1247     2454    +1207     
Impacted Files Coverage Δ
pymc/model.py 79.72% <75.00%> (-9.75%) ⬇️
pymc/tests/step_methods/test_compound.py 0.00% <0.00%> (-100.00%) ⬇️
pymc/tests/distributions/test_timeseries.py 0.00% <0.00%> (-99.51%) ⬇️
pymc/tests/step_methods/hmc/test_quadpotential.py 0.00% <0.00%> (-95.82%) ⬇️
pymc/distributions/timeseries.py 28.46% <0.00%> (-66.00%) ⬇️
pymc/model_graph.py 15.29% <0.00%> (-63.53%) ⬇️
pymc/data.py 47.96% <0.00%> (-32.12%) ⬇️
pymc/step_methods/hmc/quadpotential.py 73.76% <0.00%> (-6.94%) ⬇️
pymc/distributions/logprob.py 91.09% <0.00%> (-6.17%) ⬇️
... and 3 more

Copy link
Member

@michaelosthege michaelosthege left a comment

Choose a reason for hiding this comment

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

Just a bit of nitpicks about our testing code..

@ricardoV94
Copy link
Member Author

ricardoV94 commented Nov 4, 2022

Uh, it seems tests don't rerun when the PR has already been approved? Had to re-request a review to trigger them

@ricardoV94 ricardoV94 merged commit 9c313cb into pymc-devs:main Nov 4, 2022
@ricardoV94 ricardoV94 deleted the imputation_dims branch June 6, 2023 03:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Impute results in mismatch dimensions in dims and data
2 participants