java
java copied to clipboard
Added Tensor.dataToString()
Fixes https://github.com/tensorflow/java/issues/268
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).
:memo: Please visit https://cla.developers.google.com/ to sign.
Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.
What to do if you already signed the CLA
Individual signers
- It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.
Corporate signers
- Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
- The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
- The email used to register you as an authorized contributor must also be attached to your GitHub account.
ℹ️ Googlers: Go here for more info.
@googlebot I signed it!
@rnett Please review.
Apologies in advance. The implementation got hairy once I added maxWidth support. Please refactor this as you see fit to improve readability.
The build failure doesn't seem to be related to my changes. Please confirm.
The quick build failing in javadoc generation is a known issue we're working on in other PRs.
Looks good, I have a few documentation requests and a request for float formatting. Did you consider adding an
Operand.dataToString()default method that callsasTensor().dataToString()(or similar)?
Yes, I could do that. Are you saying that you would leave the main implementation on Tensor and simply add a convenience method on Operand that redirects to it?
FYI, I decided to remove trailing commas after closing brackets. I think it looks better this way and I think that tf.print does the same. In order words:
[
[1, 2],
[3, 4],
[5, 6]
]
Will now show up as:
[
[1, 2]
[3, 4]
[5, 6]
]
Looks good, I have a few documentation requests and a request for float formatting. Did you consider adding an
Operand.dataToString()default method that callsasTensor().dataToString()(or similar)?Yes, I could do that. Are you saying that you would leave the main implementation on
Tensorand simply add a convenience method onOperandthat redirects to it?
Yeah, that's exactly what I meant.
All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.
We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.
Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).
ℹ️ Googlers: Go here for more info.
Other than the small comments, I made a PR (cowwoc#1) against your branch with additional data type tests, and (because of said tests) wrapping string elements in quotes. Use the PR if you want, but regardless, both of those would be good to have.
Merged. Thank you.
Let me know if there is anything else. Otherwise, how do we fix the quick-build so I can merge this PR?
The test failure will be fixed in #276. The Javadoc failures we're still working on. Once #276 is merged in then I'll rerun the action and check that the tests complete.
@googlebot I consent.
@JimClarke5 Any chance you could pick up the torch (apply the changes that Karl asked for) on my behalf? I won't have time to pick this up for another week or two.
@cowwoc OK, one problem I have is I need to work with my latest version of the xxxSession classes that I am using for Models and Layers, I assume you could cherry pick the Tensor related changes.
@cowwoc OK, one problem I have is I need to work with my latest version of the xxxSession classes that I am using for Models and Layers, I assume you could cherry pick the Tensor related changes.
@JimClarke5 You should be able to fork my PR, merge in your changes and push a new PR. I will approve it which should merge the changes back into this initial PR.
I understand that, but I need to test with my latest code first.
Will Operand.dataToString() work in Graph mode with out first doing a try catch?
try (Tensor tensor = session.runner().fetch(input).run().get(0)) {
tensor.dataToString()
}
Would we want the printed values enclosed in braces ( {} ) rather than square brackets( []}. Braces make it easier to copy and past into Java code.
I think we should pattern the output after java.util.Arrays.deepToString(), which takes us back to brackets with commas.
float[][][] f = new float[][][] {
{{1,2}, {3,4}},
{{5, 6}, {7, 8}},
{{9, 10}, {11, 12}}
};
System.out.println(Arrays.deepToString(f));
// [[[1.0, 2.0], [3.0, 4.0]], [[5.0, 6.0], [7.0, 8.0]], [[9.0, 10.0], [11.0, 12.0]]]
Will
Operand.dataToString()work in Graph mode with out first doing a try catch?try (Tensor tensor = session.runner().fetch(input).run().get(0)) { tensor.dataToString() }
Per https://github.com/tensorflow/java/issues/268#issuecomment-811663700 I was asked to only support eager mode. I suggest discussing this part with @rnett.
I think we should pattern the output after
java.util.Arrays.deepToString(), which takes us back to brackets with commas.float[][][] f = new float[][][] { {{1,2}, {3,4}}, {{5, 6}, {7, 8}}, {{9, 10}, {11, 12}} }; System.out.println(Arrays.deepToString(f)); // [[[1.0, 2.0], [3.0, 4.0]], [[5.0, 6.0], [7.0, 8.0]], [[9.0, 10.0], [11.0, 12.0]]]
My 2 cents: Make the default output human-friendly (i.e. square brackets as TF.print() and others do) and provide a Builder option that will request Java-style output. I find the indentation I provided above easier to read (for humans) than Java code, but I agree that the latter is useful functionality.
Per #268 (comment) I was asked to only support eager mode. I suggest discussing this part with @rnett.
That's for Operands, Tensors should work fine regardless (which is what was in Jim's example).
My 2 cents: Make the default output human-friendly (i.e. square brackets as
TF.print()and others do) and provide a Builder option that will request Java-style output. I find the indentation I provided above easier to read (for humans) than Java code, but I agree that the latter is useful functionality.
I'd agree with this: default to tf.print style but have options. I don't find the default Java style to be very readable for large tensors.
@rnett @cowwoc FYI, in Graph mode, accessing Operand.asTensor() will fail without calling the session fetch first.
@rnett @cowwoc FYI, in Graph mode, accessing
Operand.asTensor()will fail without calling the sessionfetchfirst.
Yeah, that's why we didn't have dataToString() do anything in Graph mode. It just calls asTensor().dataToString(), so it will do the same thing as asTensor().
@cowwoc Your branch is behind master and that is causing me build issues.
@JimClarke5 One sec, I'll merge master into it.
@JimClarke5 Try now.
Actually the print code is keying on inheriting from NDArray. It has nothing to do specifically with TType. That is why @karllessard ’s comment is throwing me off.
@JimClarke5 yeah maybe my comment wasn't clear. I was just pointing out that the dataToString() method could be added in TType instead of Tensor since only typed tensors are supported anyway. The dataToString() version for raw tensors is converting to a typed tensor before printing it, something the user could do directly if needed.
But I'm also fine leaving it there, in Tensor, if that makes things easier or if you think it make more sense.
Any updates on this? I am very new to Tensorflow and I have no idea how to visualize simple output. This seems perfect but it's been a year an a half since the last comment.
@ajs1998 you can print the value of any operand (Operand) using TF debug operations, such as tf.print(tf.strings.stringFormat(tensors...)) for example.
But if you want to print the values of a Tensor object itself without iterating through it, then that is still lacking afaik and should probably be added to the ndarray module instead. Could be an interesting and easy first PR for anyone interested to contribute to the project!