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

Consider removing old shape handling functions #6107

Closed
Tracked by #7053
ricardoV94 opened this issue Sep 7, 2022 · 5 comments
Closed
Tracked by #7053

Consider removing old shape handling functions #6107

ricardoV94 opened this issue Sep 7, 2022 · 5 comments
Labels
beginner friendly help wanted needs info Additional information required

Comments

@ricardoV94
Copy link
Member

ricardoV94 commented Sep 7, 2022

Now that random draws are done by Aesara, we might not need some/all of these anymore:

"shapes_broadcasting",
"broadcast_dist_samples_shape",
"get_broadcastable_dist_samples",
"broadcast_distribution_samples",
"broadcast_dist_samples_to",

First step would be to check which ones are still used and where.

@ricardoV94 ricardoV94 added beginner friendly help wanted needs info Additional information required labels Sep 7, 2022
@Armavica
Copy link
Member

As far as I can say,

  • shapes_broadcasting is only used in broadcast_dist_samples_shape and get_broadcastable_dist_samples,
  • broadcast_dist_samples_shape is only used in get_broadcastable_dist_samples,
  • get_broadcastable_dist_samples is only used in broadcast_distribution_samples and broadcast_dist_samples_to,
  • broadcast_distribution_samples is not used anywhere,
  • broadcast_dist_samples_to is the only of these functions used somewhere else in the code, and only in MatrixNormalRV.

@ricardoV94
Copy link
Member Author

So we should check if we can get something more immediate to replace broadcast_dist_samples_to in MatrixNormalRV? I haven't looked at what it is doing

@mattiadg
Copy link
Contributor

mattiadg commented Oct 17, 2022

Is this still an open issue? I am interested in working on it

@thomasaarholt
Copy link
Contributor

This issue can be closed, all the functions mentioned in OP are removed.

@ricardoV94
Copy link
Member Author

This issue can be closed, all the functions mentioned in OP are removed.

Thanks!

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

No branches or pull requests

4 participants