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

Protobuf support in TF Java #21

Closed
karllessard opened this issue Jan 22, 2020 · 3 comments
Closed

Protobuf support in TF Java #21

karllessard opened this issue Jan 22, 2020 · 3 comments

Comments

@karllessard
Copy link
Collaborator

Right now, the TensorFlow Java client from the repository does not expose directly protobufs that are part of the contract of the C API (Though in this thread, it was mentioned that protobufs would be eventually removed from the API but I have a feeling this won't happen anytime soon).

Instead, it compiles and distribute the protobufs Java bindings as a separate artifact and the client itself remains agnostic of the content of the messages and simply expose unmapped byte arrays, like here.

We need to decide how we want to handle those protobufs in the new distribution. Possible choices are:

  • We keep distributing the compiled protobufs as an external artifact like now from the core repository build
  • We keep distributing the compiled protobufs as an external artifact but from our new repository build
  • We compile and distribute the Java protobuf bindings within the new TF Java client artifact so they get exposed directly by our API
  • We compile the Java protobuf bindings with the new TF Java client but wrap them so they are not exposed directly by our API (in case they are removed in TF core, see previous thread link)
  • Any other idea?

Note that if we decide to compile the protobufs from the new TF Java client, it will brings its load of additional dependencies, such as grpc.

CC: @sjamesr

@saudet
Copy link
Contributor

saudet commented Jan 22, 2020

I think we should just add it pretty much as is as an additional module like tensorflow-proto under tensorflow-java. That would allow us to use it easily from other modules to create higher-level APIs using it, but still have it there for users until that happens. If/when the protocol buffers themselves are removed from the API, then we can think about deprecating it.

BTW, one major issue with protobuf on Java is that all versions use the same package name com.google.protobuf, but they are not compatible with each other, especially versions 2.x and 3.x, so we'll probably need to do some shading like this: https://github.com/eclipse/deeplearning4j/blob/master/nd4j/nd4j-backends/nd4j-api-parent/nd4j-api/pom.xml#L59-L111

@karllessard
Copy link
Collaborator Author

Interesting archeological discovery related to this issue, according to this unit test, protobuf Java bindings were meant to be distributed with the Java client but couldn't due to some complications with Bazel, hence the reason they were distributed separately.

@karllessard
Copy link
Collaborator Author

Resolved by #38

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants