Skip to content
This repository was archived by the owner on Apr 23, 2025. It is now read-only.

Enable tracing in estimator training#68

Merged
mihaimaruseac merged 2 commits intotensorflow:masterfrom
burgerkingeater:enable_trace
Apr 21, 2021
Merged

Enable tracing in estimator training#68
mihaimaruseac merged 2 commits intotensorflow:masterfrom
burgerkingeater:enable_trace

Conversation

@burgerkingeater
Copy link
Copy Markdown
Member

@burgerkingeater burgerkingeater commented Mar 29, 2021 •
edited
Loading

This PR enables tracing in estimator when training so that TF profiler is able to work with estimator properly(Related issue: openxla/xprof#84). This echos the behavior of keras which enables tracing by default.

Note that based on the TF tracing impl, this change won't incur performance overhead when profiler is off as

  1. If tracer is on but the profiler is off, TF won’t actually trace anything.
  2. Tracer will start tracing only after profiler starts. As a result, it would trace the profiling steps only.

@google-cla google-cla bot added the cla: yes label Mar 29, 2021
@burgerkingeater
Copy link
Copy Markdown
Member Author

burgerkingeater commented Mar 29, 2021

@mihaimaruseac would you mind helping take a look? thanks

@burgerkingeater burgerkingeater changed the title enable tracing in estimator training Enable tracing in estimator training Mar 29, 2021
@rthadur rthadur requested a review from mihaimaruseac March 29, 2021 16:09
@burgerkingeater
Copy link
Copy Markdown
Member Author

burgerkingeater commented Mar 29, 2021

@rthadur would you mind reviewing? thanks

@burgerkingeater
Copy link
Copy Markdown
Member Author

burgerkingeater commented Mar 30, 2021

@mihaimaruseac @rthadur kindly ping..

@gbaned gbaned added the M CL Change Size: Medium label Mar 31, 2021
@burgerkingeater
Copy link
Copy Markdown
Member Author

burgerkingeater commented Apr 1, 2021

@geetachavan1 can you help reviewing ?

@burgerkingeater
Copy link
Copy Markdown
Member Author

burgerkingeater commented Apr 1, 2021

@yuefengz maybe you can help with reviewing?

@rthadur
Copy link
Copy Markdown

rthadur commented Apr 1, 2021

@burgerkingeater thanks for your patience , this PR will be reviewed soon.

ashahab
ashahab reviewed Apr 1, 2021
View reviewed changes
@burgerkingeater
Copy link
Copy Markdown
Member Author

burgerkingeater commented Apr 11, 2021

@mihaimaruseac thanks!

@burgerkingeater
Copy link
Copy Markdown
Member Author

burgerkingeater commented Apr 12, 2021

@mihaimaruseac can we get this merged?

@mihaimaruseac mihaimaruseac merged commit ae821bc into tensorflow:master Apr 21, 2021
@burgerkingeater burgerkingeater deleted the enable_trace branch April 21, 2021 21:12
@burgerkingeater burgerkingeater mentioned this pull request Aug 9, 2021
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

2 more reviewers

@ashahab ashahab ashahab left review comments

@mihaimaruseac mihaimaruseac mihaimaruseac approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@rthadur rthadur

@gbaned gbaned

Labels

cla: yes M CL Change Size: Medium ready to pull

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@burgerkingeater @rthadur @mihaimaruseac @ashahab @kokoro-team @gbaned