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 the sources containing the models from the TFLM static lib. #47018

Merged

Conversation

advaitjain
Copy link
Member

These sources are only needed for specific tests and are now explicitly specified as part of creating a test target.

From this change onwards, we will explicitly create test targets for tests that depend on sources outside of libtensorflow-microlite.a. This avoids putting unnecessary files into MICROLITE_CC_SRCS.

Manually verified that the following command does not error out:

make -f tensorflow/lite/micro/tools/make/Makefile generate_keyword_benchmark_make_project && cd tensorflow/lite/micro/tools/make/gen/linux_x86_64_default/prj/keyword_benchmark/make/ && make -j8

Fixes #46860

@google-ml-butler google-ml-butler bot added the size:S CL Change Size: Small label Feb 8, 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.

These sources are only needed for specific tests and are now explicitly
specified as part of creating a test target.

From this change onwards, we will explicitly create test targets for
tests that depend on sources outside of libtensorflow-microlite.a. This
avoids putting unnecessary files into MICROLITE_CC_SRCS.

Manually verified that the following command does not error out:
```
make -f tensorflow/lite/micro/tools/make/Makefile generate_keyword_benchmark_make_project && cd tensorflow/lite/micro/tools/make/gen/linux_x86_64_default/prj/keyword_benchmark/make/ && make -j8
```

Fixes tensorflow#46860
@advaitjain advaitjain added the kokoro:force-run Tests on submitted change label Feb 9, 2021
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Feb 9, 2021
@gbaned gbaned self-assigned this Feb 9, 2021
@gbaned gbaned added the prtype:bugfix PR to fix a bug label Feb 9, 2021
@gbaned gbaned added this to Assigned Reviewer in PR Queue via automation Feb 9, 2021
Copy link
Contributor

@yair-ehrenwald yair-ehrenwald left a comment

Choose a reason for hiding this comment

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

This is a much better way of doing things, makes a lot more sense than linking in the model data for every build when it is only needed for a specific test.
Also appreciate the added comments to the Makefile. Anything that makes that thing more understandable... :)

@google-ml-butler google-ml-butler bot added kokoro:force-run Tests on submitted change ready to pull PR ready for merge process labels Feb 9, 2021
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Feb 9, 2021
@gbaned gbaned removed the ready to pull PR ready for merge process label Feb 9, 2021
PR Queue automation moved this from Assigned Reviewer to Approved by Reviewer Feb 9, 2021
@google-ml-butler google-ml-butler bot added kokoro:force-run Tests on submitted change ready to pull PR ready for merge process labels Feb 9, 2021
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Feb 9, 2021
@copybara-service copybara-service bot merged commit fc108e0 into tensorflow:master Feb 10, 2021
PR Queue automation moved this from Approved by Reviewer to Merged Feb 10, 2021
@advaitjain advaitjain deleted the remove-models-from-tflm-lib branch February 10, 2021 01:16
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 prtype:bugfix PR to fix a bug ready to pull PR ready for merge process size:S CL Change Size: Small
Projects
PR Queue
  
Merged
Development

Successfully merging this pull request may close these issues.

[TFLM] keyword benchmark broken when using generated Makefile projects
5 participants