Open mandar2812 opened 5 years ago
Forgot to commit the modification to build.sbt
, adding it here.
@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.
@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 ).
api/ops/Linalg.scala
for lazy Output
api/tensors/ops/Linalg.scala
for eager Tensor
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 ).
@eaplatanios Lets revisit this PR sometime!
@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?
@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.
Great, thanks Mandar! The published images use 2.1.1, but I tested with all of the 2.x releases and all compiled and passed tests.
On Sun, May 24, 2020 at 2:11 PM Mandar Chandorkar notifications@github.com wrote:
@eaplatanios https://github.com/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.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/eaplatanios/tensorflow_scala/pull/173#issuecomment-633269813, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJ4EXGACC76QHNEX5ODU4TRTFPN7ANCNFSM4I4LC3BQ .
@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/
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
@eaplatanios @sbrunk @lucataglia @DirkToewe : What do you guys think?