Java bindings #2100

Vasniktel · 2019-11-29T11:31:46Z

Implementation of automatic bindings generator for Java.
We are using Nd4j on the Java side to work with matrices.
Types are being mapped as follows:

arma::Mat<T> is mapped to INDArray class of Nd4j library.
std::vector<T> is mapped to List<T>
int -> Integer
double -> Double
std::string -> String

If you want to try it yourself:

Clone the repo.
mkdir build && cd build
cmake -D BUILD_JAVA_BINDINGS=1 ..
make java
cd src/mlpack/bindings/java/maven/target

There must be a *.jar file in the directory. You can use it to run generated bindings. Don't forget to put it and its dependencies (https://mvnrepository.com/artifact/org.nd4j/nd4j-api/1.0.0-beta5) in your classpath.

TODO:

Make better docs
Generate .jar file with native libraries
Make it run in CI

mlpack-bot · 2019-11-29T11:31:50Z

Thanks for opening your first pull request in this repository! Someone will review it when they have a chance. In the mean time, please be sure that you've handled the following things, to make the review process quicker and easier:

All code should follow the style guide
Documentation added for any new functionality
Tests added for any new functionality
Tests that are added follow the testing guide
Headers and license information added to the top of any new code files
HISTORY.md updated if the changes are big or user-facing
All CI checks should be passing

Thank you again for your contributions! 👍

rcurtin

@Vasniktel, this is a truly impressive effort. Adding a binding for another language is a lot of work. I'm really excited to add these. I think that the review process will take a lot of time, though; since we're basically adding an entire new interface to mlpack, lots of testing and review have to happen in order to make it all fit. I have to apologize that I've been slow about this initial review---the PR itself is pretty intimidating. :)

My review here is pretty initial---it's likely as we dig deeper, we'll find more things to change or fix or what.

So far, I've checked out the bindings and built them locally and run the tests, which all pass fine. From my end I need to do the following before I think everything is ready for release:

high-level structural review of the code (it would probably be good to take a second pass though)
fine-grained review of all the code
test build documentation for the website and deploy
try to do some data science with mlpack from Java
understand how the Java bindings would fit in with Scala and how we can cover that also
figure out what packaging the Java bindings look like (plus Scala too?)
make sure tests run on different OSes (Linux/OS X/Windows)

Many of the comments I make during this process might be outside the scope of this PR, so we can always open issues for them that can be handled later. I'm sure we won't get everything perfect right after the first merge, but I think we can get most of the way there. :)

src/mlpack/bindings/java/get_java_type.hpp

src/mlpack/bindings/java/CMakeLists.txt

src/mlpack/bindings/java/print_output_param_impl.hpp

rcurtin · 2019-12-30T02:47:18Z

src/mlpack/core/data/scaler_methods/pca_whitening.hpp

@@ -14,6 +14,7 @@

 #include <mlpack/prereqs.hpp>
 #include <mlpack/core/math/lin_alg.hpp>
+#include <mlpack/core/math/ccov.hpp>


I think we removed this some time ago, my guess is this is just a merge artifact. But we shouldn't re-add ccov.hpp because it doesn't exist anymore. :)

Partly fixed in 6574dc8. I was unable to get it compiled without adding these and some other includes. Particularly, it says that mlpack::math::ColumnCovariance is not defined (not sure what the problem is)

rcurtin · 2019-12-30T02:47:51Z

src/mlpack/core/util/mlpack_main.hpp

+#elif(BINDING_TYPE == BINDING_TYPE_JAVA) // This is a Java binding.
+
+// TODO: not sure where we use this
+#define BINDING_MATRIX_TRANSPOSED true


This is only used for the NMF binding, because it makes a difference which matrix is W and which is H when the binding operates on transposed or non-transposed data.

Thanks for the insight :) I'm not sure though what value I should set it to for Java

rcurtin · 2019-12-30T02:48:57Z

src/mlpack/methods/logistic_regression/logistic_regression_function.hpp

@@ -15,6 +15,8 @@
 #define MLPACK_METHODS_LOGISTIC_REGRESSION_LOGISTIC_REGRESSION_FUNCTION_HPP

 #include <mlpack/prereqs.hpp>
+#include <mlpack/core/math/make_alias.hpp>
+#include <mlpack/core/math/shuffle_data.hpp>


I'm also not sure that this change is needed.

Partly fixed in 6574dc8.

src/mlpack/bindings/java/print_input_param_impl.hpp

zoq · 2020-01-24T00:53:19Z

Hello @Vasniktel do you think you could update the linux build steps: https://github.com/mlpack/mlpack/blob/master/.ci/linux-steps.yaml to enable the java bindings?

Vasniktel · 2020-02-15T21:25:50Z

Time for a small status report ;)

I've been working out the way bindings are being generated. We are compiling bindings for every method separately right now (every *.so file is being compiled separately by cmake script after the appropriate JNI wrapper file was generated by JavaCPP library). The whole process looks like this:

compile a program to generate *.java binding file (this stage is the same in all bindings generators)
generate that *.java file
run Java compiler to compile *.java file to bytecode
run JavaCPP on the compiled bytecode to get JNI *.cpp wrapper file
compile wrapper file to a shared library

Disadvantages of this approach are:

it looks too complicated to me
the compilation is slow
generation of *.jar files is probably gonna be tricky as well

An alternative approach could be to make JavaCPP do all the hard work. In that way, the process reduces to the following:

compile generator program for every method in the library
generate *.java classes for all these methods
run maven with JavaCPP plugin to compile all shared libraries and package them in a *.jar file

The only downside is that we have to pass all compiler flags to JavaCPP maven plugin to make sure bindings are compiled correctly. I'm working on this new process right now.

Meanwhile, I've noticed that usage of move assignment operator for arma::mat(data, n_rows, n_cols, false, true); actually copies memory. A solution would be to pass false as the last value to the constructor. This issue arises in both julia and go bindings

mlpack-bot · 2020-03-23T10:21:00Z

This issue has been automatically marked as stale because it has not had any recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions! 👍

Vasniktel · 2020-03-23T10:24:21Z

This issue has been automatically marked as stale because it has not had any recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions!

Hey, @rcurtin. It would be good to keep this issue open ;)

rcurtin · 2020-03-24T02:05:54Z

@Vasniktel agreed, I'm sorry I haven't managed to review this. Things have been crazy. :(

Vasniktel · 2020-03-24T19:52:07Z

@Vasniktel agreed, I'm sorry I haven't managed to review this. Things have been crazy. :(

That's ok ;). I don't have much time to work on this myself.

Yashwants19 · 2020-10-06T08:56:22Z

@Vasniktel if you want I can help you in this, or else if you don't enough time I can open a new PR for completing this. :)

Vasniktel · 2020-10-07T09:02:48Z

@Yashwants19 Thanks. It would be great if you could take this over. I don't think I will be able to finish this any time soon :( Sorry for the long delay.

caballeto · 2021-03-09T22:34:11Z

Is this coming any time soon?

rcurtin · 2021-07-28T01:59:08Z

I'd like to keep this one open---it is really nice code, it just needs to be reviewed and merged. I hope to have the time sometime...

Vasniktel added 18 commits November 1, 2019 17:22

Initial commit in Java bindings

f46f48f

Make Java binding for cli util compile

837a47c

Removed *.class files

7803774

Updated .gitignore

ab85f8c

Remove VSCode related files

18f829a

Fix javacpp generation bug

8c6ed19

Add missing includes

9cb524c

Add more utility functions; fix bugs

a86f37c

Add unittests

06bf060

Add more bindings; use nd4j 1.0.0-beta5

09ada4d

Update tests

19ac81f

Add support for model parameters

2a3b08c

Add support for categorical matrices

6f565d7

Add support for binding generation

5aa8ebc

Implement ProgramCall

a042098

Fix minor bugs

c01572d

Add bindings for methods; fix compilation bugs

2962965

Introduce JAVA_VERSION cmake variable

c63c599

mlpack-bot bot added s: needs review s: unanswered s: unlabeled labels Nov 29, 2019

Vasniktel mentioned this pull request Nov 29, 2019

Java bindings #2055

Closed

zoq added c: binding t: added feature and removed s: unanswered s: unlabeled labels Nov 29, 2019

Vasniktel added 2 commits November 30, 2019 10:25

Fix style issues

4567b12

Fix bugs for row and col bindings

ba7dd79

rcurtin reviewed Dec 30, 2019

View reviewed changes

Vasniktel added 2 commits January 10, 2020 13:49

Fix cmake issues

ffdd622

Remove unused includes

6574dc8

Vasniktel added 9 commits February 21, 2020 14:21

Remove needless templates

e3aef86

Fix indentation

77d28a5

Rename utility methods to be similar to java types

ba37472

Add support for custom prefix to mlpack::util::HyphenateString

b40716e

Improve generation process

81ab8ee

Fix incorrect arguments

876a580

Don't use auto

e495358

Improve docs

1741959

Make target specification optional

3b89355

mlpack-bot bot added the s: stale label Mar 23, 2020

favre49 added s: keep open and removed s: stale labels Mar 23, 2020

conradsnicta added s: stale and removed s: keep open labels Jul 21, 2021

mlpack-bot bot closed this Jul 28, 2021

rcurtin reopened this Jul 28, 2021

rcurtin added s: keep open and removed s: stale labels Jul 28, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Java bindings #2100

Java bindings #2100

Vasniktel commented Nov 29, 2019 •

edited

mlpack-bot bot commented Nov 29, 2019

rcurtin left a comment

rcurtin Dec 30, 2019

Vasniktel Jan 10, 2020

rcurtin Dec 30, 2019

Vasniktel Feb 15, 2020

rcurtin Dec 30, 2019

Vasniktel Jan 10, 2020

zoq commented Jan 24, 2020

Vasniktel commented Feb 15, 2020

mlpack-bot bot commented Mar 23, 2020

Vasniktel commented Mar 23, 2020

rcurtin commented Mar 24, 2020

Vasniktel commented Mar 24, 2020

Yashwants19 commented Oct 6, 2020

Vasniktel commented Oct 7, 2020

caballeto commented Mar 9, 2021

rcurtin commented Jul 28, 2021

Java bindings #2100

Are you sure you want to change the base?

Java bindings #2100

Conversation

Vasniktel commented Nov 29, 2019 • edited

mlpack-bot bot commented Nov 29, 2019

rcurtin left a comment

Choose a reason for hiding this comment

rcurtin Dec 30, 2019

Choose a reason for hiding this comment

Vasniktel Jan 10, 2020

Choose a reason for hiding this comment

rcurtin Dec 30, 2019

Choose a reason for hiding this comment

Vasniktel Feb 15, 2020

Choose a reason for hiding this comment

rcurtin Dec 30, 2019

Choose a reason for hiding this comment

Vasniktel Jan 10, 2020

Choose a reason for hiding this comment

zoq commented Jan 24, 2020

Vasniktel commented Feb 15, 2020

mlpack-bot bot commented Mar 23, 2020

Vasniktel commented Mar 23, 2020

rcurtin commented Mar 24, 2020

Vasniktel commented Mar 24, 2020

Yashwants19 commented Oct 6, 2020

Vasniktel commented Oct 7, 2020

caballeto commented Mar 9, 2021

rcurtin commented Jul 28, 2021

Vasniktel commented Nov 29, 2019 •

edited