Skip to content

Don't allow Deterministics in the graph of observed #7769

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

Open
ricardoV94 opened this issue Apr 28, 2025 · 2 comments
Open

Don't allow Deterministics in the graph of observed #7769

ricardoV94 opened this issue Apr 28, 2025 · 2 comments
Labels
bug fgraph model needs info Additional information required

Comments

@ricardoV94
Copy link
Member

ricardoV94 commented Apr 28, 2025

Description

In #7656 we allow PyTensor deterministic operations in the graph of observed variables, but we probably don't want to allow PyMC determinstics. Doing that makes model cloning failing, for starters:

import pymc as pm

with pm.Model() as m:
    x_data = pm.Data("x_data", [0, 1, 2])
    y_data = pm.Deterministic("y", x_data + 1)
    y = pm.Normal("y", observed=y_data)

m.clone()  # ValueError: Variable name y already exists.

I am not sure about the implications of mixing the two, I suggest we be conservative and don't allow it for now, unless a good reason crops up. Also a Deterministic like this would be wastefully saved in every iteration of pm.sample!

CC @williambdean

@williambdean
Copy link
Contributor

williambdean commented May 8, 2025

You are using the same name for the Deterministic and the likelihood. Is that intended?

For example, this works:

import pymc as pm

from pymc.model.fgraph import clone_model

with pm.Model() as m:
    x_data = pm.Data("x_data", [0, 1, 2])
    # Change
    y_data = pm.Deterministic("y_data", x_data + 1)
    y = pm.Normal("y", observed=y_data)


m_clone = clone_model(m)

@ricardoV94
Copy link
Member Author

@williambdean that means my example above was not a MWE unfortunately. @cetagostini found the original bug and I don't think it was a dumb name replication because the error only happened when we cloned the model. It was a similar error message that's why I didn't careful check if my MWE was failing at the right point

@ricardoV94 ricardoV94 added the needs info Additional information required label May 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug fgraph model needs info Additional information required
Projects
None yet
Development

No branches or pull requests

2 participants