Fixed bug with DEBUG_MODE bug when performing from_tensor_slices on a ragged tensor
Please don't use "add file"/"update file"/"fix file"/etc. commit messages. These are hard to reason about when looking at the history of the file/repository. Instead, please write explanatory git commit messages.

The commit message is also the title of the PR if the PR has only one commit. It is thus twice important to have commit messages that are relevant, as PRs would be easier to understand and easier to analyze in search results.

For how to write good quality git commit messages, please consult https://cbea.ms/git-commit/  Note that the proposed fix is not really a fix -- it just switches off eager mode for ragged tensor results, which is not what should happen.

Instead, the problem is really the `ops.convert_to_tensor` on the following line:
https://github.com/tensorflow/tensorflow/blob/06390d9be42502bb14c055be3978e5cd52a0f376/tensorflow/python/data/ops/structured_function.py#L240

The dataset elements can be ragged or sparse tensors, so this conversion must instead honor the type specs in `self._output_structure`. Maybe it is even the case that for ragged/sparse tensors, these must have been constructed explicitly in the mapping function and no further conversion is necessary (while for a usual tensor, the mapping function might return a Python list that needs to be converted to a tensor)?

Last but not least, such a change requires tests to verify that the fix works and that the problem will never appear again (and apart the ragged tensors mentioned in #60239, it should also include support for sparse tensors). Hi @jsparson1 Can you please check @mihaimaruseac's comments and keep us posted ? Thank you! > Hi @jsparson1 Can you please check @mihaimaruseac's comments and keep us posted ? Thank you!<

Yes, I can and I plan on it right now I have some other stuff I'm dealing with, I will be able to work on improving my fix and writing tests in the coming days, thank you all for your feedback. ข้อทราบข้อมูล ไฟล์งานที่คุณ ส่งคำขอดึง ระบบของเราด้วย
เพราะข้อมูลที่คุณส่งมา ทั้งหมด คือไฟล์งานของ ทีม ระบบ GitHub ทั้งหมด

ทีม Marketplace
Khaokho29th

เมื่อ 5 พ.ค. 2023 20:28 น. "Josh Parson" ***@***.***> เขียนว่า

Hi @jsparson1 <https://github.com/jsparson1> Can you please check
@mihaimaruseac <https://github.com/mihaimaruseac>'s comments and keep us
posted ? Thank you!
Yes I can and I plan on it right now I'm in the middle of exams, I will be
able to work on improving my fix in the coming days, thank you all for your
feedback.

—
Reply to this email directly, view it on GitHub
<https://github.com/tensorflow/tensorflow/pull/60250#issuecomment-1536256217>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ATYRUDCSE3NSIZMCFWHK5NDXET57DANCNFSM6AAAAAAWU3AWLI>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
 > Note that the proposed fix is not really a fix -- it just switches off eager mode for ragged tensor results, which is not what should happen.
> 
> Instead, the problem is really the `ops.convert_to_tensor` on the following line:
> 
> https://github.com/tensorflow/tensorflow/blob/06390d9be42502bb14c055be3978e5cd52a0f376/tensorflow/python/data/ops/structured_function.py#L240
> 
> The dataset elements can be ragged or sparse tensors, so this conversion must instead honor the type specs in `self._output_structure`. Maybe it is even the case that for ragged/sparse tensors, these must have been constructed explicitly in the mapping function and no further conversion is necessary (while for a usual tensor, the mapping function might return a Python list that needs to be converted to a tensor)?
> 
> Last but not least, such a change requires tests to verify that the fix works and that the problem will never appear again (and apart the ragged tensors mentioned in #60239, it should also include support for sparse tensors).

I just got an opportunity to look at it again and I actually think its the previous line
`print("TestStart")`
 `ret = structure.to_tensor_list(self._output_structure, ret)`
  `print("TestEnd")`


Note only TestStart prints out
TestStart
2023-05-10 22:16:10.385226: W tensorflow/core/framework/op_kernel.cc:1817] INVALID_ARGUMENT: ValueError: Value [1 2] is not convertible to a tensor with dtype <dtype: 'variant'> and shape ().
Traceback (most recent call last):

a possible fix could be padding the ragged tensors but that's kind hacky I will look into an alternative solution You are right that the crash already happens in `to_tensor_list`.

Problem is that the ragged tensor has already been converted using the https://www.tensorflow.org/api_docs/python/tf/raw_ops/RaggedTensorToVariant, so the `_output_structure` contains just "this is a Tensor of dtype variant" instead of "this is a ragged tensor". But maybe we could modify the `TensorSpec._to_components` to be able to handle such situation -- i.e., if it gets a ragged tensor and should convert it to `variant` dtype, it could use the above mentioned method to do it, instead of `ops.convert_to_tensor`? I just tried, but I could not make it work easily; maybe we should investigate more how exactly the processing works without the debug mode and handle the conversions ragged<->dense as it is done there. Hi @jsparson1 Any update on this PR? Please. Thank you! This PR is stale because it has been open for 14 days with no activity. It will be closed if no further activity occurs. Thank you. This PR was closed because it has been inactive for 14 days since being marked as stale. Please reopen if you'd like to work on this further.
Fixed bug with DEBUG_MODE bug when performing from_tensor_slices on a ragged tensor related to issue #60239

Thank you, if there are any suggestions feel free to comment.