Skip to content
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

ENH: Consider representing observed scaled RVs directly in the forward graph #6332

Closed
ricardoV94 opened this issue Nov 22, 2022 · 0 comments · Fixed by #6523
Closed

ENH: Consider representing observed scaled RVs directly in the forward graph #6332

ricardoV94 opened this issue Nov 22, 2022 · 0 comments · Fixed by #6523
Labels

Comments

@ricardoV94
Copy link
Member

ricardoV94 commented Nov 22, 2022

When we create a scaled RV (in relation to MiniBatching) we add an extra piece of information rvs_to_total_sizes (potentially renamed in #6331) in the Model that we use when obtaining the logp. It would be neat if we didn't have to do this, and could instead represent the RV itself as being a MiniBatchedRV or whatever (don't know if there is a mathematical concept for what we are trying to do here), which may take the total_size as an extra input.

This way we would obtain the right logp without the over-head of managing this extra mapping at the Model level. The fewer mappings we need to maintain the easier it is to transform Models, as in pymc-devs/pymc-extras#91


Relevant code sections:

pymc/pymc/model.py

Lines 750 to 756 in b4c7eb1

rv_logps = _joint_logp(
rvs=rvs,
rvs_to_values=self.rvs_to_values,
rvs_to_transforms=self.rvs_to_transforms,
rvs_to_total_sizes=self.rvs_to_total_sizes,
jacobian=jacobian,
)

total_size = rvs_to_total_sizes.get(rv, None)
if total_size is not None:
scaling = _get_scaling(total_size, value_var.shape, value_var.ndim)
logp_term *= scaling

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant