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

Support for Linear Algebra ops #173

Open
wants to merge 17 commits into
base: master
Choose a base branch
from
Open

Conversation

mandar2812
Copy link

@mandar2812 mandar2812 commented Oct 1, 2019

Linear Algebra Support

Created a starting point for linear algebra support (solving #84). Added eager and lazy versions of ops.

Also added new type check IsRealOrComplex to check if underlying type of a tensor or output is float, double or complex.

Ops Added

  • Cholesky
  • Matrix Determinant
  • Log Matrix Determinant
  • Matrix Inverse
  • Matrix Solve A\B
  • Least squares matrix solve
  • Triangular matrix solve
  • Eigen-decomposition of self-adjoint matrices
  • QR decomposition
  • SVD decomposition
tf.matrixDeterminant[T: TF: IsRealOrComplex](
  matrix: Output[T], 
  name: String = "MatrixDeterminant"): Output[T]

tfi.matrixDeterminant[T: TF: IsRealOrComplex](matrix: Tensor[T]): Tensor[T]

@eaplatanios @sbrunk @lucataglia @DirkToewe : What do you guys think?

@mandar2812
Copy link
Author

Forgot to commit the modification to build.sbt, adding it here.

  -- Added logMatrixDeterminant op
  -- Added matrix inversion op.
  -- Added matrix solve function
  -- Added least squares matrix solve op.
  -- Added triangular matrix solve op.
  -- Added Cholesky and GradCholesky op.
  -- Added SelfAdjointEig (v2), QR, SVD ops.
@eaplatanios
Copy link
Owner

@mandar2812 Thanks for this PR and sorry for the slow review but I have been very busy with multiple projects lately. I'll go through it in detail but quick question: why did you have to implement these ops in C++ yourself? I believe that there exist TF ops for multiple of these operations with already implemented kernels for both CPUs and GPUs.

@mandar2812
Copy link
Author

mandar2812 commented Oct 21, 2019

@eaplatanios I didn't implement them in C++, only added them in the Scala API using the the following procedure you (probably) described in another issue ( #32 ).

  1. Adding an entry in api/ops/Linalg.scala for lazy Output
  2. Adding an entry in api/tensors/ops/Linalg.scala for eager Tensor
  3. Creating JNI export in build.sbt
    "Linalg" -> Seq(
        "Cholesky", "CholeskyGrad", "LogMatrixDeterminant", "MatrixDeterminant", 
        "MatrixInverse", "MatrixSolve", "MatrixSolveLs", 
        /* "MatrixSquareRoot", */ "MatrixTriangularSolve", "Qr", 
        "SelfAdjointEigV2", "Svd")

After compiling and building I saw that the jni export task defined in the sbt build generated/updated some C++ headers which are used by JNI to interface between the scala and C++ api (If I have understood this Scala-JNI-Native stack correctly :D ).

@mandar2812
Copy link
Author

@eaplatanios Lets revisit this PR sometime!

@eaplatanios
Copy link
Owner

@mandar2812 I'm happy to take a look at this now. Sorry for the super massive delay. I just merged changes that make TF Scala work with TF 2.x and also add support for Scala 2.13 (I had to drop support for Scala 2.11 as some of the dependencies have also dropped support -- e.g., circe). Would you like to update this PR so it works with the current master and then I can take a look?

@mandar2812
Copy link
Author

@eaplatanios yes I will update this with the latest changes in master. A quick question, which version of tensorflow 2.x did you use to build master? Specific tag or commit hash would be helpful.

@eaplatanios
Copy link
Owner

eaplatanios commented May 24, 2020 via email

@mandar2812
Copy link
Author

mandar2812 commented Jul 5, 2020

@eaplatanios So I pulled your changes and I can merge the source tree just fine :)

Only issue is not in building ... I see a bunch of native compilation errors around Eigen. I have TF 2.2.0 installed in my local python environment.

[info] [  4%] Building CXX object CMakeFiles/tensorflow_ops.dir/ops/beam_search_ops.cc.o
[info] /usr/bin/c++  -Dtensorflow_ops_EXPORTS -I/home/mandar/Development/tensorflow_scala/modules/jni/src/main/native/. -I/home/mandar/Development/tensorflow_scala/modules/jni/src/main/native/./generated -I/home/mandar/Development/tensorflow_scala/modules/jni/src/main/native/./include -I/home/mandar/Development/tensorflow_scala/modules/jni/src/main/native/./include/third_party -I/home/mandar/Development/tensorflow_scala/modules/jni/src/main/native/./ops -I/usr/lib/jvm/java-8-openjdk-amd64/include -I/usr/lib/jvm/java-8-openjdk-amd64/include/linux  -D_GLIBCXX_USE_CXX11_ABI=0 -I/home/mandar/.local/lib/python3.7/site-packages/tensorflow/include -D_GLIBCXX_USE_CXX11_ABI=0 -L/home/mandar/.local/lib/python3.7/site-packages/tensorflow -l:libtensorflow_framework.so.2 -O3 -DNDEBUG -fPIC   -std=gnu++14 -o CMakeFiles/tensorflow_ops.dir/ops/beam_search_ops.cc.o -c /home/mandar/Development/tensorflow_scala/modules/jni/src/main/native/ops/beam_search_ops.cc
In file included from /home/mandar/.local/lib/python3.7/site-packages/tensorflow/include/unsupported/Eigen/CXX11/../../../Eigen/Core:156:0,
                 from /home/mandar/.local/lib/python3.7/site-packages/tensorflow/include/unsupported/Eigen/CXX11/Tensor:14,
                 from /home/mandar/.local/lib/python3.7/site-packages/tensorflow/include/third_party/eigen3/unsupported/Eigen/CXX11/Tensor:1,
                 from /home/mandar/.local/lib/python3.7/site-packages/tensorflow/include/tensorflow/core/framework/tensor_types.h:19,
                 from /home/mandar/Development/tensorflow_scala/modules/jni/src/main/native/ops/beam_search_ops.h:19,
                 from /home/mandar/Development/tensorflow_scala/modules/jni/src/main/native/ops/beam_search_ops.cc:22:
/home/mandar/.local/lib/python3.7/site-packages/tensorflow/include/unsupported/Eigen/CXX11/../../../Eigen/src/Core/util/IntegralConstant.h:189:36: error: template declaration of ‘const Eigen::internal::FixedInt<N> Eigen::fix’
 static const internal::FixedInt<N> fix{};

Could these errors be due to some C++ flags or GCC++ version issues? Can you tell me what environment setup I would need to get the repo to compile? I can try to replicate your setup.

EDIT: I see these lines in modules/jni/source/main/native/CMakeLists.txt

set(CMAKE_CXX_STANDARD 14)
set(CMAKE_CXX_STANDARD_REQUIRED on)

Is this causing the errors above? The Eigen source code which produces all of them seems to be in the directory unsupported/Eigen/CXX11/

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