ZeroInflatedPoisson and ZeroInflatedNegativeBinomial show up as Deterministic on graphviz

[twiecki]

It's probably because they are not pure RandomVariables. We need to update the model_graph logic to accommodate them

[cscheffler]

The NormalMixture RV has the same issue.

with pm.Model() as mixture_model:
    weights = pm.Dirichlet('w', np.ones(2))
    sigma2 = pm.InverseGamma('sigma2', shape=2, alpha=alpha_0, beta=beta_0)
    mu = pm.Normal('mu', shape=2, mu=mu_0, sigma=np.sqrt(sigma2 / nu_0))
    pm.NormalMixture('x', w=weights, mu=mu, sigma=np.sqrt(sigma2), observed=data)

pm.model_to_graphviz(mixture_model)

[ricardoV94]

Any non pure RV shows up as a Deterministic. We need to update the logic to check if a variable is in model.basic_RVs instead of checking if it's a RandomVariable which not all model variables are since we introduced SymbolicDistributions

[larryshamalama]

@ricardoV94 bringing the discussion back here.

Short summary: While attempting to refactor graphviz support for SymbolicRVs in #6149, we realized that it would be good to use the distribution's name as input rather than what we have available in _print_name for Aesara random variables. This is because SymbolicRVs use distributions as input (e.g. TruncatedRV, CensoredRV, MixtureRV, etc.); therefore, PyMC-like random variable classes were introduce to provide descriptive _print_names in #6219. However, a dispatching approach to graphviz display would be better in the longer run, hence reverting the previously mentioned PR in #6249.

pymc/pymc/model_graph.py

Lines 157 to 178 in 2dc49e5

	elif v.owner and isinstance(v.owner.op, RandomVariable):
	shape = "ellipse"
	if hasattr(v.tag, "observations"):
	# observed RV
	style = "filled"
	else:
	shape = "ellipse"
	style = None
	symbol = v.owner.op.__class__.__name__
	if symbol.endswith("RV"):
	symbol = symbol[:-2]
	label = f"{var_name}\n~\n{symbol}"
	else:
	shape = "box"
	style = None
	label = f"{var_name}\n~\nDeterministic"
	kwargs = {
	"shape": shape,
	"style": style,
	"label": label,
	}

Should we dispatch the creation of kwargs for graphviz nodes? Would it be better to create classmethods within distribution classes or dispatch functions outside of them (akin to dispatching of moments and logps)

[ricardoV94]

The part where we define symbol should be done with dispatching, the rest can stay the same. This is what will allow flexibility for SymbolicRVs to provide fancy names based on their inputs. Vanilla RandomVariables will always return just Normal or whatever, independent of their parameters.

It should be done with dispatching following the approach taken in the rest of the library

[ricardoV94]

The issue of making it a clever name is separate from showing nodes as RVs instead of Deterministic though, we should open a separate issue for that if it doesn't exist already