SparseFillEmptyRows Op #7442
SparseFillEmptyRows Op #7442
Conversation
|
@mbrookhart @tkonolige Please review this Op. it has been completely rewritten to address the many issues in the previous PR #7126 and has also been tested with the customer model. |
There was a problem hiding this comment.
Hi Ricky, thanks for submitting this. I've left some comments for you to address. Also, could you add more documentation and comments. Especially to the compute and shape functions. They are quite involved, so comments would help future readers of the code to understand them.
|
@tkonolige I have addressed all your comments and added the relevant documentation/changes wherever you asked. Please re-review. |
There was a problem hiding this comment.
I still wouldn't say I fully understand the op, so I'm going to assume your tests cover what you need them to :D
With that caveat, the overall structure looks good to me, good work. No change requests on my side, I'll approve/merge if/when @tkonolige does.
|
@tkonolige . Addressed all your comments and added test cases for int32. Please re-review. |
* Initial Commit * Fix formats * Remove comments * Black * THreeops * Add Frontend Code * Add Default Value to feed dict * Add Frontend Code * New test Cases and new code to handle them * Add Python Implementation' ' * Remove stuff * Remove unused imports * Pylint * Pylint * PyLint Shape Func * Make tests cpu only * Add unsorted tests * Add frontend code * Row Major Sorting Only Test * Handle Dynamic Shapes * Add dynamic input shapes * Dynamic Shape Tests * Add documentation * Dtypes * PR Comments * Added comments and changed naming * Add comments * Comments to Shape Func * Documentation * PR Changes * PR Comments * Resolve input and output dtype compat Co-authored-by: Ubuntu <ubuntu@ip-172-31-42-251.us-east-2.compute.internal>
* Initial Commit * Fix formats * Remove comments * Black * THreeops * Add Frontend Code * Add Default Value to feed dict * Add Frontend Code * New test Cases and new code to handle them * Add Python Implementation' ' * Remove stuff * Remove unused imports * Pylint * Pylint * PyLint Shape Func * Make tests cpu only * Add unsorted tests * Add frontend code * Row Major Sorting Only Test * Handle Dynamic Shapes * Add dynamic input shapes * Dynamic Shape Tests * Add documentation * Dtypes * PR Comments * Added comments and changed naming * Add comments * Comments to Shape Func * Documentation * PR Changes * PR Comments * Resolve input and output dtype compat Co-authored-by: Ubuntu <ubuntu@ip-172-31-42-251.us-east-2.compute.internal>
This PR adds SparseFillEmptyRows Op to relay and corresponding frontend code for TF framework.
TF Op Docs. This Op is only supported for CPU targets now which is what the customer models are running on. So GPU implementation is not a part of this PR and can be added later if needed.