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

Refactor tree_map and replace apply_to_primitive_constituents #1570

Open
wants to merge 69 commits into
base: main
Choose a base branch
from

Conversation

SF-N
Copy link
Contributor

@SF-N SF-N commented Jul 3, 2024

Refactor tree_map and replace apply_to_primitive_constituents (building upon PR#1530)

Description

  • refactors tree_map to be able to handle e.g. tree_map(lambda x: x + 1, ((1, 2), 3)) and adds respective tests
  • renames result_collection_type to result_collection_constructor
  • introduces new alias for utils.tree_map(collection_type=ts.TupleType, result_collection_constructor=lambda x: ts.TupleType(types=[*x]))
  • replaces apply_to_primitive_constituents by calls to tree_map adds respective tests

TODOs

  • Handle apply_to_primitive_constituents with with_path_arg=True
  • pre-commit still failing: src/gt4py/next/type_system/type_info.py:186:17: error: No overload variant of "tree_map" matches argument types "type[TupleType]", "Callable[[Any], TupleType]" [call-overload]
  • merge main and resolve conflicts, especially check license in new files

havogt and others added 26 commits April 18, 2024 17:29
… and run pre-commit (still failing because of src/gt4py/next/type_system/type_info.py:186:17: error: No overload variant of "tree_map" matches argument types "type[TupleType]", "Callable[[Any], TupleType]" [call-overload]

)
@SF-N SF-N requested a review from tehrengruber July 3, 2024 17:01
def tree_map(
*args: Callable[_P, _R],
collection_type: type | tuple[type, ...] = tuple,
result_collection_constructor: Optional[type] = None, # Todo: check name with Enrique
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
result_collection_constructor: Optional[type] = None, # Todo: check name with Enrique
result_collection_constructor: Optional[type] = None,

Name is been approved by Hannes.

tehrengruber added a commit that referenced this pull request Sep 17, 2024
Extends the common subexpression elimination to support the new
itir.Program node and pushes the intermediate Fencil -> Program
conversion upwards the pass manager. The CSE pass now uses the type
inference such that only field expressions or composites thereof are
collected in field-view context (i.e. outside of as_fieldop).

This PR was initially meant to be merged into the temporary GTIR branch
and reviewed by @egparedes here: #1570. The only change since then is to
make dace tests pass (see commit 160a616).

---------

Co-authored-by: edopao <[email protected]>
Co-authored-by: Enrique González Paredes <[email protected]>
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.

3 participants