Skip to content

Conversation

@ttjost
Copy link

@ttjost ttjost commented Jun 7, 2023

Conv1D simplication had a wrong output type. This PR fixes the issue.

@ttjost ttjost requested review from flemairen6 and mgehre-amd June 7, 2023 16:46
@ttjost ttjost force-pushed the tiagot.fix_result_type_conv1d branch 2 times, most recently from 2182968 to 9e8c284 Compare June 7, 2023 17:49
@mgehre-amd
Copy link
Collaborator

Why did this not come up in the end2end tests? Should we have an end2end test covering this?

Copy link
Collaborator

@mgehre-amd mgehre-amd left a comment

Choose a reason for hiding this comment

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

LGTM, but check end2end test

@mgehre-amd mgehre-amd mentioned this pull request Jun 9, 2023
51 tasks
@ttjost
Copy link
Author

ttjost commented Jun 9, 2023

Why did this not come up in the end2end tests? Should we have an end2end test covering this?

So, we didn't see it before because we were only doing non-group Conv1D, and the type was not properly set for Group convolution. I will wait for #78 to get merged and then update this PR with a test for it.

@ttjost ttjost force-pushed the tiagot.fix_result_type_conv1d branch from 9e8c284 to baf0da4 Compare June 9, 2023 14:00
@ttjost ttjost changed the base branch from feature/misc_fixes to tiagot.tosa_group_convolution June 9, 2023 14:00
@ttjost ttjost force-pushed the tiagot.fix_result_type_conv1d branch from baf0da4 to 7d0bfc0 Compare June 9, 2023 14:02
@ttjost ttjost changed the base branch from tiagot.tosa_group_convolution to feature/misc_fixes June 9, 2023 14:05
@ttjost ttjost changed the base branch from feature/misc_fixes to tiagot.tosa_group_convolution June 9, 2023 14:06
@ttjost ttjost force-pushed the tiagot.fix_result_type_conv1d branch from 9177df8 to 0af2ef3 Compare June 9, 2023 14:10
@ttjost
Copy link
Author

ttjost commented Jun 9, 2023

I added a test that covers this issue.

Base automatically changed from tiagot.tosa_group_convolution to feature/misc_fixes June 9, 2023 14:44
@mgehre-amd mgehre-amd enabled auto-merge (squash) June 9, 2023 15:28
@mgehre-amd mgehre-amd merged commit 7dfe606 into feature/misc_fixes Jun 9, 2023
@mgehre-amd mgehre-amd deleted the tiagot.fix_result_type_conv1d branch June 9, 2023 15:39
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