-
Notifications
You must be signed in to change notification settings - Fork 4
Add MLModel Service for Android #4
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
Conversation
bhaney
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inital questions and comments - there aren't that many! I don't know java very well but the ML stuff is looking good. I'm going to test out your example first before I approve
...es/mlmodel-module/src/main/java/com/viam/sdk/android/examples/module/MNISTMLModelModule.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a nice cute function - takes care of all the re-shaping and RGB -> grayscale stuff
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah it's basically magic to me
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just for my own knowledge, could you tell me more about the manager, and its function? What is it managing, and why does it have to be an argument to the module?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A great explanation is here https://docs.djl.ai/docs/development/memory_management.html. I don't love it but basically we need it with this library. It's managing all the memory for NDArrays. I assume it handles pooling and what not. It also handles CPU <-> GPU memory transfers (and other Devices)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are all of these put commands copies of the data? No way to just assign the pointer to the dataList into the buffer?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On the from proto to ndarray side, no, unfortunately unless there's some special version of NDManager that can do it which I haven't found.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FWIW, the other libraries I used as well were also opinionated about copying data in.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand the copy to proto has to happen, but I'm curious is there is a way to keep from having to copy the data from the Proto message?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think these two comments are mixed up on directions since this one is to proto. For the toProto case, there's no clear way to set the data to a pointer other than to implement your own serializer. I think if we're worried about performance, it should be measured first. The NDManager may be good at being speedy and a good steward of memory usage.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea, didn't mean to put the comment on this one. I'm more worried about proto -> ndArray, as it seems like you could just use the data in the proto struct directly and copying would be unecessary. The NDManager does sound like a good lead if it turns out to be a problem
bhaney
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this! LGTM!
This introduces an MLModel service abstracted around https://djl.ai/ with an example implementation at android/examples/mlmodle-module. Since this type of code adds considerable size to the final APK, the following modules were added outside the core: viam-core-sdk-mlmodel-service, viam-java-sdk-mlmodel-service, and viam-android-sdk-mlmodel-service. A user wanting to use this in android would depend on
viam-android-sdk-mlmodel-serviceas it has the needed android specific shared objects and runtime dependencies.Inside a module, you use
com.viam.sdk.core.service.mlmodel.Registry.useService();to register the mlmodel service before constructing a module. This is needed due to the mlmodel java module being outside of the core SDK proper.NDArrays are backed by anNDManagerwhich can be set as a singleton in com.viam.sdk.core.service.mlmodel.MLModelRPCService#setNDManager. If not set,NDManager.newBaseManager()is used and it is passed intoinfermethods.More on the example:
Some python code to run against this if you have an image (there's one in the example resources directory):