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

transformations: Split ApplyOp and AccessOp shape inference patterns. #1054

Merged
merged 2 commits into from Jun 2, 2023

Conversation

PapyChacal
Copy link
Collaborator

To naturally handle different input offsets.

Add a test case for this.
Now, shape inference walk ops regions first; each stencil.access offset it's stencil.temp operand size individually. stencil.apply shape inference then just spreads these bounds to its operands (from its block arguments).

@PapyChacal PapyChacal added the transformations Changes or adds a transformatio label Jun 1, 2023
@PapyChacal PapyChacal self-assigned this Jun 1, 2023
@PapyChacal PapyChacal requested a review from meshtag June 1, 2023 13:19
@codecov
Copy link

codecov bot commented Jun 1, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.03 🎉

Comparison is base (d896723) 86.92% compared to head (d50c090) 86.95%.

❗ Current head d50c090 differs from pull request most recent head 1403a17. Consider uploading reports for the commit 1403a17 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1054      +/-   ##
==========================================
+ Coverage   86.92%   86.95%   +0.03%     
==========================================
  Files         129      129              
  Lines       19703    19713      +10     
  Branches     2981     2979       -2     
==========================================
+ Hits        17126    17141      +15     
+ Misses       2070     2068       -2     
+ Partials      507      504       -3     
Impacted Files Coverage Δ
...l/transforms/experimental/StencilShapeInference.py 79.77% <100.00%> (+5.09%) ⬆️

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

…dle different input offsets.

Add a test case for this.
Now, shape inference walk ops regions first; each accessop offset it's stencil.temp operand size individually.
stencil.apply shape inference then just spreads these bounds to its operands (from its block arguments).
@PapyChacal PapyChacal force-pushed the emilien/stencil-inference-split branch from ccd926a to d50c090 Compare June 1, 2023 17:45
Copy link
Contributor

@meshtag meshtag left a comment

Choose a reason for hiding this comment

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

LGTM!

@PapyChacal PapyChacal merged commit 532d22b into main Jun 2, 2023
11 checks passed
@PapyChacal PapyChacal deleted the emilien/stencil-inference-split branch June 2, 2023 11:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
transformations Changes or adds a transformatio
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants