-
Notifications
You must be signed in to change notification settings - Fork 89
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
feat: to/from TensorFlow Tensor #3292
feat: to/from TensorFlow Tensor #3292
Conversation
Note: as discussed here when converting a TensorFlow Tensor into Numpy array with When converting Awkward Array to TensorFlow (while on CPU), this function: |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files
|
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.
This looks perfect! Thank you again! This looks complete, and if you agree, I'll merge it into main
.
On copying data from TensorFlow to NumPy: it might not be_technically_ necessary, since NumPy arrays have flags and can be marked as not writable. Thus, a NumPy array could zero-copy view a TensorFlow tensor in RAM (not GPU memory), if TensorFlow has stable pointers. Some systems, such as Java, copy data to different locations to defragment their memory use, which causes simple pointers (static integers) to get out of date. If TensorFlow does that, then it has to copy when converting to NumPy, even with NumPy's "writable" flag.
But this is probably out of our hands, since we have to rely on what functions TensorFlow gives us. They might have stable pointers that could be viewed as a non-writable NumPy array and yet just don't give us a function to do that—all we could do is ask them to add such a thing to their library (which we won't, because it's not such a big deal).
(If you agree and it's ready to be merged, write a comment, rather than an emoji, since I get emails for comments but not emojis. Thanks!) |
It looks done to me. I'm going to merge it. Thanks! |
No description provided.