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

Distribution wrappers in handlers.pyro should be deprecated in favor of defterm #203

Open
eb8680 opened this issue Jan 15, 2025 · 2 comments · May be fixed by #257
Open

Distribution wrappers in handlers.pyro should be deprecated in favor of defterm #203

eb8680 opened this issue Jan 15, 2025 · 2 comments · May be fixed by #257
Labels
enhancement New feature or request usability

Comments

@eb8680
Copy link
Contributor

eb8680 commented Jan 15, 2025

It should be possible to replace PositionalDistribution with defterm and defdata implementations for TorchDistributions. The same things would also serve as a replacement for NamedDistribution, though we'd need an extra Operation there since the basic Distribution API doesn't support __getitem__ or an equivalent.

@eb8680
Copy link
Contributor Author

eb8680 commented Jan 31, 2025

#214 makes some progress toward this but doesn't yet fully resolve it.

I think the general solution would be a pair of operations bind_dims and unbind_dims in handlers.torch that subsume handlers.torch.to_tensor and positional_distribution/named_distribution from #214:

T = TypeVar("T", bound=torch.Tensor | torch.distributions.Distribution | tree.Structure[torch.Tensor | torch.distributions.Distribution)

@defop
def bind_dims(
  value: Annotated[T, Scoped[A | B]], 
  *names: Annotated[Operation[[], int], Scoped[B]]
) -> Annotated[T, Scoped[A]]:
  ...

@defop
def unbind_dims(
  value: Annotated[T, Scoped[A]],
  *names: Annotated[Operation[[], int], Scoped[B]]
) -> Annotated[T, Scoped[A | B]]:
  ...

bind_dims is basically the same as our current to_tensor extended with logic for handling call. unbind_dims is a little trickier because it shouldn't distribute over operations that bind positional dimensions.

@eb8680 eb8680 removed this from the Post-release cleanup milestone Jan 31, 2025
@eb8680
Copy link
Contributor Author

eb8680 commented Jan 31, 2025

Removing from the milestone since #214 is sufficient for now and the general solution would be quite a bit more effort.

@jfeser jfeser removed their assignment Feb 4, 2025
@jfeser jfeser linked a pull request Mar 18, 2025 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request usability
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants