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

Start using the vectorized EncodePng Op #6344

Merged
merged 9 commits into from
May 3, 2023

Conversation

rainwoodman
Copy link
Member

@rainwoodman rainwoodman commented Apr 25, 2023

Motivation for features / changes

The vectorization unblocks running tf.summary.image under DTensor.

Vectorization of EncodePNG is added in tensorflow/tensorflow@7c42227

Technical description of changes

Applied a forward compatibility wrapper for 14 days since the EncodePNG change landed.

Screenshots of UI changes (or N/A)

Detailed steps to verify changes work correctly (as executed by you)

Verified by both branches work without DTensor, and the new (vectorized) branch works under DTensor.
Note that I manually applied the internal patch to the OSS format.

Alternate designs / implementations considered (or N/A)

N/A

Verification:

cl/488391097

Vectorization supported in tensorflow/tensorflow@7c42227

The vectorization unblocks running tf.summary.image under DTensor.
@japie1235813
Copy link
Contributor

Could you give more context about this PR? Why this change is needed and how that might affect. We will also need to test the PR does not affect our current Images Plugin.

@yatbear yatbear self-requested a review April 26, 2023 19:47
@rainwoodman
Copy link
Member Author

rainwoodman commented Apr 26, 2023

This change is part of b/276803093, to support tf.summary.image under DTensor. We are building (and experimentally rolling out) a new distribution strategy based on DTensor. The dtensor side 'integrated' test will be added in the follow up change on tensorflow main repo. I'll tag you on the changelist.

Copy link
Member

@yatbear yatbear left a comment

Choose a reason for hiding this comment

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

Thanks! Please make sure to link the CL that has a global TAP run in the PR description as the verification artifact.

@rainwoodman
Copy link
Member Author

Done.

@yatbear yatbear merged commit 7474a8d into tensorflow:master May 3, 2023
groszewn pushed a commit that referenced this pull request Oct 4, 2023
See #6344 for context.

Googlers, see b/276803093 for more details.

Tested internally: cl/570508513

#oncall #summary #image
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants