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

Remove unused MicroInterpreter::tensor() API. #49114

Merged
merged 1 commit into from May 11, 2021

Conversation

advaitjain
Copy link
Member

@advaitjain advaitjain commented May 11, 2021

The implementation for this function was always allocating a persistent buffer which would mean that calling this function repeatedly would unexpectedly result in an error as a result of running out of space in the arena (basically a memory leak).

Additionally, it appears that the function was only being used to test ResetVariableTensor and that test case has also been removed with this change.

http://b/187845286 will be used to add a unit test for ResetVaiableTensor.

@google-ml-butler google-ml-butler bot added the size:M CL Change Size: Medium label May 11, 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 May 11, 2021
@advaitjain advaitjain requested a review from njeffrie May 11, 2021 18:46
@advaitjain advaitjain added kokoro:force-run Tests on submitted change ready to pull PR ready for merge process labels May 11, 2021
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label May 11, 2021
The implementation for this function was always allocating a persistent
buffer which would mean that calling this function repeatedly would
unexpectedly result in an error as a result of running out of space in
the arena (basically a memory leak).

Additionally, it appears that the function was only being used to test
the ResetVariableTensor API.
@google-ml-butler google-ml-butler bot removed the ready to pull PR ready for merge process label May 11, 2021
@advaitjain advaitjain added the kokoro:force-run Tests on submitted change label May 11, 2021
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label May 11, 2021
@advaitjain advaitjain added kokoro:force-run Tests on submitted change ready to pull PR ready for merge process labels May 11, 2021
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label May 11, 2021
Copy link
Contributor

@njeffrie njeffrie left a comment

Choose a reason for hiding this comment

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

Do we have any way to test ResetVariableTensor once we remove its test here? Other than that looks good.

@google-ml-butler google-ml-butler bot added the kokoro:force-run Tests on submitted change label May 11, 2021
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label May 11, 2021
@advaitjain
Copy link
Member Author

Do we have any way to test ResetVariableTensor once we remove its test here? Other than that looks good.

Not immediately. I have filed a bug for that -- will figure out how we can test it in a bit.

@njeffrie
Copy link
Contributor

Got it. I suspect we can add a test along with subgraph support.

@copybara-service copybara-service bot merged commit cf5ddf6 into tensorflow:master May 11, 2021
@advaitjain advaitjain deleted the remove-unused-apis branch May 11, 2021 21:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes ready to pull PR ready for merge process size:M CL Change Size: Medium
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants