pymc
pymc copied to clipboard
Default rewrites in inner graph
This PR will add implementation of default rewrites for nodes with inner fgraph and related with issue #6697. For now I just added new test cases
:books: Documentation preview :books:: https://pymc--6996.org.readthedocs.build/en/6996/
Codecov Report
Merging #6996 (b8fde47) into main (547bcb4) will decrease coverage by
4.25%. Report is 11 commits behind head on main. The diff coverage is94.11%.
:exclamation: Current head b8fde47 differs from pull request most recent head 0c80a68. Consider uploading reports for the commit 0c80a68 to get more accurate results
Additional details and impacted files
@@ Coverage Diff @@
## main #6996 +/- ##
==========================================
- Coverage 92.17% 87.92% -4.25%
==========================================
Files 101 101
Lines 16849 16891 +42
==========================================
- Hits 15530 14851 -679
- Misses 1319 2040 +721
| Files | Coverage Δ | |
|---|---|---|
| pymc/logprob/utils.py | 96.19% <94.11%> (-1.31%) |
:arrow_down: |
The tests are great!
Logic for removing CheckParameterValues from Scan is very similar to one we recently added in #6873 for moment calculation in Scan, so may be I will move it to some helper function later
There is some refactoring going on in #6976 that will impact the location of these changes.
One thing we need to think about is nested Scans/OpFromGraph, I think this approach will only work for the first level.
PS: Agree with refactoring the functionality if it's so similar
I added recursive check for nested Scans. For now it is only remove CheckParameterValues nodes from fgraph, but I will add replacement by ninf next step
@ricardoV94 I added logic for replacement CheckParameterValues by ninf in Scan and rebased my branch. Now I am going to add support for OpFromGraph and want to extract common logic for rewrites in inner fgraphs - probably some helper function. I think we can add some general solution for inner fgraphs rewriting.
I added support for replacements in OpFromGraph
I move some common logic into InnerGraphRewrite class
@ricardoV94 Could you please take a look when you will have some time?
I will re-implement this with WalkingNestedGraphRewriter after merging https://github.com/pymc-devs/pytensor/pull/556