Skip to content

Conversation

@tillahoffmann
Copy link
Collaborator

This PR adds the Wishart distribution for covariance matrices. It should probably be rebased and reviewed after #1778 is merged.

Copy link
Member

@fehiepsi fehiepsi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @tillahoffmann! It is great to have this distribution. I left initial comments below.

Copy link
Member

@fehiepsi fehiepsi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the late review, @tillahoffmann!

@fehiepsi fehiepsi added this to the 0.15 milestone May 12, 2024
Copy link
Member

@fehiepsi fehiepsi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @tillahoffmann! Just have a comment at the computation of slogdet.

batch_shape = lax.broadcast_shapes(batch_shape, matrix[:-2])
event_shape = lax.broadcast_shapes(event_shape, matrix[-1:])
return batch_shape, event_shape
return batch_shape, event_shape
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this indent intended?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, assert_one_of ensures that exactly one of the matrices is available, and returning early avoids looping over precision_matrix and scale_tril if covariance_matrix is given, for example. Either with/without the indent should work though.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

got it, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants