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

Added PRelu to CoreML emitter. #108

Merged
merged 1 commit into from Mar 15, 2018
Merged

Conversation

galli-leo
Copy link
Contributor

Note: I haven't tested yet if this gives the correct output, but I am assuming it should.

Also, you should consider adding .DS_Store to the .gitignore, since these files are very prevalant when working on a mac.

@kitstar kitstar merged commit 8b343f6 into microsoft:master Mar 15, 2018
@kitstar
Copy link
Contributor

kitstar commented Mar 15, 2018

Thanks @galli-leo !

@galli-leo
Copy link
Contributor Author

Might have been a premature merge 😅 . Just started testing the conversion of insightface and outputs seem to differ by a lot from CoreML and MXNet version. Do you have an idea on the orders of parameters MXNet and CoreML have for activation layers? Or do you know an easy way to check that?

@kitstar
Copy link
Contributor

kitstar commented Mar 15, 2018

Did you try if the MXNet->MXNet conversion works?

@galli-leo
Copy link
Contributor Author

Ah that's a good idea, will try shortly.

@galli-leo
Copy link
Contributor Author

Ok, so MXNet-> MXNet works (I had to add PRelu to the emitter though). MXNet -> CoreML still doesn't :/ Not sure if that's because of PRelu or something else. Last time I converted Torch -> CoreML (using a different framework), I had issues with the Batchnorm layers. Did you verify whether those pass the correct parameters?

@kitstar
Copy link
Contributor

kitstar commented Mar 15, 2018

I haven't tested the CoreML emitter well (Formal test is on the way), just tested with VGG, inception networks from TF.

@galli-leo
Copy link
Contributor Author

galli-leo commented Mar 15, 2018

So I tried removing the PRelu layers entirely from both MXNet and CoreML models and the results are still way off :/ At least we know it's not solely the PRelu layer. I think the problem lies with the batchnorm layer, though I haven't found out what.

Edit: I definitely think somethings wrong with the BatchNorm layer. I saw someone copied over these lines: https://github.com/Microsoft/MMdnn/blob/master/mmdnn/conversion/coreml/coreml_emitter.py#L564-L569 from the keras implementation of coremltools. No idea why they are there though. I tried using the real mean and var (not setting them to 0 and 1 respectively), but that just resulted in nan as output of the network. Do you have any idea why that would happen? Also do you have any idea what aforementioned code exactly does? AFAIK CoreML and Keras implementation of BatchNorm should be the same.

@galli-leo
Copy link
Contributor Author

A small update: Doing MXNet -> Caffe (using this repo) then doing Caffe -> CoreML via coremltools, results in even more different results (all values seem to be in the million range for some reason).

@galli-leo
Copy link
Contributor Author

galli-leo commented Mar 21, 2018

Finally got it to work using: https://github.com/GarrickLin/MXNet2Caffe. So something is definitely off in the caffe and coreml emitters. Or maybe the error happens when reading the mxnet stuff (i.e. wrong conversion of variables) and then just works again since the emitter also makes some wrong assumptions?

Hmm after comparing both the prototxt generated here and the one generated from the GH above, the only difference I can see, is that you don't use bias_terms for convolution. (Btw. sorry for spamming your Github, but my observations might help someone 😬)

bias_term does not help when converting to caffe and I cannot seem to find any other differences, so I am guessing the weights are somehow wrong.

@galli-leo
Copy link
Contributor Author

I finally managed to get MXNet -> Caffe working with MMdnn. Had to do some manual editing in the generated file. If it is desired, I can try and implement these changes in the caffe emitter? Mostly just adding support for PReLU and fixing Batchnorm with scaling.

MXNet -> CoreML is still not working, though I am suspecting this is due to the Batchnorm stuff. I asked the coremltools dev what exactly they are doing with the keras conversion and then I can try and fix that too.

@kitstar
Copy link
Contributor

kitstar commented Mar 22, 2018

Many thanks! Please help us to improve CaffeEmit! And To CoreML part is under testing these days. We will ping you if coreml test and modification is finished. Then we can go on the MXNet -> CoreML part.

Thanks again!

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

2 participants