Do not reject PatternNodeRewriter due unrelated multiple clients

[ricardoV94]

Description

The existing check was to restrictive, because it would reject a rewrite due to completely unrelated multiple clients above the replaced subgraph, as well as multiple uses of the output being replaced, neither of which make sense.

Checklist

• Checked that the pre-commit linting/style checks pass
• Included tests that prove the fix is effective or that the new feature works
• Added necessary documentation (docstrings and/or example notebooks)
• If you are a pro: each commit corresponds to a relevant logical change

Type of change

• New feature / enhancement
• Bug fix
• Documentation
• Maintenance
• Other (please specify):

[jessegrabowski]

Looks fine to me. Do we know the original logic behind not allowing multiple clients? On first blush, it seems like it shouldn't have ever mattered.

[ricardoV94]

Looks fine to me. Do we know the original logic behind not allowing multiple clients? On first blush, it seems like it shouldn't have ever mattered.

That's the default and the point is to avoid rewrites that accidentally split the work and end up with a graph that recomputes the same thing in different ways. This happens when a separate variable in the graph depends on one of the intermediate computations of the pattern being substituted. Those intermediate computations would remain in the graph anyway.

The way they did it was way too crude though. Also not sure if this should be the default, but that can be a discussion for another time.

[ricardoV94]

Actually @jessegrabowski I think the issue wan introduced when the old unification code was scrapped in favor o the external library: 459c570