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

Refactor ReshapeSparseTensor into a template+class #46275

Merged

Conversation

benbarsdell
Copy link
Contributor

This is in preparation for adding a GPU implementation.
No functional change.

cc @nluehr

- This is in preparation for adding a GPU implementation.
- No functional change.
@google-ml-butler google-ml-butler bot added the size:M CL Change Size: Medium label Jan 8, 2021
@google-cla google-cla bot added the cla: yes label Jan 8, 2021
@gbaned gbaned self-assigned this Jan 8, 2021
@gbaned gbaned added the comp:core issues related to core part of tensorflow label Jan 8, 2021
@gbaned gbaned added this to Assigned Reviewer in PR Queue via automation Jan 8, 2021
@gbaned gbaned requested a review from cheshire January 8, 2021 07:58
@cheshire cheshire removed their request for review January 8, 2021 18:58
@cheshire
Copy link
Member

cheshire commented Jan 8, 2021

@allenlavoie Any idea who should review this?

@allenlavoie
Copy link
Member

@penpornk has been working on sparse kernels I believe. Penporn, want to take a look?

@penpornk penpornk self-requested a review January 8, 2021 19:18
@penpornk
Copy link
Member

penpornk commented Jan 8, 2021

@allenlavoie @cheshire Sure, I can review this. :)

Copy link
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 and I'm so sorry for the delay! I have minor comments.

tensorflow/core/kernels/reshape_util.cc Outdated Show resolved Hide resolved
tensorflow/core/kernels/reshape_util.h Outdated Show resolved Hide resolved
tensorflow/core/kernels/reshape_util.cc Outdated Show resolved Hide resolved
tensorflow/core/kernels/reshape_util.cc Show resolved Hide resolved
PR Queue automation moved this from Assigned Reviewer to Reviewer Requested Changes Jan 24, 2021
- Remove unused context parameter.
- Rename functor::ReshapeSparseTensor to functor::ReshapeSparseTensorFunctor.
- Make integer constants const.
@gbaned gbaned requested a review from penpornk January 26, 2021 03:36
@gbaned gbaned added the awaiting review Pull request awaiting review label Jan 28, 2021
@benbarsdell
Copy link
Contributor Author

Gentle ping.

Copy link
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.

I'm sorry again for the delay!

tensorflow/core/kernels/reshape_util.cc Show resolved Hide resolved
PR Queue automation moved this from Reviewer Requested Changes to Approved by Reviewer Feb 4, 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 Feb 4, 2021
@penpornk penpornk removed the awaiting review Pull request awaiting review label Feb 4, 2021
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Feb 4, 2021
@copybara-service copybara-service bot merged commit 5eeb7c7 into tensorflow:master Feb 11, 2021
PR Queue automation moved this from Approved by Reviewer to Merged Feb 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes comp:core issues related to core part of tensorflow ready to pull PR ready for merge process size:M CL Change Size: Medium
Projects
PR Queue
  
Merged
Development

Successfully merging this pull request may close these issues.

None yet

6 participants