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

Add PReLU layer #85

Merged
merged 6 commits into from
Jun 9, 2021
Merged

Add PReLU layer #85

merged 6 commits into from
Jun 9, 2021

Conversation

mkaze
Copy link
Contributor

@mkaze mkaze commented Jun 1, 2021

Resolves #54. This PR adds support for PReLU layer.

  • Implement layer class
  • Add unit tests for layer implementation
  • Add support for importing models containing this layer from JSON config files
  • Add support for exporting models containing this layer to JSON config files

@zaleslaw, I have a problem with testing my implementation. As you can see, I have written a unit test to start with; however, it fails with the following error:

org.tensorflow.TensorFlowException: Could not find valid device for node.
Node:{{node Assign}}
All kernels registered for op Assign :
  device='CPU'; T in [DT_QUINT16]
  device='CPU'; T in [DT_QINT32]
  device='CPU'; T in [DT_QUINT8]
  device='CPU'; T in [DT_QINT8]
  device='CPU'; T in [DT_VARIANT]
  device='CPU'; T in [DT_RESOURCE]
  device='CPU'; T in [DT_STRING]
  device='CPU'; T in [DT_BOOL]
  device='CPU'; T in [DT_COMPLEX128]
  device='CPU'; T in [DT_COMPLEX64]
  device='CPU'; T in [DT_DOUBLE]
  device='CPU'; T in [DT_FLOAT]
  device='CPU'; T in [DT_BFLOAT16]
  device='CPU'; T in [DT_HALF]
  device='CPU'; T in [DT_INT8]
  device='CPU'; T in [DT_UINT8]
  device='CPU'; T in [DT_INT16]
  device='CPU'; T in [DT_UINT16]
  device='CPU'; T in [DT_INT32]
  device='CPU'; T in [DT_INT64]

	at org.tensorflow.EagerOperationBuilder.execute(Native Method)
	at org.tensorflow.EagerOperationBuilder.build(EagerOperationBuilder.java:35)
	at org.tensorflow.EagerOperationBuilder.build(EagerOperationBuilder.java:24)
	at org.tensorflow.op.core.Assign.create(Assign.java:94)
	at org.tensorflow.op.Ops.assign(Ops.java:887)
	at org.jetbrains.kotlinx.dl.api.core.initializer.Initializer.apply(Initializer.kt:40)
	at org.jetbrains.kotlinx.dl.api.core.layer.Layer.addWeight(Layer.kt:127)
	at org.jetbrains.kotlinx.dl.api.core.layer.activation.PReLU.build(PReLU.kt:69)
	at org.jetbrains.kotlinx.dl.api.core.layer.PReLUTest.default(PReLUTest.kt:39)
        ...

It seems the error happens in build method when it wants to initialize and add the weights of the layer. I tried to investigate what the issue is, but couldn't figure it out further... this happens even if I use Dense layer instead of PReLU in the unit test! I am not sure what I am missing?! Surely, you can help. Thanks!

@zaleslaw
Copy link
Collaborator

zaleslaw commented Jun 1, 2021

Looks like something wrong with types, maybe usage int or long or double instead of float
I found only this link related to the subject
Try to find such a place in your code, if you found nothing related I'll try to debug your solution on the next week

@mkaze mkaze changed the title [WIP] Add PReLU layer Add PReLU layer Jun 2, 2021
@mkaze
Copy link
Contributor Author

mkaze commented Jun 2, 2021

@zaleslaw I just fixed the tests and this is ready to be reviewed. It seems I had to use Graph mode instead of Eager mode to be able to correctly build the layer and run a forward pass on it; so I took inspiration from ConvLayerTest.kt to implement the tests and they are now working.

@zaleslaw
Copy link
Collaborator

zaleslaw commented Jun 2, 2021

The eager mode in 1.15 was unstable and has a lot of bugs, I've used it for testing purposes only

@zaleslaw zaleslaw added Review This PR is under review LGTM PR reviewed and is ready to merge and removed Review This PR is under review labels Jun 8, 2021
@zaleslaw
Copy link
Collaborator

zaleslaw commented Jun 8, 2021

Hi, @mkaze please update your PR from master branch, be careful with weights (it's converted to var) and should have "get" and "set" methods to extract and assign weights.

Notify me here and I will run TC builds

@mkaze
Copy link
Contributor Author

mkaze commented Jun 9, 2021

@zaleslaw I merged it with master branch and updated the weights.

By the way, out of curiosity, are you the only one from JetBrains who is working on this project full-time? I was just wondering how much committed JetBrains is to push forward this project.

@zaleslaw
Copy link
Collaborator

zaleslaw commented Jun 9, 2021

@mkaze It's easy to see in the commit history:) Yes, I'm the only developer from JetBrains who assigned KotlinDL full-time. But, of course, not only my brains are committed to this project. Also, I'm part of Kotlin for Data Science Team, we are working together on Mutlik, data frame, lets-plot-dsl, and Kotlin kernel for Jupyter Notebook. I hope, if the project will be successful, the team will be extended, and the contributor community will grow!

@zaleslaw zaleslaw merged commit 92f87e8 into Kotlin:master Jun 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
LGTM PR reviewed and is ready to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add PReLU layer
2 participants