Skip to content

Don't mask inputs to pm.Data containers #6189

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

Conversation

michaelosthege
Copy link
Member

What is this PR about?
I ran into a problem where NaN values in my data got masked away when creating pm.Data containers.
This was quite unexpected.

We need the masking for observed, but that's being taken care of in other places already.

Checklist

Major / Breaking Changes

  • NaN values passed to pm.Data are no longer masked automatically.

Bugfixes / New features

  • None

Docs / Maintenance

  • None

@michaelosthege michaelosthege self-assigned this Oct 7, 2022
@codecov
Copy link

codecov bot commented Oct 7, 2022

Codecov Report

Merging #6189 (dd5a1af) into main (244c37d) will decrease coverage by 0.02%.
The diff coverage is 98.76%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #6189      +/-   ##
==========================================
- Coverage   93.04%   93.01%   -0.03%     
==========================================
  Files          91      101      +10     
  Lines       20813    22092    +1279     
==========================================
+ Hits        19365    20549    +1184     
- Misses       1448     1543      +95     
Impacted Files Coverage Δ
pymc/aesaraf.py 89.66% <ø> (-3.88%) ⬇️
pymc/model.py 88.10% <ø> (-0.14%) ⬇️
pymc/parallel_sampling.py 85.76% <ø> (-0.05%) ⬇️
pymc/tests/backends/test_ndarray.py 100.00% <ø> (ø)
pymc/tests/smc/test_smc.py 100.00% <ø> (ø)
pymc/sampling.py 82.63% <85.71%> (-0.02%) ⬇️
pymc/distributions/multivariate.py 92.24% <91.66%> (-0.07%) ⬇️
pymc/stats/convergence.py 93.15% <93.15%> (ø)
pymc/distributions/shape_utils.py 98.66% <97.14%> (-0.29%) ⬇️
pymc/tests/variational/test_inference.py 99.02% <99.02%> (ø)
... and 29 more

@michaelosthege michaelosthege force-pushed the dont-mask-datacontainer-inputs branch from eaaa745 to dd5a1af Compare October 9, 2022 23:13
@michaelosthege
Copy link
Member Author

@ricardoV94 and I discussed this offline and didn't find a good solution that keeps pm.Data inputs unmasked while not breaking imputation or model_to_graphviz at the same time.

Without a good solution, and keeping in mind that @ricardoV94 wants to refactor how we manage observed variables I think we can close this PR.

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.

1 participant