Extend basic matcher to handle multiple-output-nodes #1734
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1734 +/- ##
==========================================
+ Coverage 74.96% 75.01% +0.04%
==========================================
Files 245 245
Lines 26383 26451 +68
Branches 4802 4826 +24
==========================================
+ Hits 19779 19843 +64
+ Misses 5682 5676 -6
- Partials 922 932 +10 ☔ View full report in Codecov by Sentry. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Test Results 24 files ± 0 24 suites ±0 3h 18m 22s ⏱️ - 3m 36s For more details on these failures, see this check. Results for commit 27e29b8. ± Comparison against base commit 937558f. This pull request removes 2179 tests.♻️ This comment has been updated with latest results. |
Clarifying to understand better: is it referring to that of the non-match subgraphs or the target pattern? For example, if we have a simple pattern with small outputs and a graph with many outputs, will it still be costly? |
The complexity is sort of So, if we are looking for a pattern |
| @property | ||
| def output_node(self) -> NodePattern: |
There was a problem hiding this comment.
Should this be a method instead? Properties can usually be assigned
| @property | |
| def output_node(self) -> NodePattern: | |
| def output_node(self) -> NodePattern: |
| @@ -706,18 +710,18 @@ def __reversed__(self) -> Iterator[NodePattern]: | |||
|
|
|||
| @property | |||
There was a problem hiding this comment.
| @property |
nit
| if output_values is None: | ||
| return match | ||
| if not _valid_to_replace(match.nodes, output_values): | ||
| return match.fail("Matched nodes have other uses preventing replacement.") |
There was a problem hiding this comment.
Print all uses for info?
This PR extends the basic matcher to handle multiple output nodes. This provides an alternative to the generic-matcher algorithm, which is incomplete and fails in some circumstances. This can also be useful in debugging match-failures (when it is unclear if the failure is valid or due to limitations of the matching algorithm). The drawback is that this algorithm can, in some cases, be expensive, especially when the number of output-nodes is large and the graph size is large. (So far, however, we haven't encountered patterns with more than 2 output-nodes.)