Potential flaky test: truncation_discrete_random

[OriolAbril]

Description of your problem

The test pymc/tests/distributions/test_truncated.py::test_truncation_discrete_random[rejection-2-inf] failed for #1246 which modifies sample_posterior_predictive only. I think it is (or has become) a flaky test as it did pass in previous commits but failed eventually when the only changes had been to test files to fix tests related to sample_posterior_predictive.

[mattiadg]

#1246 is an issue from 2016, and tests #1246 was successful https://github.com/pymc-devs/pymc/actions/runs/3230416900.
Where can we find the error?

[mattiadg]

This looks different from the other 2 issues about Flaky tests #6210 and #6211.
Here, it looks like the author of the test expected it to pass for any draws https://github.com/mattiadg/pymc/blob/main/pymc/tests/distributions/test_truncated.py#L167-L181
I am not sure if it's correct to always expect to get at least one value equal to the lower bound of the distribution and one equal to the upper bound, but it looks like a strange thing to test only for a specific seed, and it also passed almost every time so far. Maybe this can be a symptom of a small issue in the distribution?

Also, this test is not a SeededTest, which makes me think the idea was exactly that those tests must hold for all possible draws

[ricardoV94]

From the first comment, the condition that fails was probably this one: https://github.com/mattiadg/pymc/blob/7503730dd20d4e7318b31a9834951aae647929d7/pymc/tests/distributions/test_truncated.py#L182-L184

The test intends to check that 3 draws are not sufficient to obtain 500 draws from a Geometric(0.2) that do not include 1 via rejection sampling. This might not have been enough though. I think the probability is supposed to be (1 - 0.2**3) ** 500 = 0.018. That is the probability of not getting 1 three times in a row in 500 independent draws. Anyway, it's probably better to just seed it, and mention the expected probability in case our seed changes in the future and we have to reassess whether it is still working or not.

[mattiadg]

And would you seed it by adding a seed in the function or by creating a new class that derives from Seeded Test? Il lun 17 ott 2022, 11:12 Ricardo Vieira ***@***.***> ha scritto:

…

From the first comment, the condition that fails was probably this one: https://github.com/mattiadg/pymc/blob/7503730dd20d4e7318b31a9834951aae647929d7/pymc/tests/distributions/test_truncated.py#L182-L184 The test intends to check that 3 draws are not sufficient to obtain 500 draws from a Geometric(0.2) that are above 2 via rejection sampling. This might not have been enough though. I think the probability is supposed to be (1 - 0.2**3) ** 500 = 0.018. That is the probability of not getting 1 three times in a row in 500 independent draws. Anyway, it's probably better to just seed it, and mention the expected probability in case our seed changes in the future and we have to reassess whether it is still working or not. — Reply to this email directly, view it on GitHub <#6206 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AA7LDIQXOH5O6E6VTHSYNW3WDUKBPANCNFSM6AAAAAARCWNP4I> . You are receiving this because you commented.Message ID: ***@***.***>

[ricardoV94]

And would you seed it by adding a seed in the function or by creating a new class that derives from Seeded Test? Il lun 17 ott 2022, 11:12 Ricardo Vieira @.***> ha scritto:

Pass a seed to the draw function. The SeededTest is mostly a legacy thing when we used to rely on global seeding internally

[ricardoV94]

Closed via #6229