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

Adding layers (based on keras) supporting multiple outputs #65

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dhruvrajan
Copy link
Contributor

@dhruvrajan dhruvrajan commented May 29, 2020

This draft PR is tracking the implementation of keras layers in Java; will update with a README once the API is complete, and mark for review when it's ready.

@karllessard
Copy link
Collaborator

@dhruvrajan , can you please provide some feedbacks for the status of this PR? Do you still intend to make it official?

@KartikChugh
Copy link

I might be interested in picking this up if there is interest in this

@karllessard
Copy link
Collaborator

Hi @KartikChugh , there is certainly interest in this and happy if you could help!

We did not hear from @dhruvrajan for awhile so I don't know if he was still planning to complete his work. But @JimClarke5 is another member of this community who is working actively in the new Keras layer, we should probably sync up with him. Jim, did you already start to work on the layers? How would you like Kartik to help?

@JimClarke5
Copy link
Contributor

Thanks @karllessard

@KartikChugh, I welcome the offer to help.

I am in the early stages of looking at layers. My game plan is to build Input, Dense, and Flatten, and this will drive flushing out the base Layer class, as well as Model and Sequential. I am working in my own repository and can push what I currently have locally to it, JimClarke5/tensorflow-java-keras. You can start looking at this repository to get an understanding on how I am approaching the Keras port. The plan is to then move these classes over to the main repository. I have already created the first PR on the Optimizers to check into the main repository and as I expected this is creating a lot of discussion on changes to the other Java modules.

@deansher
Copy link
Contributor

deansher commented Aug 16, 2020

I would also be interested in helping. I have limited time and I'm a deep learning beginner. From looking at JimClark5/tensorflow-java-keras/ src/test/java/org/tensorflow/keras/optimizers/AdamTest.java, It appears that testBasic() is a transliteration of the corresponding Python Keras test. Perhaps a good contribution for me to make would be to continue transliterating tests in this fashion? I could code them to use classes and interfaces that @JimClarke5 and others have built out, while scaffolding whatever doesn't exist yet?

I imagine our goal would be to transliterate Python Keras pretty closely? Like if I looked at a Python Keras program and I'm a Java expert, I should be able to imagine the idiomatic Java way to code those things and that's the API I can expect? And then I should expect the exact same calls on TensorFlow from the Java version as from the Python version?

@karllessard
Copy link
Collaborator

Transliterating directly from Python code might not be that straightforward in all cases, as the TF Python client used to exist way before Keras was added to the project so it has a lot of high-level (and deprecated?) APIs that wraps up the raw operators and can bring some complexities that might not be required for Keras usage. Since in TF Java we basically only deal with raw operators (with a few exceptions), we need to identify what is really required by Keras out of these wrappers and discard the rest, at least for now... but other than that, yes, that's pretty much it!

I'm not taking part of the Keras Java library so much so far but if I can help you to synchronize your work, by creating a branch in the repo or speeding up some PRs (yes sorry for that @JimClarke5 ;) ), just let me know and I will be pleased to assist. Also note that the second-monthly SIG meeting will be pretty much be dedicated for Keras Java development in the future, that can also help with the whole collaboration process.

@JimClarke5
Copy link
Contributor

I haven’t looked at multiple inputs/outputs closely, but have designed my layer base class to accept them, based on the python Layer class. Dense and Flatten only accept 1 input/output. I would be glad to see the USE cases though.

@JimClarke5
Copy link
Contributor

As far as test cases, I am copying test cases from python when it makes sense. Some python test cases use numpy for comparison, which is not concise when porting directly to Java. Some tests don’t exist, so I have to create them from scratch.

I find that I first port the Python Keras code over, then immediately do the test case, that in turn results in fixing the original port of the Java Keras code.

Some python ism’s do not port easily, like accessing tensor values using overloaded operators.

@KartikChugh
Copy link

I've heard from Dhruv that the JS code is much better to port over

@karllessard
Copy link
Collaborator

Just a quick mention that anyone interested in participating to the development of Keras API for Java is invited to join our monthly meeting about this topic, next session is Friday August 28th at 9am PST and you can find the agenda and links here. Thanks!

@JimClarke5
Copy link
Contributor

@karllessard, @Craigacp : In my work analyzing Callbacks, I have discovered a lot of dependencies on Model and Layers, and TensorBoard assumes a Keras model is exported in JSON Keras format from the TensorBoard callback.

As a result, I am starting initial work on a version of Models and Layers. I will probably do it in phases with the first phase including framework.layers.InputLayer, framework.layers.Dense, framework.layers.Flatten, as well as the initial framework.models.Model and framework.models.Sequential classes. Do we still want this PR, #65? Or, just create a new one when I get my first working Phase 1 version done ? My currentModel/Layersbranch depends on Metrics (Phase 1 and 2) and the new #174 TTypes.

My initial approach is to mimic Keras, but maybe we should have some discussion on this.

@karllessard
Copy link
Collaborator

Hey @JimClarke5 , sounds good to me, I suggest we start a new PR and close this one.

@JimClarke5 JimClarke5 mentioned this pull request May 6, 2021
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

Successfully merging this pull request may close these issues.

None yet

5 participants