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

dialects: (csl-stencil) Add coefficients to apply op #3320

Merged
merged 11 commits into from
Oct 22, 2024

Conversation

n-io
Copy link
Collaborator

@n-io n-io commented Oct 17, 2024

No description provided.

@n-io n-io self-assigned this Oct 17, 2024
@n-io n-io added dialects Changes on the dialects minor For minor PRs, easy and quick to review, quickly mergeable labels Oct 17, 2024
@n-io n-io changed the title dialects: (csl-stencil) Add coefficients property dialects: (csl-stencil) Add coefficients to apply op Oct 17, 2024
Copy link

codecov bot commented Oct 17, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.06%. Comparing base (6c736f7) to head (8c82b29).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3320      +/-   ##
==========================================
+ Coverage   90.04%   90.06%   +0.02%     
==========================================
  Files         446      446              
  Lines       56330    56385      +55     
  Branches     5405     5411       +6     
==========================================
+ Hits        50720    50782      +62     
+ Misses       4183     4177       -6     
+ Partials     1427     1426       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@n-io n-io marked this pull request as draft October 18, 2024 10:56
@n-io n-io marked this pull request as ready for review October 18, 2024 13:23
Copy link
Collaborator

@dk949 dk949 left a comment

Choose a reason for hiding this comment

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

Couple of minor points, but overall looks great!

@@ -313,6 +313,16 @@ class PtrType(ParametrizedAttribute, TypeAttribute, ContainerType[Attribute]):
kind: ParameterDef[PtrKindAttr]
constness: ParameterDef[PtrConstAttr]

@staticmethod
def get(typ: Attribute, is_single: bool, is_const: bool):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe typ could be a TypeAttribute, since PtrType.type is?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done as suggested

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Changing back to Attribute to fix tests

xdsl/transforms/lower_csl_stencil.py Show resolved Hide resolved
xdsl/transforms/lower_csl_stencil.py Outdated Show resolved Hide resolved
@n-io n-io requested a review from dk949 October 22, 2024 14:13
Copy link
Collaborator

@dk949 dk949 left a comment

Choose a reason for hiding this comment

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

Thank you!

@n-io n-io merged commit 42c2812 into main Oct 22, 2024
14 checks passed
@n-io n-io deleted the nicolai/csl-stencil-coeffs branch October 22, 2024 16:11
EdmundGoodman pushed a commit to EdmundGoodman/xdsl that referenced this pull request Dec 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dialects Changes on the dialects minor For minor PRs, easy and quick to review, quickly mergeable
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants