Skip to content

Conversation

jrmerwin
Copy link
Contributor

What changes were proposed in this pull request?

(Please fill in changes proposed in this fix)

How was this patch tested?

(Please explain how this patch was tested. E.g. unit tests, integration tests, manual tests)

Please review
https://github.com/deeplearning4j/deeplearning4j/blob/master/CONTRIBUTING.md before opening a pull request.

@crockpotveggies
Copy link
Contributor

@saudet if you could take a quick look and give your blessing I'll get this merged.

testCompile 'junit:junit:4.12'

//Image loading dependencies
compile 'org.datavec:datavec-data-codec:0.9.1'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need datavec-data-codec here, please use datavec-data-image instead to reduce size.

Also, binaries of OpenCV, Leptonica, and HDF5 for other platforms are what is taking most of the space here. We should add exclude lines for opencv-platform, leptonica-platform, and hdf5-platform, and add back maybe just the binaries for OpenCV on Android.

And please add these instructions (datavec-data-image and platform artifact exclusion) to the docs at deeplearning4j/deeplearning4j-docs#7. Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed the image loading dependency to datavec-data-image, excluded leptonica and hdf5, and left opencv. The tutorial documents and demo code have been updated.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! But we need to exclude opencv-platform as well, or we'll get binaries for iOS, Linux, Mac, and Windows. This is pretty big. We can reinclude only the one for Android after with something similar to ND4J and OpenBLAS:

        compile 'org.bytedeco.javacpp-presets:opencv:3.2.0-1.3'
        compile 'org.bytedeco.javacpp-presets:opencv:3.2.0-1.3:android-x86'
        compile 'org.bytedeco.javacpp-presets:opencv:3.2.0-1.3:android-arm'

Changed 'datavec-data-codec' to 'datavec-data-image' and excluded platform specific files to reduce overall size.
Using Deeplearning4J in Android Applications

This tutorial demonstrates how the Java-based deep learning library Deeplearning4J can use a trained neural network in Android Applications. The application loads a neural network trained on the Mnist data set and uses it to identify user drawn numbers as input. The key features of this demo include storing and accessing a trained model stored in the application’s Raw folder, internally saving an user-generated image, and accessing and converting that image for testing against the network.
A tutorial for the application can be found [here](https://github.com/jrmerwin/Skymind-Android-Documentation/blob/master/DL4JImageRecognitionDemo.md#head_link6).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update link!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated

testCompile 'junit:junit:4.12'

//Image loading dependencies
compile 'org.datavec:datavec-data-codec:0.9.1'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! But we need to exclude opencv-platform as well, or we'll get binaries for iOS, Linux, Mac, and Windows. This is pretty big. We can reinclude only the one for Android after with something similar to ND4J and OpenBLAS:

        compile 'org.bytedeco.javacpp-presets:opencv:3.2.0-1.3'
        compile 'org.bytedeco.javacpp-presets:opencv:3.2.0-1.3:android-x86'
        compile 'org.bytedeco.javacpp-presets:opencv:3.2.0-1.3:android-arm'

// Context of the app under test.
Context appContext = InstrumentationRegistry.getTargetContext();

assertEquals("com.example.jmerwin.dl4jimagerecognitiondemo", appContext.getPackageName());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rename package to org.deeplearning4j.examples!

compile 'org.nd4j:nd4j-native:0.9.1'
compile 'org.nd4j:nd4j-native:0.9.1:android-x86'
compile 'org.nd4j:nd4j-native:0.9.1:android-arm'
compile 'org.bytedeco.javacpp-presets:systems-platform:1.4'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update dependencies, excludes, here and below as well, but this looks like a copy/paste of the docs?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I forgot there were copies of the tutorials in the demo folders. If it is OK with you I will just remove them since there is a link to the tutorial in the readme. Then I won't have to keep two copies up to date.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, sounds good, let's just keep the links. Thanks!

@jrmerwin
Copy link
Contributor Author

jrmerwin commented Apr 5, 2018

Changed the package names in both demo apps, excluded opencv in the ImageDemo followed by include statements for android specific opencvs. Updated the demo tutorial with the new exclusions.

@saudet
Copy link
Contributor

saudet commented Apr 9, 2018

Thanks! Looks good. Just one last thing: The README.md file is empty. It might make sense to just merge the 2 ones found in the project subdirectories? Anyway, it's a really minor point. I'm good with merging this for now and let's get this updated for 1.0.0-alpha first. :)

@saudet saudet merged commit 9ddd3c9 into deeplearning4j:master Apr 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants