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.

Sign up for GitHub

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

Jump to bottom

[lite/micro] Override operator delete in memory planner #32417

Merged
merged 1 commit into from
Jan 9, 2020
Merged

[lite/micro] Override operator delete in memory planner #32417

merged 1 commit into from
Jan 9, 2020

Conversation

csukuangfj
Copy link
Contributor

Fix #32416 .
Override operator delete.

@tensorflow-bot tensorflow-bot bot added the size:XS CL Change Size: Extra Small label Sep 11, 2019
@gbaned gbaned self-assigned this Sep 11, 2019
@gbaned gbaned added the comp:lite TF Lite related issues label Sep 11, 2019
@alanchiao alanchiao requested review from wangtz and petewarden and removed request for alanchiao September 11, 2019 16:52
@gbaned gbaned added the awaiting review Pull request awaiting review label Sep 13, 2019
wangtz
wangtz previously approved these changes Sep 16, 2019
View reviewed changes
Copy link
Member

@wangtz wangtz left a comment

Choose a reason for hiding this comment

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

Thanks! Do you also need this for linear_memory_planner.h?

@tensorflow-bot tensorflow-bot bot added kokoro:force-run Tests on submitted change ready to pull PR ready for merge process labels Sep 16, 2019
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Sep 16, 2019
@tensorflowbutler tensorflowbutler removed the awaiting review Pull request awaiting review label Sep 16, 2019
@csukuangfj
Copy link
Contributor Author

@wangtz
LinearMemoryPlanner is never used so it does not cause link time error.
I'll fix it in the next patch.

@gbaned gbaned added the prtype:bugfix PR to fix a bug label Sep 17, 2019
wangtz
wangtz previously approved these changes Sep 20, 2019
View reviewed changes
@tensorflow-bot tensorflow-bot bot added the kokoro:force-run Tests on submitted change label Sep 20, 2019
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Sep 20, 2019
@gbaned gbaned added ready to pull PR ready for merge process and removed ready to pull PR ready for merge process labels Oct 3, 2019
@gbaned gbaned added ready to pull PR ready for merge process and removed ready to pull PR ready for merge process labels Oct 24, 2019
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Nov 19, 2019
gbaned
gbaned reviewed Nov 21, 2019
View reviewed changes
tensorflow/lite/experimental/micro/memory_planner/greedy_memory_planner.h Outdated
@@ -16,6 +16,7 @@ limitations under the License.
#ifndef TENSORFLOW_LITE_EXPERIMENTAL_MICRO_MEMORY_PLANNER_GREEDY_MEMORY_PLANNER_H_
#define TENSORFLOW_LITE_EXPERIMENTAL_MICRO_MEMORY_PLANNER_GREEDY_MEMORY_PLANNER_H_

#include "tensorflow/lite/experimental/micro/compatibility.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

Here is the internal error we are getting , can you please check this.
/third_party/tensorflow/lite/experimental/micro/memory_planner/greedy_memory_planner.h:19:10: fatal error: 'third_party/tensorflow/lite/experimental/micro/compatibility.h' file not found
#include "third_party/tensorflow/lite/experimental/micro/compatibility.h"
cc @petewarden @wangtz

@mihaimaruseac
Copy link
Collaborator

Nit: Please don't include issue number in title, include it in the description of the PR. And give the PR a descriptive title

@gbaned gbaned added the stat:awaiting response Status - Awaiting response from author label Nov 25, 2019
@gbaned
Copy link
Contributor

gbaned commented Dec 10, 2019

@csukuangfj Could you please address above internal error and resolve conflicts? Thanks!

@tensorflowbutler tensorflowbutler added the stale This label marks the issue/pr stale - to be closed automatically if no activity label Dec 10, 2019
@csukuangfj
Copy link
Contributor Author

@gbaned
you've mentioned your two colleagues but neither of them replies. I do NOT know
why the file compatibility.h is missing and I guess there must be some reasons for
its removal.

Three months have passed and such a small change is still not merged. I can think of the
high communication cost in such a large project.

I will resolve the conflicts tomorrow.

@csukuangfj csukuangfj changed the title fix #32416. [lite/micro] Override operator delete in memory planner Dec 10, 2019
@tensorflowbutler tensorflowbutler added stat:awaiting response Status - Awaiting response from author and removed stat:awaiting response Status - Awaiting response from author stale This label marks the issue/pr stale - to be closed automatically if no activity labels Dec 11, 2019
fix #32416.
0d08d8d 
Override operator delete.
@csukuangfj csukuangfj dismissed stale reviews from petewarden and wangtz via 0d08d8d December 16, 2019 05:44
@gbaned gbaned added awaiting review Pull request awaiting review and removed stat:awaiting response Status - Awaiting response from author ready to pull PR ready for merge process labels Dec 17, 2019
@tensorflow-bot tensorflow-bot bot added kokoro:force-run Tests on submitted change ready to pull PR ready for merge process labels Dec 19, 2019
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Dec 19, 2019
@petewarden
Copy link
Contributor

Thanks for your patience, sorry this has taken so long, we're trying to improve the process so it doesn't take so long in the future. Feel free to email me directly at petewarden@google.com if you find yourself stuck with a long-running PR like this again.

@gbaned gbaned removed the awaiting review Pull request awaiting review label Dec 20, 2019
@csukuangfj csukuangfj mentioned this pull request Dec 24, 2019
Closed
@rthadur
Copy link
Contributor

rthadur commented Jan 9, 2020

Changes have been submitted internally , waiting for auto-merge to happen. Thank you

tensorflow-copybara pushed a commit that referenced this pull request Jan 9, 2020
@tensorflower-gardener
Merge pull request #32417 from csukuangfj:fix-32416
85dc9e2 
PiperOrigin-RevId: 288978816
Change-Id: I839d3b6e4e5c4c94d436b5a08b1ae842724428b9
@tensorflow-copybara tensorflow-copybara merged commit 0d08d8d into tensorflow:master Jan 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gbaned gbaned gbaned left review comments

@petewarden petewarden petewarden approved these changes

@wangtz wangtz wangtz left review comments

Assignees

@gbaned gbaned

Labels
cla: yes comp:lite TF Lite related issues comp:micro Related to TensorFlow Lite Microcontrollers prtype:bugfix PR to fix a bug ready to pull PR ready for merge process size:XS CL Change Size: Extra Small
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[lite/micro] missing delete() in GreedyMemoryPlanner