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

fix max_pool2d padding issue when lowering it to tosa #8

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

lisaliu1
Copy link
Owner

No description provided.

@lisaliu1 lisaliu1 requested a review from ehsan-toosi May 10, 2023 14:38
Copy link
Collaborator

@ehsan-toosi ehsan-toosi left a comment

Choose a reason for hiding this comment

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

  • Is there some kind of test (gtest or lit-test) in torch-mlir that you can validate this?

auto outH = outputTensorTy.getShape()[1];
auto outW = outputTensorTy.getShape()[2];
padArr[1] = (outH - 1) * strideInts[0] - inH - padArr[0] + kernelSizeInts[0];
padArr[1] = (padArr[1] < padArr[0]) ? padArr[0] : padArr[1];
Copy link
Collaborator

Choose a reason for hiding this comment

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

If you meant a swap to keep the bigger pad on the end, this is wrong. You can do swap(pad[0], pad[1]). Because with the current implementation if you come up with 1,0,1,0, then the pad array would be 1,1,1,1.

Copy link
Owner Author

Choose a reason for hiding this comment

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

no, the code is not for swapping. It's to compute the round up integer number since torch has the same padding value for left and right, top and bottom.

padArr[1] = padArr[1] + paddingInts[0];
padArr[3] = padArr[3] + paddingInts[1];
RankedTensorType outputTensorTy = outputTy.cast<RankedTensorType>();
auto inH = inputTy.getShape()[2];
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
auto inH = inputTy.getShape()[2];
auto inH = inputTy.getDimSize(2);

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