Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
FillEmptyRows-16
operator specification #28046New 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
base: master
Are you sure you want to change the base?
FillEmptyRows-16
operator specification #28046Changes from 3 commits
7d810fc
e255fc9
f5f4d8e
2816e14
8415489
06c0dd0
25de91d
0d4c5c2
ebe5127
0c9d884
b3070b7
025966a
8a7ea6b
1bd2996
b85c7dc
54019ff
f459a3e
163e33e
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we really need generalization of tf.raw_ops.SparseFillEmptyRows? May be to consider only 2D sparse tensor that is only needed by TF?
I think ND case can be converged to 2D case by reshape
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If only 2D is needed, let's limit it. Applied.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To have the whole sparse tensor representation, shouldn't the
dense_shape
be an output as well?On the other hand, assuming it's the same as the input
dense_shape
maybe it's reduntant.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tf.raw_ops.SparseFillEmptyRows
doesn't return the outputSparseTensor
at all according to https://www.tensorflow.org/api_docs/python/tf/raw_ops/SparseFillEmptyRows#returns. Do you think we should have it?That's also interesting because there's
tf.sparse.fill_empty_rows
and then there'stf.raw_ops.SparseFillEmptyRows
, which have quite different outputs:https://www.tensorflow.org/api_docs/python/tf/sparse/fill_empty_rows
https://www.tensorflow.org/api_docs/python/tf/raw_ops/SparseFillEmptyRows
However the BST model I received is using the
raw_ops
one and in the graph theSparseTensor
is not returned.