Skip to content

Support full [b]float16 in embedding_lookup_sparse#49604

Merged
copybara-service[bot] merged 2 commits intotensorflow:masterfrom
benbarsdell:embedding-lookup-sparse-fp16
Jun 8, 2021
Merged

Support full [b]float16 in embedding_lookup_sparse#49604
copybara-service[bot] merged 2 commits intotensorflow:masterfrom
benbarsdell:embedding-lookup-sparse-fp16

Conversation

@benbarsdell
Copy link
Copy Markdown
Contributor

  • Adds float16 support for SparseSegment* ops.
  • Removes a forced cast to float32 in embedding_lookup_sparse and instead outputs the same type as the input. The inner computations are still done in float32 to avoid numerical issues.
  • This improves performance and makes the operation consistent with all other operations that output the same type as the input.

cc @nluehr @reedwm

- This removes a forced cast to float32 and instead outputs the same
  type as the input. The inner computations are still done in float32
  to avoid numerical issues.
- This improves performance and makes the op consistent with all other
  ops that output the same type as the input.
@google-ml-butler google-ml-butler bot added the size:M CL Change Size: Medium label May 25, 2021
@google-cla google-cla bot added the cla: yes label May 25, 2021
@gbaned gbaned self-assigned this May 25, 2021
@gbaned gbaned requested a review from sanjoy May 25, 2021 12:37
@sanjoy sanjoy requested a review from penpornk May 26, 2021 06:55
Copy link
Copy Markdown
Member

@penpornk penpornk left a comment

Choose a reason for hiding this comment

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

Thank you for the PR!

@google-ml-butler google-ml-butler bot added kokoro:force-run Tests on submitted change ready to pull PR ready for merge process labels May 26, 2021
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label May 26, 2021
@rthadur rthadur added ready to pull PR ready for merge process and removed ready to pull PR ready for merge process labels May 26, 2021
@gbaned gbaned removed the ready to pull PR ready for merge process label May 28, 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 May 28, 2021
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label May 28, 2021
@benbarsdell
Copy link
Copy Markdown
Contributor Author

@penpornk I noticed that SparseSegmentSumGrad was added since I submitted this PR, so we should add float16 for that op too.
I can rebase and force push to this PR, or I can submit a separate PR. Let me know which you'd prefer.

@pkanwar23 pkanwar23 added ready to pull PR ready for merge process and removed ready to pull PR ready for merge process labels Jun 4, 2021
@benbarsdell
Copy link
Copy Markdown
Contributor Author

Gentle ping

@copybara-service copybara-service bot merged commit 4f2c130 into tensorflow:master Jun 8, 2021
@penpornk
Copy link
Copy Markdown
Member

penpornk commented Jun 8, 2021

@benbarsdell Sorry for the late reply! Please submit another PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes ready to pull PR ready for merge process size:M CL Change Size: Medium

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants