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

Add an InitializeTarget function that can be specialized for a given target. #47077

Merged
merged 1 commit into from Feb 12, 2021

Conversation

advaitjain
Copy link
Member

@advaitjain advaitjain commented Feb 10, 2021

This will allow the unit tests to be run on additional targets that need some addiitonal initialization (for example corstone_300 from #46830).

This particular change is broken out from the Corstone PR #46830 to be able to have smaller more reviewable PRs.

In the past, we have added state to the DebugLog() and GetCurrentTimeTicks() functions as a way to avoid having an InitializeTarget function. With this change, we are deciding to go with an explicit intitialization step instead.

This change has added calls to tflite::InitializeTarget to the tests, benchmarks, and examples and converted the Arduino and SparkfunEdge to make use of this explicit initialization.

The changes for the Arduino and SparkfunEdge have not been tested on actual hardware.

Progress towards #46829
Fixes http://b/150808076

@google-ml-butler google-ml-butler bot added the size:M CL Change Size: Medium label Feb 10, 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 10, 2021
@advaitjain advaitjain added comp:micro Related to TensorFlow Lite Microcontrollers kokoro:force-run Tests on submitted change labels Feb 10, 2021
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Feb 10, 2021
@gbaned gbaned self-assigned this Feb 11, 2021
@gbaned gbaned added this to Assigned Reviewer in PR Queue via automation Feb 11, 2021
@gbaned gbaned added the prtype:bugfix PR to fix a bug label Feb 11, 2021
Copy link
Contributor

@mansnils mansnils left a comment

Choose a reason for hiding this comment

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

Looking good! Just a minor comment

namespace tflite {

// This should called during initialization of TFLM binaries and tests. It can
// be specialized if there us a need for custom target-specific intialization.
Copy link
Contributor

Choose a reason for hiding this comment

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

nitpick: typo

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed.

@advaitjain advaitjain changed the title Add an InitializeTarget function that can be sepcialized for a given target. Add an InitializeTarget function that can be specialized for a given target. Feb 11, 2021
@advaitjain advaitjain added kokoro:force-run Tests on submitted change and removed prtype:bugfix PR to fix a bug labels Feb 11, 2021
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Feb 11, 2021
@advaitjain advaitjain added the kokoro:force-run Tests on submitted change label Feb 11, 2021
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Feb 11, 2021
@advaitjain
Copy link
Member Author

@njeffrie: after our dicussion, I decided to go one step further and not add calls to InitializeTarget from DebugLog for the Arduino and SparkfunEdge.

Instead, I added a call to InitializeTarget from all of our examples. I like this approach more because it gives full control of the initialization to the application rather than having some mysterious init calls hidden away in DebugLog.

The down-side is that I might have broken some examples on Arduino/SparkfunEdge because the change is not tested on actual hardware.

@advaitjain advaitjain added the kokoro:force-run Tests on submitted change label Feb 11, 2021
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Feb 11, 2021
@advaitjain advaitjain added ready to pull PR ready for merge process and removed ready to pull PR ready for merge process labels Feb 11, 2021
…target.

This will allow the unit tests to be run on additional targets that need
some addiitonal initialization (for example cornstone_300 from tensorflow#46830).

This particular change is broken out from the Cornstone PR tensorflow#46830 to
be able to have smaller more reviewable PRs.

In the past, we have added state to the DebugLog() and
GetCurrentTimeTicks() functions as a way to avoid having an
InitializeTarget function. With this change, we are deciding to go with
an explicit intitialization step instead.

This change has added calls to tflite::InitializeTarget to the tests,
benchmarks, and examples and converted the Arduino and SparkfunEdge to
make use of this explicit initialization.

The changes for the Arduino and SparkfunEdge have not been tested on
actual hardware.

Progress towards tensorflow#46829
@google-ml-butler google-ml-butler bot removed the ready to pull PR ready for merge process label Feb 12, 2021
@advaitjain advaitjain added kokoro:force-run Tests on submitted change ready to pull PR ready for merge process labels Feb 12, 2021
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Feb 12, 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.

We will have to update internal examples as well once this has landed.

PR Queue automation moved this from Assigned Reviewer to Approved by Reviewer Feb 12, 2021
@google-ml-butler google-ml-butler bot added the kokoro:force-run Tests on submitted change label Feb 12, 2021
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Feb 12, 2021
@mansnils mansnils added the kokoro:force-run Tests on submitted change label Feb 12, 2021
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Feb 12, 2021
@copybara-service copybara-service bot merged commit e15657c into tensorflow:master Feb 12, 2021
PR Queue automation moved this from Approved by Reviewer to Merged Feb 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes comp:micro Related to TensorFlow Lite Microcontrollers ready to pull PR ready for merge process size:M CL Change Size: Medium
Projects
PR Queue
  
Merged
Development

Successfully merging this pull request may close these issues.

None yet

5 participants