Skip to content

Regions: Implementation#36

Closed
jdahm wants to merge 45 commits intoGridTools:masterfrom
ai2cm:gdp-regions
Closed

Regions: Implementation#36
jdahm wants to merge 45 commits intoGridTools:masterfrom
ai2cm:gdp-regions

Conversation

@jdahm
Copy link
Copy Markdown
Contributor

@jdahm jdahm commented Jun 15, 2020

Description

Implements the GDP-2 region() feature in:

  • GTScript frontend
  • Definition IR, infos, and Implementation IR
  • Python backends (debug, numpy)

To do:

  • Add ability to parse region() in the gtscript frontend
  • Add parsing tests
  • Lower to IIR
  • Implement in numpy backend
  • Implement in the debug backend
  • Add integration test for feature

Comment thread src/gt4py/frontend/gtscript_frontend.py Outdated
Comment thread tests/test_unittest/test_gtscript_frontend.py Outdated
@jdahm jdahm changed the title GDP-2: Regions - Implementation GDP-2 Implementation Jun 25, 2020
@jdahm jdahm changed the title GDP-2 Implementation Regions: Implementation Jun 25, 2020
Comment thread src/gt4py/gtscript.py Outdated
Copy link
Copy Markdown

@mcgibbon mcgibbon left a comment

Choose a reason for hiding this comment

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

You mentioned there's a lot of converting between different representations of "interval", it may be worth formalizing these representations into classes and providing conversion class methods NewClass.from_old_class(old_instance) to make it clear which representation is used (through type hints).

Comment thread src/gt4py/analysis/passes.py Outdated
Comment thread src/gt4py/backend/debug_backend.py Outdated
Comment thread src/gt4py/backend/numpy_backend.py Outdated
Comment thread src/gt4py/frontend/gtscript_frontend.py
Comment thread src/gt4py/ir/nodes.py Outdated
Comment thread src/gt4py/utils/meta.py Outdated
Comment thread tests/test_unittest/test_gtscript_frontend.py Outdated
@jdahm
Copy link
Copy Markdown
Contributor Author

jdahm commented Jul 6, 2020

You mentioned there's a lot of converting between different representations of "interval", it may be worth formalizing these representations into classes and providing conversion class methods NewClass.from_old_class(old_instance) to make it clear which representation is used (through type hints).

I started by making a class with a few classmethods and converters, but exposing that in the gtscript frontend file causes some circular dependencies, because it's meant to lower representations throughout the compilation process, and the dependencies mirror that.

I cleaned up the conversions as best I could using separate methods and a wrapper Region class.

@jdahm jdahm marked this pull request as draft July 9, 2020 23:55
@jdahm
Copy link
Copy Markdown
Contributor Author

jdahm commented Aug 5, 2020

Closing for now, but I will open another one soon.

@jdahm jdahm closed this Aug 5, 2020
@jdahm jdahm deleted the gdp-regions branch December 21, 2021 23:29
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