Skip to content

Conversation

ricardoV94
Copy link
Member

@ricardoV94 ricardoV94 commented Oct 31, 2024

Closes #1037

It also refactors the two rewrites to avoid so much duplicated code

The fix revealed some subtle bugs in the C implementation of Scalar Ops with negative literal constants, that were also fixed.


📚 Documentation preview 📚: https://pytensor--1059.org.readthedocs.build/en/1059/

@ricardoV94 ricardoV94 added bug Something isn't working graph rewriting labels Oct 31, 2024
@ricardoV94 ricardoV94 changed the title Fix bug in zero mul/div sink rewrite Fix bug in local_[mul|div]_switch_sink rewrite Oct 31, 2024
@ricardoV94 ricardoV94 force-pushed the zero_sink_rewrite_bug branch from 956b73f to faa1abb Compare October 31, 2024 10:24
f = self.function_remove_nan([x], pytensor.gradient.grad(y, x), self.mode)
assert f(5) == 1, f(5)

@pytest.mark.slow
Copy link
Member Author

Choose a reason for hiding this comment

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

It was not particularly slow and not slower than the test above

@ricardoV94 ricardoV94 force-pushed the zero_sink_rewrite_bug branch from faa1abb to ecf9829 Compare October 31, 2024 10:26
@ricardoV94 ricardoV94 changed the title Fix bug in local_[mul|div]_switch_sink rewrite Fix downcasting bug in local_[mul|div]_switch_sink rewrite Oct 31, 2024
@ricardoV94 ricardoV94 force-pushed the zero_sink_rewrite_bug branch from ecf9829 to e664da8 Compare October 31, 2024 11:58
@ricardoV94 ricardoV94 force-pushed the zero_sink_rewrite_bug branch from 2dffc91 to 096f285 Compare October 31, 2024 12:28
Copy link

codecov bot commented Oct 31, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.95%. Comparing base (6132203) to head (096f285).
Report is 97 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1059      +/-   ##
==========================================
- Coverage   81.95%   81.95%   -0.01%     
==========================================
  Files         182      182              
  Lines       47872    47857      -15     
  Branches     8618     8619       +1     
==========================================
- Hits        39233    39219      -14     
  Misses       6472     6472              
+ Partials     2167     2166       -1     
Files with missing lines Coverage Δ
pytensor/scalar/basic.py 80.52% <100.00%> (ø)
pytensor/scalar/loop.py 88.88% <100.00%> (ø)
pytensor/tensor/rewriting/math.py 89.82% <100.00%> (-0.05%) ⬇️
pytensor/tensor/variable.py 87.85% <100.00%> (+0.02%) ⬆️

@ricardoV94 ricardoV94 merged commit 4f7d709 into pymc-devs:main Oct 31, 2024
61 checks passed
@ricardoV94 ricardoV94 deleted the zero_sink_rewrite_bug branch October 31, 2024 22:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working C-backend graph rewriting

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Downcasting in local_[mul|div]_switch_sink rewrite

2 participants