Skip to content
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

Equality comparison of DAGs can have exponential complexity #163

Closed
inducer opened this issue Oct 12, 2021 · 3 comments · Fixed by #166
Closed

Equality comparison of DAGs can have exponential complexity #163

inducer opened this issue Oct 12, 2021 · 3 comments · Fixed by #166

Comments

@inducer
Copy link
Owner

inducer commented Oct 12, 2021

Every time expressions are reused, they are re-traversed, since __eq__ has no mechanism/means to memoize that expressions previously compared equal. (And we probably wouldn't want such a cache on the instance. Or maybe we do? Not sure.) I'm thinking we could introduce an EqualityComparisonMapper that would facilitate such memoization. To avoid having the cat biting its tail, it obviously couldn't use the arrays themselves as hash keys (since that would require equality comparison). But id() is available, and we could back that up with a is b as a check. An (IMO manageably) unfortunate consequence of this would be that we'd have to reimplement equality comparison of all types that can contain Array types, notably dict.

@MTCam is hitting this with cache retrievals upon freeze when exercising lazy evaluation for chemistry in https://github.com/illinois-ceesd/mirgecom.

cc @kaushikcfd

@kaushikcfd
Copy link
Collaborator

(And we probably wouldn't want such a cache on the instance.

It's better to not memoize such instances as there will be an equality comparison in accessing the cache dict itself.

I'm taking on implementing the EqualityComparisonMapper, if anyone else is already on it please let me know.

The demand for #160 seems to be growing :).

@inducer
Copy link
Owner Author

inducer commented Oct 12, 2021

I'm taking on implementing the EqualityComparisonMapper, if anyone else is already on it please let me know.

Sounds good. (And thanks!) I meant to do it yesterday, but I only got as far as typing up all these issues. I haven't yet started.

@kaushikcfd
Copy link
Collaborator

kaushikcfd commented Oct 13, 2021

Here's an example for which we can observe the exponential complexity:

def construct_intestine_graph(depth=20, seed=0):
    from numpy.random import default_rng

    rng = default_rng(seed)

    x = pt.make_placeholder("x", shape=(10,), dtype=float)

    for i in range(depth):
        coeff1, coeff2 = rng.integers(0, 10, 2)
        x = coeff1 * x + coeff2 * x

    return x

graph1 = construct_intestine_graph()
graph2 = construct_intestine_graph()

assert graph1 == graph2

The comparison operation is exponential in depth.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants