Skip to content
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

UINT8 is used but only partly supported in the Java API. #12797

Closed
andrewcmyers opened this issue Sep 4, 2017 · 4 comments
Closed

UINT8 is used but only partly supported in the Java API. #12797

andrewcmyers opened this issue Sep 4, 2017 · 4 comments
Labels
stat:contribution welcome Status - Contributions welcome

Comments

@andrewcmyers
Copy link
Contributor

The datatype uint8 exists in the Java API (see DataType.UINT8) and is used by the LabelImage program. However, it does not seem to be fully supported. In particular, elemByteSize() in tensor_jni.cc does not have a case for TF_UINT8, which prevents uint8 tensors from being created or extracted from TensorFlow.

andrewcmyers added a commit to andrewcmyers/tensorflow that referenced this issue Sep 4, 2017
…ully by the Java API,

addressing tensorflow#12797.
* Resurrected the uint8 test case.
* Allowed arrays of bytes to be used to construct both tensors of strings and tensors of uint8.
* Simplified the computation of the number of dimensions of a Java object representing a tensor.
@drpngx drpngx added the stat:contribution welcome Status - Contributions welcome label Sep 6, 2017
@drpngx
Copy link
Contributor

drpngx commented Sep 6, 2017

/CC: @asimshankar

It looks like you have a pending PR to fix this, LMK if you expect some action on our part.

@asimshankar
Copy link
Contributor

@andrewcmyers : Thanks for pointing this out. Do you want to package the commit in your repository as an independent PR?

@andrewcmyers
Copy link
Contributor Author

It's a bit entangled with the generics stuff. I could create a separate commit that fixes this problem, I guess. That would create some conflicts to iron out later.

@quaeler
Copy link
Contributor

quaeler commented Sep 7, 2017

The genesis of the fix is due to a merge down for the generics stuff, so it makes more sense (imo) to include it in the PR for the generics stuff and not as a postscript to it.

@yifeif yifeif closed this as completed in 8403a19 Sep 7, 2017
yifeif added a commit that referenced this issue Sep 7, 2017
sb2nov pushed a commit that referenced this issue Sep 29, 2017
* Phase 1 of the proposed generic Java API.

This adds new classes to represent each of the possible tensor types,
and some scripting support for generating those classes. There is
essentially no effect on existing classes, except that DataType is
made slightly more efficient.

All tests pass.

* Addressed Asim's review.

* Hoisted copyright into a separate declaration. Maybe it should go
in a separate file?

* Added private constructors to TF types and shortened their javadoc to be
more standard.

* Added more explanation about the enum relationship.

* Used more-idiomatic import statement.

* Rename zero column.

* Removed the datatype code from tftypes.csv

* Fix the default value for Double, add one for UInt8.

* Got rid of 'boxed type' column in CSV file

* Somehow I did not notice that TFType.java was not checked in.

* Phase 2 : Tensor, Output and friends are now parameterized.

* All tests now pass.

* Cleaned up and added some Javadoc and made some static fields private.

* Made Outputs more convenient to use.
Improved Javadoc regarding this functionality.
Added explicit type parameters to examples and tests to make them better models of expected practice.

* Removed extra copy of method.

* This change to the Android demo app should allow it to compile successfully

* Backed out unnecessary but presumably harmless removal of calls to clear().

* Change from Unicode times symbol to x, to be more consistent with
the rest of the Javadoc.

* Updated Constant and ConstantTest with generics.

* Registered UInt8 like all the other data types.

* Removed the UINT8 test because UINT8 doesn't seem to be fully supported in next
layer down. That probably should be fixed but it's orthongonal to this change.

* * Added some missing pieces so that uint8 seems now to be supported fully by the Java API,
addressing #12797.
* Resurrected the uint8 test case.
* Allowed arrays of bytes to be used to construct both tensors of strings and tensors of uint8.
* Simplified the computation of the number of dimensions of a Java object representing a tensor.

* Get rid of tab characters that violate the Google Java style guide. My IDE
was not configured correctly.

* Fix javadoc nit.

* Replace testUInt8 with the generic version.

* Ran formatter on code.

* Addressed some of Asim's comments.

- implemented constant() methods in terms of each other to reduce code duplication
- improved a spec regarding when types are checked
- got rid of an unnecessary method that used wildcards

* Back out change to comments in Operand.java

* This is what things look like if we make Tensor run on DataType as much as
possible. Only Tensor.expect() is still using class objects as a way to
represent tensor datatypes. It can be moved off to class Tensors when Tensors
exists, though it will not be as convenient as when it's a method of Tensor.

* Fixed build errors. This is is being committed primarily so Asim can take a look at it conveniently.
More work will be needed before merging.

* - Changed from TF-prefixed types to regular Java classes, e.g. Integer instead of
TFInt32. Deleted most classes in org.tensorflow.types, including TFType.
- Made Tensor mostly work in terms of Class<T> since that is the user-facing
  interface.
- Moved zeroValue() stuff off to the testfile where it belongs

* Remove unnecessary run-time check.

* Updated Android inference test to latest Java API changes.

* Address Asim's comments (thanks!)

- Removed now-gratuitous run-time type-check.
- Fixed non-Google-styled if.
- Reworded/fixed a few comments as requested.
- Removed all uses of unsafe casts and @SuppressWarnings in test cases.
- Cleaned up constant() implementations in LabelImage example.
- Removed reference to Tensors class (next PR!)

* Ran gformat on everything.

* Fixed an old typo in a comment.
Removed a couple of unnecessary casts from the example program.

* Fixed the last suppressed warnings.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stat:contribution welcome Status - Contributions welcome
Projects
None yet
Development

No branches or pull requests

4 participants