-
Notifications
You must be signed in to change notification settings - Fork 186
Rewrite det(inv(X)) → 1/det(X) #2102
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -112,7 +112,7 @@ | |
| from pytensor.tensor.rewriting.blockwise import blockwise_of | ||
| from pytensor.tensor.rewriting.elemwise import apply_local_dimshuffle_lift | ||
| from pytensor.tensor.shape import Shape, Shape_i, specify_shape | ||
| from pytensor.tensor.subtensor import Subtensor | ||
| from pytensor.tensor.subtensor import Subtensor, _is_provably_positive | ||
| from pytensor.tensor.type import ( | ||
| complex_dtypes, | ||
| uint_dtypes, | ||
|
|
@@ -689,6 +689,95 @@ def local_exp_log_nan_switch(fgraph, node): | |
| return [new_out] | ||
|
|
||
|
|
||
| @register_canonicalize | ||
| @register_stabilize | ||
| @register_specialize | ||
| @node_rewriter([log]) | ||
| def local_log_reciprocal(fgraph, node): | ||
| """Rewrite log(reciprocal(x)) -> -log(x).""" | ||
| (inp,) = node.inputs | ||
| if ( | ||
| inp.owner | ||
| and isinstance(inp.owner.op, Elemwise) | ||
| and isinstance(inp.owner.op.scalar_op, ps.Reciprocal) | ||
| ): | ||
| return [neg(log(inp.owner.inputs[0]))] | ||
|
|
||
|
|
||
| @register_canonicalize | ||
| @register_stabilize | ||
| @register_specialize | ||
| @node_rewriter([log]) | ||
| def local_log_div(fgraph, node): | ||
| """Rewrite log(a / b) -> log(a) - log(b) when a or b is provably positive. | ||
|
|
||
| The provably-positive side is typically a constant or a shape, which the | ||
| surrounding pipeline constant-folds. | ||
| """ | ||
| (inp,) = node.inputs | ||
| if not ( | ||
| inp.owner | ||
| and isinstance(inp.owner.op, Elemwise) | ||
| and isinstance(inp.owner.op.scalar_op, ps.TrueDiv) | ||
| ): | ||
| return None | ||
|
|
||
| num, den = inp.owner.inputs | ||
| if _is_provably_positive(num, strict=True) or _is_provably_positive( | ||
| den, strict=True | ||
| ): | ||
| return [log(num) - log(den)] | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think complexity wise we only want to do this if one is a constant so we replace 1 log and 1 division by 1 log and one subtraction, not 2 logs and one subtraction |
||
|
|
||
|
|
||
| @register_canonicalize | ||
| @register_stabilize | ||
| @register_specialize | ||
| @node_rewriter([sign]) | ||
| def local_sign_reciprocal(fgraph, node): | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would combine the reciprocal and div cases in these rewrites, conceptually the same |
||
| """Rewrite sign(reciprocal(x)) -> sign(x).""" | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same here sign a/b, where one is a positive constant -> sign of the other term. If the constant is negative, 1-sign of the other. If it's mixed, can't do anything |
||
| (inp,) = node.inputs | ||
| if ( | ||
| inp.owner | ||
| and isinstance(inp.owner.op, Elemwise) | ||
| and isinstance(inp.owner.op.scalar_op, ps.Reciprocal) | ||
| ): | ||
| return [sign(inp.owner.inputs[0])] | ||
|
|
||
|
|
||
| @register_canonicalize | ||
| @register_stabilize | ||
| @register_specialize | ||
| @node_rewriter([sign]) | ||
| def local_sign_div(fgraph, node): | ||
| """Rewrite sign(a / b) using a known-sign numerator or denominator. | ||
|
|
||
| Provably positive side -> ``sign(other)``; negative constant side -> | ||
| ``-sign(other)``. Bails out otherwise. | ||
| """ | ||
| (inp,) = node.inputs | ||
| if not ( | ||
| inp.owner | ||
| and isinstance(inp.owner.op, Elemwise) | ||
| and isinstance(inp.owner.op.scalar_op, ps.TrueDiv) | ||
| ): | ||
| return None | ||
|
|
||
| num, den = inp.owner.inputs | ||
|
|
||
| if _is_provably_positive(num, strict=True): | ||
| return [sign(den)] | ||
| if _is_provably_positive(den, strict=True): | ||
| return [sign(num)] | ||
|
|
||
| for side, other in ((num, den), (den, num)): | ||
| try: | ||
| val = get_underlying_scalar_constant_value(side) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Doesn't need to be scalar, I would just check for constant and then all |
||
| except NotScalarConstantError: | ||
| continue | ||
| if np.all(val < 0): | ||
| return [neg(sign(other))] | ||
|
|
||
|
|
||
| @register_canonicalize | ||
| @register_specialize | ||
| @node_rewriter([Sum]) | ||
|
|
||
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.
we should do the more general as well (reciprocal is fine): log(a/b), where a or b is a non-negative constant -> log(a) - log(b) (the constant constant-folded already).