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.

Sign up for GitHub

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

Jump to bottom

Modify Transpose kernel to work in TFLu #47446

Merged
merged 4 commits into from
May 20, 2021
Merged

Modify Transpose kernel to work in TFLu #47446

merged 4 commits into from
May 20, 2021

Conversation

patriklaurell
Copy link
Contributor

@patriklaurell patriklaurell commented Feb 27, 2021 •
edited
Loading

This PR modifies the transpose kernel to make it run in micro, ports the tests and adds the kernel to the micro build.

This is PR 4/4 in delivering #45695

@google-ml-butler google-ml-butler bot added the size:M CL Change Size: Medium label Feb 27, 2021
@google-ml-butler
Copy link

Thanks for contributing to TensorFlow Lite Micro.

To keep this process moving along, we'd like to make sure that you have completed the items on this list:

We would like to have a discussion on the Github issue first to determine the best path forward, and then proceed to the PR review.

@google-cla google-cla bot added the cla: yes label Feb 27, 2021
@patriklaurell patriklaurell mentioned this pull request Feb 27, 2021
Closed
@gbaned gbaned self-assigned this Mar 1, 2021
@gbaned gbaned added the comp:micro Related to TensorFlow Lite Microcontrollers label Mar 1, 2021
@gbaned gbaned added this to Assigned Reviewer in PR Queue via automation Mar 1, 2021
@gbaned gbaned requested a review from advaitjain March 1, 2021 02:12
@gbaned gbaned added the prtype:bugfix PR to fix a bug label Mar 1, 2021
@advaitjain advaitjain removed their request for review March 1, 2021 02:18
@abattery
Copy link
Contributor

abattery commented Mar 1, 2021

This PR also impacts the performance changes on the TFLite. Could you use the micro macro to divide the logics? For example, keeping the original logic for the TFlite and using a new logics for only micro cases.

@patriklaurell
Copy link
Contributor Author

@abattery I am not sure I understand what you mean. The changes in this PR should only affect the micro kernel. How does this impact the performance changes on TFLite?

@abattery
Copy link
Contributor

abattery commented Mar 2, 2021

Sorry @patriklaurell you are right. I misread the filename. Please ignore my comment.

@gbaned gbaned added the awaiting review Pull request awaiting review label Mar 19, 2021
@dmpiergiacomo
Copy link
Contributor

Hi, which is the status of this PR? Thank you.

@patriklaurell patriklaurell force-pushed the tflu-transpose-update-micro-kernel branch from 60ce10d to 774b573 Compare April 30, 2021 15:10
@patriklaurell
Copy link
Contributor Author

@petewarden I updated this PR and added two commits for porting the tests and adding transpose to the micro build. It is ready for review.

@patriklaurell patriklaurell added the kokoro:force-run Tests on submitted change label May 4, 2021
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label May 4, 2021
@patriklaurell
Copy link
Contributor Author

@advaitjain @petewarden The MacOS CPU Python3 test is failing but there is very little information in the test log and I have no way of reproducing the error locally. Do you have any insight into why it fails?

PR Queue automation moved this from Assigned Reviewer to Reviewer Requested Changes May 5, 2021
@advaitjain
Copy link
Member

@advaitjain @petewarden The MacOS CPU Python3 test is failing but there is very little information in the test log and I have no way of reproducing the error locally. Do you have any insight into why it fails?

Unfortunately, I don't know why it is failing either. Let's have you address the review comments and then we can try and figure out what is going on with the MacOS build.

@patriklaurell
Copy link
Contributor Author

patriklaurell commented May 7, 2021 •
edited
Loading

@advaitjain ready for review. Did you find anything regarding the MacOS test?

@gbaned gbaned requested review from advaitjain and removed request for advaitjain May 10, 2021 09:52
@google-ml-butler google-ml-butler bot added kokoro:force-run Tests on submitted change ready to pull PR ready for merge process labels May 10, 2021
PR Queue automation moved this from Reviewer Requested Changes to Approved by Reviewer May 10, 2021
@advaitjain
Copy link
Member

@advaitjain ready for review. Did you find anything regarding the MacOS test?

I really do not know what is going on with the MacOS failure. @petewarden, any ideas?

@advaitjain
Copy link
Member

Can you merge master and then get this PR building again after changes from 822ddbf

That will fix the MacOS and internal CI errors.

@gbaned gbaned added ready to pull PR ready for merge process and removed awaiting review Pull request awaiting review ready to pull PR ready for merge process labels May 20, 2021
@copybara-service copybara-service bot merged commit 3f4093b into tensorflow:master May 20, 2021
PR Queue automation moved this from Approved by Reviewer to Merged May 20, 2021
@dmpiergiacomo dmpiergiacomo mentioned this pull request May 20, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@advaitjain advaitjain advaitjain approved these changes

@petewarden petewarden Awaiting requested review from petewarden

Assignees

@gbaned gbaned

Labels
cla: yes comp:micro Related to TensorFlow Lite Microcontrollers kokoro:force-run Tests on submitted change prtype:bugfix PR to fix a bug ready to pull PR ready for merge process size:M CL Change Size: Medium
Projects
PR Queue
  
Merged
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@patriklaurell @abattery @dmpiergiacomo @advaitjain @kokoro-team @gbaned