-
Notifications
You must be signed in to change notification settings - Fork 77
added datype arguments for ttir operations and testcases #5036
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
base: main
Are you sure you want to change the base?
added datype arguments for ttir operations and testcases #5036
Conversation
| }]; | ||
|
|
||
| let arguments = (ins DenseI32ArrayAttr:$shape); | ||
| let arguments = (ins DenseI32ArrayAttr:$shape,TypeAttr:$dtype); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix formatting
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
| starting point for accumulation operations. | ||
| }]; | ||
| let arguments = (ins DenseI32ArrayAttr:$shape, | ||
| TypeAttr:$dtype); // Added dtype argument |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of adding this directly to zerosop, add to TTIR_NamedFullOp so that it applied to all named full-like ops (ones, zeros, etc.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes added
|
|
||
| // Create TypeAttr for the dtype argument |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed the comment
| .create<ttir::ArangeOp>( // perform arange on the last dimension to | ||
| // match how ttnn behaves | ||
| op.getLoc(), arangeOutputType, start, end, step, 0) | ||
| op.getLoc(), arangeOutputType, start, end, step, 0,outputType.getElementType()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| op.getLoc(), arangeOutputType, start, end, step, 0,outputType.getElementType()) | |
| op.getLoc(), arangeOutputType, start, end, step, 0, outputType.getElementType()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed formatting
| func.func @forward_reuse_zeros(%arg0: tensor<32x32xbf16> {ttcore.argument_type = #ttcore.argument_type<input>}, %arg1: tensor<32x32xbf16> {ttcore.argument_type = #ttcore.argument_type<parameter>}, %arg2: tensor<32x32xbf16> {ttcore.argument_type = #ttcore.argument_type<parameter>}, %arg3: tensor<32x32xbf16> {ttcore.argument_type = #ttcore.argument_type<constant>}) -> tensor<32x32xbf16> { | ||
| // CHECK: = ttcore.load_cached(@forward_reuse_zeros_const_eval_0, [%arg1, %arg2]) | ||
| %0 = "ttir.zeros"() <{shape = array<i32:32, 32>}> : () -> tensor<32x32xbf16> | ||
| %0 = "ttir.zeros"() <{shape = array<i32:32, 32>,dtype = i64}> : () -> tensor<32x32xbf16> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| %0 = "ttir.zeros"() <{shape = array<i32:32, 32>,dtype = i64}> : () -> tensor<32x32xbf16> | |
| %0 = "ttir.zeros"() <{shape = array<i32:32, 32>, dtype = i64}> : () -> tensor<32x32xbf16> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed the formatting
| func.func @zeros_f32() -> tensor<32x64x128xf32> { | ||
| // CHECK: {{.*}} = "ttnn.zeros"({{.*}}) {{.*}} -> tensor<32x64x128xf32{{.*}}> | ||
| %0 = "ttir.zeros"() <{shape = array<i32:32, 64, 128>}> : () -> tensor<32x64x128xf32> | ||
| %0 = "ttir.zeros"() <{shape = array<i32:32, 64, 128>,dtype = i64}> : () -> tensor<32x64x128xf32> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| %0 = "ttir.zeros"() <{shape = array<i32:32, 64, 128>,dtype = i64}> : () -> tensor<32x64x128xf32> | |
| %0 = "ttir.zeros"() <{shape = array<i32:32, 64, 128>, dtype = i64}> : () -> tensor<32x64x128xf32> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed the formatting
|
|
||
| func.func @zeros() -> tensor<13x24x56x42xbf16> { | ||
| %0 = "ttir.zeros"() <{shape = array<i32:13, 24, 56, 42>}> : () -> tensor<13x24x56x42xbf16> | ||
| %0 = "ttir.zeros"() <{shape = array<i32:13, 24, 56, 42>, dtype = i64}> : () -> tensor<13x24x56x42xbf16> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why i64?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You've added i64 in many other places, please fix everywhere.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changed according to the return type
|
@svuckovicTT fixed the changes and passed the tests successfully.Can you please review? |
|
Hey @HariK3927, I've enabled the CI pipeline, there are some failures in the pre-commit stage. You can look at the logs and manually fix things, but I suggest installing the pre-commit hooks and running the tool which will do this automatically for you. You can find more info here: https://docs.tenstorrent.com/tt-mlir/getting-started.html?#pre-commit |
|
@nsumrakTT @vvukomanTT Hey guys, any ideas why the |
|
@svuckovicTT Yes, permission error, I'll fix in separate PR. |
|
@HariK3927 can you please rebase once this PR lands? I'll launch tests after. Thanks! |
|
@svuckovicTT It seems the build fix PR is merged to main.Shall I merge this PR to updated main or rebasing needed ? |
|
@HariK3927 Whatever is easier for you. |
|
@nsumrakTT I have merged to latest main branch . If anyfurther changes required please let me know . Thank you. |
|
@svuckovicTT yes using this command fixed the errors and passed the build .Hope some luck this time |
|
@nsumrakTT @svuckovicTT Can I execute CI workflow ? |
|
Hi @HariK3927, you should be able to reproduce the error for some of these failing tests on your side. E.g:
Refer to this to run the test suite locally. It does not cover all the CI tests but is a good sanity check. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5036 +/- ##
==========================================
+ Coverage 69.21% 69.23% +0.02%
==========================================
Files 333 333
Lines 50762 50766 +4
==========================================
+ Hits 35136 35150 +14
+ Misses 15626 15616 -10 ☔ View full report in Codecov by Sentry. |
anuragsingh-tt
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
test_ones is still failing.
|
Will check |
Ticket
#2094
Problem description
Added dataarguments for ttir operations
What's changed
added the dtype arguments and done the changes in test files for verification.
Checklist