Remove samples and keep_size from sample_posterior_predictive

[pibieta]

Removed samples and keep_size from sample_posterior_predictive in sampling.py

In response to issue #5775, we removed the samples and keep_size parameters on the sample_posterior_predictive function. Consequently, we removed all the calls to these two variables inside the functions and edited the code such that it runs as keep_size was always True even though the variable is not actually defined.

Regarding the samples parameter, it is defined by the trace object inside the function, so there was no need for it be a function parameter.

Moreover, we edited the all test functions that used these two parameters.

Pre-commit and linting tests were passed.

#DataUmbrellaPyMCSprint
P.D.: Thanks for all the help to @OriolAbril !
cc: @vitaliset @OriolAbril @reshamas

[codecov]

Codecov Report

Merging #6029 (37f3ebf) into main (eaa51f3) will decrease coverage by 0.00%.
The diff coverage is 93.33%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6029      +/-   ##
==========================================
- Coverage   93.58%   93.58%   -0.01%     
==========================================
  Files         101      101              
  Lines       22151    22136      -15     
==========================================
- Hits        20731    20716      -15     
  Misses       1420     1420              

Impacted Files	Coverage Δ
pymc/tests/backends/test_arviz.py	99.00% <ø> (-0.03%)	⬇️
pymc/sampling.py	82.48% <87.50%> (-0.16%)	⬇️
pymc/tests/distributions/test_mixture.py	99.35% <100.00%> (ø)
pymc/tests/distributions/test_simulator.py	99.50% <100.00%> (ø)
pymc/model.py	88.26% <0.00%> (+0.03%)	⬆️

[OriolAbril]

Thanks!

I took a quick look at the test failures. They are due to the lack of chain dimension in the asserts in many/most cases:

• https://github.com/pymc-devs/pymc/runs/7692438573?check_suite_focus=true#step:7:1501 for test TestMixture.test_single_poisson_predictive_sampling_shape
• https://github.com/pymc-devs/pymc/runs/7692438573?check_suite_focus=true#step:7:1577 for test TestMixture.test_list_mvnormals_predictive_sampling_shape
• https://github.com/pymc-devs/pymc/runs/7692439777?check_suite_focus=true#step:7:250 for test TestSamplePPC.test_normal_scalar

Can you take a look at the logs and see if you can update the asserts on shapes that need to be modified?

[pibieta]

Thanks for looking at the logs, @OriolAbril. I will take care of that during the weekend!

[ricardoV94]

Looks great, I left some minor suggestions

[reshamas]

@OriolAbril @pibieta @vitaliset
Do we know why the tests are failing here?

[pibieta]

Yes, we have an idea @reshamas . I will work on it today, I just didn't have time to work on this PR.

[cluhmann]

@pibieta @vitaliset Let me know if you want me to rehash what we discussed on Friday. Thanks again for your contribution(s)!

[reshamas]

Hi @pibieta @vitaliset,
Let us know if you have any questions on this PR.

[cluhmann]

I think there was a question (probably for @ricardoV94 ) about the p-value in one failing test. That was at least 1 remaining thing. There may have been others.

[ricardoV94]

Which test is it? Is the test failing systematically when running locally?

[OriolAbril]

Thanks for your work @pibieta and @vitaliset, I hope you don't mind I took over the PR to get it over the finish line. I will do further changes to sample_posterior_predictive (see #6208 ) and having merged this will make things a bit easier. Moreover, my other PR would have introduced many more git conflicts if merged before this one, making it even more hard to get this PR back to good shape and ready to merge.

[pibieta]

Thanks for taking over this PR, @OriolAbril ! We were actually having trouble to fix the tests, so I think the best was for you to take over, haha. We will be working on other issues from now on. Thanks again, Oriol!