Skip to content

BUG: Flaky test test_truncation_specialized_op #6384

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

Closed
michaelosthege opened this issue Dec 11, 2022 · 13 comments · Fixed by #6521
Closed

BUG: Flaky test test_truncation_specialized_op #6384

michaelosthege opened this issue Dec 11, 2022 · 13 comments · Fixed by #6521
Labels
bug needs info Additional information required tests

Comments

@michaelosthege
Copy link
Member

Describe the issue:

It failed in #6304 and on main: https://github.com/pymc-devs/pymc/actions/runs/3665014447/jobs/6195812078

Reproduceable code example:

pytest -v pymc/tests/distributions/test_truncated.py::test_truncation_specialized_op

Error message:

FAILED pymc/tests/distributions/test_truncated.py::test_truncation_specialized_op[observed] - assert False
 +  where False = isinstance(<pytensor.tensor.elemwise.Elemwise object at 0x7fdf5924e1a0>, TruncatedNormalRV)
 +    where <pytensor.tensor.elemwise.Elemwise object at 0x7fdf5924e1a0> = Elemwise{identity}(AdvancedIncSubtensor{inplace=False,  set_instead_of_inc=True}.0).op
 +      where Elemwise{identity}(AdvancedIncSubtensor{inplace=False,  set_instead_of_inc=True}.0) = xt.owne

PyMC version information:

main

Context for the issue:

No response

@michaelosthege michaelosthege changed the title BUG: Flyka test test_truncation_specialized_op BUG: Flaky test test_truncation_specialized_op Dec 11, 2022
@ricardoV94
Copy link
Member

ricardoV94 commented Dec 14, 2022

I cannot reproduce the failure locally. Can someone else?

@ricardoV94 ricardoV94 added needs info Additional information required tests labels Dec 14, 2022
@ricardoV94
Copy link
Member

ricardoV94 commented Dec 14, 2022

Failed again in https://github.com/pymc-devs/pymc/actions/runs/3693633480/jobs/6253850355

It is very strange. Looks like the dispatch may be broken? I can't reproduce it locally

@michaelosthege
Copy link
Member Author

Just a wild speculation, but what if somewhere during graph rewrites there's a non-deterministic order of operations? Could happen by iterating over a set, for example.

@ricardoV94
Copy link
Member

No, the part of the test that failed is that the dispatch of the specialized truncation Op Truncated(NormalRV) -> TruncatedNormalRV didn't happen and it returned the fallback implementation.

@larryshamalama
Copy link
Member

larryshamalama commented Jan 7, 2023

Just to chip in after some weeks: I also still can't reproduce this bug locally... I found out that it just failed recently again here

@larryshamalama
Copy link
Member

@michaelosthege With your recent PR merged, this test seems to now pass...? Did you make any change to the test or the code?

https://github.com/pymc-devs/pymc/actions/runs/4084608130/jobs/7041511557#step:7:34

@ricardoV94
Copy link
Member

This is flaky, I wouldn't read to much to it having passed once

@michaelosthege
Copy link
Member Author

AFAIK this test is still flaky - I just got lucky in my PR and merge. Also, my PR was unrelated

@ricardoV94
Copy link
Member

Found the reason, that tests sets observed=np.empty(100), which sometimes PyMC converts to a maskedarray, and then we get automatic imputation on the TruncatedRV! It's non deterministic, because it depends on the contents of np.empty

@larryshamalama
Copy link
Member

I see the non deterministic aspect of np.empty(100), but why would this inhibit substitution the Op to a TruncatedNormalRV?

@ricardoV94
Copy link
Member

Because when there are nan in the observation the automatic imputation in PyMC is triggered. The variable returned after automatic-imputation is not a pure RV, but a set_subtensor that joins the observed and missing components of the partially observed RV

@larryshamalama
Copy link
Member

Because when there are nan in the observation the automatic imputation in PyMC is triggered. The variable returned after automatic-imputation is not a pure RV, but a set_subtensor that joins the observed and missing components of the partially observed RV

AHHHHHHHH, great that you have figured this out 👏

However, would this potentially cause problems elsewhere when imputing for nans before performing node_rewrites?

@ricardoV94
Copy link
Member

Imputing for nan is the correct behavior. The problem is the test did not expect this to happen (that was not the point of the test)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug needs info Additional information required tests
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants