TST: test correctness of tfp-unsupported random variables

[eigenfoo]

Following up from #54 (comment) and #54 (comment).

We rely on tfp to correctly implement many random variables. However, the random variables that we implement ourselves (e.g. the zero-inflated random variables) need to be tested for correctness.

[twiecki]

Aren't they just combinations of TFP distributions (e.g. mixture or transformed)? So if we assume these implementations to be properly tested, we only need to test that we combined things correctly, so I wonder if our existing tests aren't sufficient for this.

[canyon289]

I don't think they're sufficient because we could mess up the combining. Like on this line we could make the mistake of using a minus instead of a plus

https://github.com/pymc-devs/pymc4/blob/master/pymc4/_random_variables.py#L319

Might be good to just write a couple tests to test for numerical values for some points, not just that value is returned

[sshekh]

Hello everyone,

I am new to pymc4 and would like to start working on this issue.

Just to make it clear, the issue is that the current random variable test only checks that the values are not None in line 90 of pymc4/tests/test_random_variables.py but does not check the correctness of values. Line 92-93 checks that vals is close to expected_value when it exists but since expected keyword is not there in any of the _random_variable_args this condition will never be executed.

So we can add checks for tfp-unsupported rvs by adding expected output values for different parameter configurations right? Please correct me if I am wrong 😅 , I cannot understand @canyon289 's comment as the link is broken (permalink would've been better).

Thanks!

[canyon289]

Hey @sshekh
Sorry about not having a permalink.

Right now I would suggest holding off on PyMC4 prs in the RandomVariable module because there are 3 big PRs open (Distribution Transforms, AST parsing, TF2 and TFP 0.7.0)

My suggestion if you want to contribute to PyMC4 is to contribute to PyMC3, specifically in issues that relate to the MCMC samplers. The reason I say that is because after we figure out the three PRs above the next thing will be transferring the MCMC samplers to PyMC4 from PyMC3 and the better understanding you have the easier it will be!

Let me know if what you think!

[sshekh]

Hi @canyon289 , thank you for the prompt reply. I will look for issues in PyMC3 to contribute to. Currently the one related to sampling seems to be pymc-devs/pymc#2509 . Not sure if it's still open though. I will try to look!