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

Micro transpose op ported and tested for TFLM #48192

Closed

Conversation

dmpiergiacomo
Copy link
Contributor

Fixes #45695
Fixes #43472

Addition of TRANSPOSE operation and its relevant test file to TF Lite for Microcontrollers. This operation has been successfully tested in the following ways:

  1. Running transpose_test with Bazel
  2. Building TFLM for a target nRF52840 DK (Cortex-M) and verifying that the target error Didn't find op for builtin opcode 'TRANSPOSE' version '2' disappeared

More details abut point 1.
The command used is:

bazel test //tensorflow/lite/micro/kernels:transpose_test --verbose_failures

It returns:

.....
Removed for brevity
.....
INFO: Analyzed target //tensorflow/lite/micro/kernels:transpose_test (0 packages loaded, 0 targets configured).
INFO: Found 1 test target...
Target //tensorflow/lite/micro/kernels:transpose_test up-to-date:
  bazel-bin/tensorflow/lite/micro/kernels/transpose_test
INFO: Elapsed time: 0.156s, Critical Path: 0.00s
INFO: 1 process: 1 internal.
INFO: Build completed successfully, 1 total action
//tensorflow/lite/micro/kernels:transpose_test                  (cached) PASSED in 0.0s

Executed 0 out of 1 test: 1 test passes.
INFO: Build completed successfully, 1 total action

More details abut point 2.
I always build locally my version of TFLM with the command:

make -f tensorflow/lite/micro/tools/make/Makefile \
    TARGET=cortex_m_generic \
    TARGET_ARCH=cortex-m4+fp \
    TARGET_TOOLCHAIN_ROOT=/opt/gcc-arm-none-eabi-9-2020-q2-update/bin/ \
    OPTIMIZED_KERNEL_DIR=cmsis_nn microlite

Before applying the fixes of this PR, the error on the target nRF52840 DK was:

[ERR] ./model/debug_log.cc:12: Didn't find op for builtin opcode 'TRANSPOSE' version '2'. An older version of this builtin might be supported. Are you using an old TFLite binary with a newer model?
[ERR] ./model/debug_log.cc:12: 
[ERR] ./model/debug_log.cc:12: Failed to get registration from op code TRANSPOSE
[ERR] ./model/debug_log.cc:12: 
[ERR] ./model/debug_log.cc:12: Failed starting model allocation.
[ERR] ./model/debug_log.cc:12: 
[ERR] ./model/debug_log.cc:12: AllocateTensors() failed

Now, after applying the fixes of this PR, the error disappears and the code can process further at runtime.

@google-ml-butler google-ml-butler bot added the size:L CL Change Size: Large label Mar 30, 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
Copy link

google-cla bot commented Mar 30, 2021

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

@google-cla google-cla bot added the cla: no label Mar 30, 2021
@dmpiergiacomo
Copy link
Contributor Author

Hi, please note that commits b0174fc and ba839f8 are already accepted in this other PR 48144.

@dmpiergiacomo
Copy link
Contributor Author

@googlebot I fixed it.

@google-cla
Copy link

google-cla bot commented Mar 30, 2021

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

@victorromeo
Copy link

CLA provided

@victorromeo
Copy link

@googlebot I fixed it.

@google-cla
Copy link

google-cla bot commented Mar 31, 2021

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

@gbaned gbaned self-assigned this Mar 31, 2021
@gbaned gbaned added this to Assigned Reviewer in PR Queue via automation Mar 31, 2021
@dmpiergiacomo
Copy link
Contributor Author

@googlebot I fixed it.

@google-cla
Copy link

google-cla bot commented Mar 31, 2021

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

@dmpiergiacomo
Copy link
Contributor Author

Hi @victorromeo, I think I succesfully signed the CLA, but the bot keeps complaining. What about your side? Did you sign with the same email used for the commit?

Thanks!

@victorromeo
Copy link

@dmpiergiacomo I have determined that the email the commits were done under was not a verifiable email address and as a result, I am unable to sign the CLA, without modifying the email addresses first.

I've tried a git filter-branch style replace on a separate branch to swap the email addresses, but this appears to have modified more than I had anticipated. Am hoping to get my historical commits assigned to victorromeo.gh@gmail.com if you know how to do this?

@dmpiergiacomo
Copy link
Contributor Author

@victorromeo, thank you for the explanation, I understand the problem.

I think the easiest solution would be the manual intervention of a Googler. I can see here that they should have the power to override the cla: yes issue label. @gbaned is this correct? Could you help us?

Another solution could be a git rebase through which I could amend your commits switching to the email address victorromeo.gh@gmail.com. I could do this from the same branch of this PR. I would however prefer the first solution, if possible for a Googler.

@victorromeo
Copy link

Alternative 3) Please let me know if you'd like me to re-commit my changes on a clean branch, using the correct email account.

I would however prefer the first solution, if possible for a Googler.

Agreed, as using a Googler will maintain the code base in its current state, however either solution is fine by me.

@gbaned
Copy link
Contributor

gbaned commented Apr 8, 2021

@dmpiergiacomo Can you please make sure to use same GitHub username and email-id associated with it.

@dmpiergiacomo
Copy link
Contributor Author

dmpiergiacomo commented Apr 15, 2021

Hi @gbaned apologies for the delay, I had some hard deadlines to respect.

@dmpiergiacomo Can you please make sure to use same GitHub username and email-id associated with it.

I believe I am using the same GitHub username and email-id associated with it. If not, could you please clarify?

From my understanding the issue is that the email used by @victorromeo to push his changes can no longer be accessed to sign Google CLA. We would therefore kindly ask you to override the CLA flag with your admin power. Would this be possible?

Thank you.

@gbaned gbaned added cla: yes comp:micro Related to TensorFlow Lite Microcontrollers and removed cla: no labels Apr 22, 2021
@gbaned gbaned requested a review from advaitjain April 22, 2021 17:33
@gbaned gbaned added the awaiting review Pull request awaiting review label Apr 30, 2021
@dmpiergiacomo
Copy link
Contributor Author

Thank you @gbaned for forcing the CLA flag to yes.

It looks like the community CI build now fails, probably something has changed in the meanwhile. Does it make sense to debug it, or better to wait for @advaitjain review first?

Thank you.

@advaitjain
Copy link
Member

Sorry for the delay here. I'm going to close the current PR since #48192 is doing the same and is soon going to be merged.

@advaitjain advaitjain closed this May 20, 2021
PR Queue automation moved this from Assigned Reviewer to Closed/Rejected May 20, 2021
@dmpiergiacomo
Copy link
Contributor Author

dmpiergiacomo commented May 20, 2021

Hi @advaitjain, I believe you might have provided a wrong link in your last message. If it's the case, could you provide the correct one? Thanks.

Are we talking about #47446 maybe?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting review Pull request awaiting review cla: yes comp:micro Related to TensorFlow Lite Microcontrollers size:L CL Change Size: Large
Projects
PR Queue
  
Closed/Rejected
Development

Successfully merging this pull request may close these issues.

micro: Port TRANSPOSE from lite to micro Missing TRANSPOSE Op Kernel
4 participants