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

TFLu: Port TFL detection postprocess operator #43566

Merged
merged 24 commits into from Dec 3, 2020

Conversation

mansnils
Copy link
Contributor

@mansnils mansnils commented Sep 25, 2020

This is fixing issue: #43565

Change-Id: I117f9e3f206f2433d55d60e680bf7aa04b954445
@google-ml-butler google-ml-butler bot added the size:XL CL Change Size:Extra Large label Sep 25, 2020
@gbaned gbaned self-assigned this Sep 25, 2020
@gbaned gbaned added comp:lite TF Lite related issues comp:micro Related to TensorFlow Lite Microcontrollers labels Sep 25, 2020
@gbaned gbaned added this to Assigned Reviewer in PR Queue via automation Sep 25, 2020
Adding support for running test_kernel_detection_postprocess_test with
Renode by using pre-generated Flexbuffers::Builder data.
@gbaned
Copy link
Contributor

gbaned commented Oct 6, 2020

@mansnils Can you please resolve conflicts? Thanks!

@gbaned gbaned added the stat:awaiting response Status - Awaiting response from author label Oct 9, 2020
@gbaned
Copy link
Contributor

gbaned commented Oct 14, 2020

@mansnils Any update on this PR? Please. Thanks!

@mansnils
Copy link
Contributor Author

Removing support for unknown output dimensions. Because it does not work with TFLiteEvalTensor API and it would require major changes in TFLu.
Most models seen with this operator have unknown output dimensions. So to run those they have to be modified first so that the out dimensions are known beforehand.

@tensorflowbutler tensorflowbutler removed the stat:awaiting response Status - Awaiting response from author label Oct 18, 2020
@gbaned gbaned requested a review from njeffrie October 23, 2020 18:15
@gbaned gbaned added the awaiting review Pull request awaiting review label Oct 23, 2020
PR Queue automation moved this from Assigned Reviewer to Approved by Reviewer Oct 27, 2020
@google-ml-butler google-ml-butler bot added kokoro:force-run Tests on submitted change ready to pull PR ready for merge process labels Oct 27, 2020
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Oct 27, 2020
@gbaned gbaned removed the awaiting review Pull request awaiting review label Oct 28, 2020
@advaitjain advaitjain added the kokoro:force-run Tests on submitted change label Oct 28, 2020
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Oct 28, 2020
@gbaned gbaned added ready to pull PR ready for merge process and removed ready to pull PR ready for merge process labels Oct 29, 2020
@advaitjain advaitjain mentioned this pull request Nov 24, 2020
@advaitjain
Copy link
Member

Still having issues unfortunately :-/

The CI is failing with:

tensorflow/lite/micro/kernels/detection_postprocess.cc: In function ‘TfLiteStatus tflite::{anonymous}::Eval(TfLiteContext*, TfLiteNode*)’:
tensorflow/lite/micro/kernels/detection_postprocess.cc:591:58: error: potential null pointer dereference [-Werror=null-dereference]
   tflite::micro::GetTensorData<float>(num_detections)[0] =
   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^
       size_of_sorted_indices;
       ~~~~~~~~~~~~~~~~~~~~~~                              
tensorflow/lite/micro/kernels/detection_postprocess.cc:687:58: error: potential null pointer dereference [-Werror=null-dereference]
   tflite::micro::GetTensorData<float>(num_detections)[0] = output_box_index;

but it is not reproducible locally.

My current guess is that the gcc version used as part of CI is old (gcc-6.3.0). I have a change under review that updates

to

python:3.9.0-buster

which should mean that we use gcc 8.3.0 as part of CI. Let's see what happens when that change get merged.

@mansnils mansnils added the kokoro:force-run Tests on submitted change label Nov 25, 2020
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Nov 25, 2020
@mansnils
Copy link
Contributor Author

@advaitjain It seems like the updated docker didn't help, or do we need to rebase?

@advaitjain advaitjain added the kokoro:force-run Tests on submitted change label Nov 30, 2020
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Nov 30, 2020
@google-ml-butler google-ml-butler bot removed the ready to pull PR ready for merge process label Nov 30, 2020
@advaitjain advaitjain added the kokoro:force-run Tests on submitted change label Nov 30, 2020
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Nov 30, 2020
…cess

Manually resolved the following conflicts:
	tensorflow/lite/micro/micro_mutable_op_resolver.h
@advaitjain advaitjain added the kokoro:force-run Tests on submitted change label Nov 30, 2020
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Nov 30, 2020
@advaitjain
Copy link
Member

Still having issues unfortunately :-/

The CI is failing with:

tensorflow/lite/micro/kernels/detection_postprocess.cc: In function ‘TfLiteStatus tflite::{anonymous}::Eval(TfLiteContext*, TfLiteNode*)’:
tensorflow/lite/micro/kernels/detection_postprocess.cc:591:58: error: potential null pointer dereference [-Werror=null-dereference]
   tflite::micro::GetTensorData<float>(num_detections)[0] =
   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^
       size_of_sorted_indices;
       ~~~~~~~~~~~~~~~~~~~~~~                              
tensorflow/lite/micro/kernels/detection_postprocess.cc:687:58: error: potential null pointer dereference [-Werror=null-dereference]
   tflite::micro::GetTensorData<float>(num_detections)[0] = output_box_index;

but it is not reproducible locally.

My current guess is that the gcc version used as part of CI is old (gcc-6.3.0). I have a change under review that updates

to

python:3.9.0-buster

which should mean that we use gcc 8.3.0 as part of CI. Let's see what happens when that change get merged.

I wasn't able to reproduce locally because I hadn't done a make clean_downloads.

Created #45286 to address this issue. This comment has some additional context.

@advaitjain advaitjain added the kokoro:force-run Tests on submitted change label Dec 1, 2020
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Dec 1, 2020
@advaitjain advaitjain added the ready to pull PR ready for merge process label Dec 1, 2020
@google-ml-butler google-ml-butler bot removed the ready to pull PR ready for merge process label Dec 2, 2020
@google-ml-butler google-ml-butler bot added kokoro:force-run Tests on submitted change ready to pull PR ready for merge process labels Dec 2, 2020
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Dec 2, 2020
@copybara-service copybara-service bot merged commit 15c8383 into tensorflow:master Dec 3, 2020
PR Queue automation moved this from Approved by Reviewer to Merged Dec 3, 2020
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:XL CL Change Size:Extra Large
Projects
PR Queue
  
Merged
Development

Successfully merging this pull request may close these issues.

None yet

8 participants