-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Allow draws from Weibull
, MvStudentT
, LKJCorr
and LKJCholeskyCovRV
in alternative backends
#7685
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
Conversation
431666f
to
5e57d03
Compare
5e57d03
to
b5de436
Compare
Weibull
, MvStudentT
, LKJCorr
and LKJCholeskyCovRV
can now be sampled in alternative backends
Weibull
, MvStudentT
, LKJCorr
and LKJCholeskyCovRV
can now be sampled in alternative backends Weibull
, MvStudentT
, LKJCorr
and LKJCholeskyCovRV
in alternative backends
b5de436
to
02a5a36
Compare
I added an option to forward the decomposition method to the underlying MvNormal. This will require the next PyTensor release |
374e582
to
b7f4fc5
Compare
b7f4fc5
to
999d4a2
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #7685 +/- ##
==========================================
+ Coverage 83.15% 92.83% +9.67%
==========================================
Files 107 107
Lines 18313 18320 +7
==========================================
+ Hits 15228 17007 +1779
+ Misses 3085 1313 -1772
🚀 New features to boost your workflow:
|
7dd770b
to
48ffc97
Compare
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.
Skimmed.
z = z / pt.sqrt(pt.einsum("ij,ij->i", z, z.copy()))[..., np.newaxis] | ||
P = P[..., 0:mp1, mp1].set(pt.sqrt(y[..., np.newaxis]) * z) | ||
P = P[..., mp1, mp1].set(pt.sqrt(1.0 - y)) | ||
C = pt.einsum("...ji,...jk->...ik", P, P.copy()) |
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.
Would P.mT @ P
just work here? We know P
will always be at least 2d.
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.
Yes
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.
Einsum should arrive at the same thing though
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.
Sure, and I love einsum more than the average person, and actually find it more readable. Just curious what you thought about the alternative
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.
The .mt @ is a bit more direct (avoids the OFG that is inlined later), this makes it easier to review the changes :)
This PR converts more RandomVariable Ops to SymbolicRandomVariable (built on top of other RV Ops), which makes them automatically compatible with alternative backends.
Depends on:
pymc-devs/pytensor#1222
pymc-devs/pytensor#1223
📚 Documentation preview 📚: https://pymc--7685.org.readthedocs.build/en/7685/