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

THG's substitution applier #334

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from
Draft

THG's substitution applier #334

wants to merge 7 commits into from

Conversation

matthiasdiener
Copy link
Collaborator

@matthiasdiener matthiasdiener commented Jun 1, 2022

Based on 61690f4.

Addresses #162 in part.

Needs:

@matthiasdiener
Copy link
Collaborator Author

This currently fails in some of the codegen tests with

[...]
  File "/Users/mdiener/Work/emirge/pymbolic/pymbolic/mapper/__init__.py", line 153, in __call__
    return method(expr, *args, **kwargs)
  File "/Users/mdiener/Work/emirge/pytato/pytato/target/loopy/codegen.py", line 645, in map_reduce
    inner_expr = self.rec(inner_expr, prstnt_ctx,
  File "/Users/mdiener/Work/emirge/pymbolic/pymbolic/mapper/__init__.py", line 153, in __call__
    return method(expr, *args, **kwargs)
  File "/Users/mdiener/Work/emirge/pymbolic/pymbolic/mapper/__init__.py", line 615, in map_substitution
    child = self.rec(expr.child, *args, **kwargs)
  File "/Users/mdiener/Work/emirge/pymbolic/pymbolic/mapper/__init__.py", line 153, in __call__
    return method(expr, *args, **kwargs)
  File "/Users/mdiener/Work/emirge/pytato/pytato/target/loopy/codegen.py", line 586, in map_subscript
    return result.to_loopy_expression(self.rec(expr.index, prstnt_ctx,
  File "/Users/mdiener/Work/emirge/pymbolic/pymbolic/mapper/__init__.py", line 151, in __call__
    return self.map_foreign(expr, *args, **kwargs)
  File "/Users/mdiener/Work/emirge/pymbolic/pymbolic/mapper/__init__.py", line 206, in map_foreign
    return self.map_tuple(expr, *args, **kwargs)
  File "/Users/mdiener/Work/emirge/pymbolic/pymbolic/mapper/__init__.py", line 583, in map_tuple
    children = tuple([self.rec(child, *args, **kwargs) for child in expr])
  File "/Users/mdiener/Work/emirge/pymbolic/pymbolic/mapper/__init__.py", line 583, in <listcomp>
    children = tuple([self.rec(child, *args, **kwargs) for child in expr])
  File "/Users/mdiener/Work/emirge/pymbolic/pymbolic/mapper/__init__.py", line 153, in __call__
    return method(expr, *args, **kwargs)
  File "/Users/mdiener/Work/emirge/pytato/pytato/target/loopy/codegen.py", line 605, in map_variable
    array = local_ctx.lookup(expr.name)
  File "/Users/mdiener/Work/emirge/pytato/pytato/target/loopy/codegen.py", line 165, in lookup
    return self.local_namespace[name]
KeyError: '_r0'

@kaushikcfd
Copy link
Collaborator

Thanks for bringing this back to life! Also, I'm curious are we currently running into any cases where this could be profitable?

@kaushikcfd
Copy link
Collaborator

kaushikcfd commented Jun 1, 2022

For context, on some large expression graphs implemented in drivers-y2-isolator, we obtain this profile -- https://gist.github.com/kaushikcfd/0867bd549acfff43781077e8f8b0ba56 spanning around 8 minutes for walltime, and I don't see substitute(..) accounting for a significant amount in the flamegraph.

@matthiasdiener
Copy link
Collaborator Author

matthiasdiener commented Jun 2, 2022

I think illinois-ceesd/mirgecom#645 was a case where we saw extremely long times spent in substitute.

@kaushikcfd
Copy link
Collaborator

kaushikcfd commented Jun 2, 2022

I think illinois-ceesd/mirgecom#645 was a case where we saw extremely long times spent in substitute.

Aah! (Thanks!)
After trying to run the example, it still seems to take a very long time to compile. At this point, it seems to me that there are some mappers that aren't using memorized variants. (c.f. inducer/loopy#625). Seems like there are more mappers like the one in loopy.realize_reduction that do not use the memoized variants. At this point I don't have enough data to blame the substitute here, if you already have some data, please do share.

@matthiasdiener matthiasdiener self-assigned this Aug 22, 2023
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 this pull request may close these issues.

2 participants