snuspl / dolphin

14 stars 2 forks source link

Replace ND4J library with the new jblas-based matrix library #148

Closed beomyeol closed 8 years ago

beomyeol commented 8 years ago

This pull request replaces ND4J library with the new jblas-based matrix library. Here are the changes included in this PR.

The changes of this pull request make our DNN implementation lose the support of arbitrary dimensional inputs. I'll make another PR to restore this feature later.

This closes #142

jsjason commented 8 years ago

The tests have failed with

java.lang.UnsatisfiedLinkError: org.jblas.NativeBlas.sgemm(CCIIIF[FII[FIIF[FII)V

What's the problem?

jsjason commented 8 years ago

We needed to install libgfortran3 on the CI server to resolve the test failures.

beomyeol commented 8 years ago

For 64-bit Linux and Windows, additional libraries need to be installed for using jblas. https://github.com/mikiobraun/jblas/wiki/Missing-Libraries

jsjason commented 8 years ago

@beomyeol The new interface seems great. I left a few comments. Could you briefly explain why e5a6ca1 was needed?

beomyeol commented 8 years ago

@jsjason I unintentionally added the configuration files that use a group communication and a parameter server. But, the commit that included this change did not state these configurations files. Since these files are useful, I made another separate commit for these and pushed it forcedly.

beomyeol commented 8 years ago

e5a6ca1 is essential to compute the derivatives for the square function and the absolute function. The output values of these functions lose the signs of their input values. This change makes the computation of derivatives more expensive than before, because applying functions is needed one more time. However, this makes it possible to support all activation functions before the replacement of BLAS library.

beomyeol commented 8 years ago

@jsjason I've address your comments and left some comments. Please take another look :)

jsjason commented 8 years ago

I've tested this on several configurations and it works nicely. The code looks good, too. I'll merge it.