-
Notifications
You must be signed in to change notification settings - Fork 129
Add rewrites for inv(diag(x)) and inv(eye) #898
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
96c9cbf
to
d91c5fe
Compare
d5d48fc
to
1db9999
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #898 +/- ##
=======================================
Coverage 81.69% 81.70%
=======================================
Files 182 182
Lines 47585 47626 +41
Branches 11584 11601 +17
=======================================
+ Hits 38875 38913 +38
- Misses 6520 6521 +1
- Partials 2190 2192 +2
|
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.
Getting close! Can you also add a test of cases that should not be rewritten, for example eye with k!=0
. Check the different off-ramps in the rewrites and make sure they happen.
@@ -3,6 +3,7 @@ | |||
from typing import cast | |||
|
|||
from pytensor import Variable | |||
from pytensor import tensor as pt |
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.
Import what you need directly from the various packages within pytensor.tensor
, it makes the code easier to follow for people in the future
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.
ah that makes sense. i'll do that from next time cuz the test file has way too many uses of pt rn 😓
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.
Small change, then I think this is ready
Description
Added rewrites for inv(diag(x)) = 1/x and rewrites inv(eye) -> eye
Related Issue
Checklist
Type of change