-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Fix MatrixNormal.random #4368
Fix MatrixNormal.random #4368
Conversation
Codecov Report
@@ Coverage Diff @@
## master #4368 +/- ##
==========================================
+ Coverage 87.95% 88.07% +0.11%
==========================================
Files 88 88
Lines 14493 14476 -17
==========================================
+ Hits 12748 12750 +2
+ Misses 1745 1726 -19
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow, much simpler like this!
I addressed all the suggestions above. The PR is ready for another round of review. |
The new code looks much better! I can't comment on the math behind these changes, as I'm not familiar enough with it. |
Does this PR branch need a rebase with current master? |
No, just release notes. |
Thanks @Sayam753! |
Thank your for opening a PR!
Depending on what your PR does, here are a few things you might want to address in the description:
Fixes Bug in MatrixNormal.sample #3585
These random methods are amazing 🤩 . Just broadcast everything before hand and then go for any calculations.
Yes. I have added a related test.