Matrix CWrappers #992

Merged
merged 33 commits into from Jul 7, 2016

Projects

None yet

4 participants

@rajithv
Contributor
rajithv commented Jun 20, 2016

No description provided.

rajithv added some commits Jun 20, 2016
@rajithv rajithv Init DenseMatrix 86f7150
@rajithv rajithv Removing RCP for matrices
eed02c2
@isuruf isuruf commented on an outdated diff Jun 21, 2016
symengine/cwrapper.h
@@ -5,6 +5,8 @@
#include <stdlib.h>
#include <gmp.h>
+#include "symengine/matrix.h"
@isuruf
isuruf Jun 21, 2016 Member

Don't include C++ headers in this C header. Take a look at CVecBasic to see how vec_basic is wrapped.

rajithv added some commits Jun 21, 2016
@rajithv rajithv Adapted the CVecBasic-like method for DenseMatrix
aca561d
@rajithv rajithv Used MatrixBase instead of DenseMatrix 2a08e8e
@rajithv rajithv Reverting to DenseMatrix from MatrixBase
f0b6b8f
@rajithv rajithv All constructor wrappers for DenseMatrix
ff97883
@rajithv rajithv Fixing errors with code order
3185fc9
@rajithv rajithv Wrapper for the __str__() function of MatrixBase
b6ae493
@abinashmeher999 abinashmeher999 and 2 others commented on an outdated diff Jun 23, 2016
symengine/cwrapper.cpp
+
+struct CDenseMatrix {
+ SymEngine::DenseMatrix m;
+};
+
+CDenseMatrix *dense_matrix_new()
+{
+ return new CDenseMatrix;
+}
+
+void dense_matrix_free(CDenseMatrix *self)
+{
+ delete self;
+}
+
+void dense_matrix(CDenseMatrix *s)
@abinashmeher999
abinashmeher999 Jun 23, 2016 Contributor

This should be named something like dense_matrix_init.

@rajithv
rajithv Jun 23, 2016 Contributor

noted

@isuruf
isuruf Jun 23, 2016 Member

@rajithv, you don't need this function right? You can use dense_matrix_new(), right?

@abinashmeher999 abinashmeher999 and 1 other commented on an outdated diff Jun 23, 2016
symengine/cwrapper.cpp
@@ -734,6 +780,12 @@ void ntheory_binomial(basic s, const basic a, unsigned long b)
s->m = SymEngine::binomial(static_cast<const Integer &>(*(a->m)), b);
}
+// Matrix Wrappers
+SymEngine::DenseMatrix* matrix_dense_matrix()
@abinashmeher999
abinashmeher999 Jun 23, 2016 Contributor

Do you plan to use this somewhere?

@rajithv
rajithv Jun 23, 2016 Contributor

That was actually something I wrote earlier, and had forgotten to remove. Now removed it.

@certik
Contributor
certik commented Jun 24, 2016

@rajithv please ping me once you fix the conflicts and want to review.

rajithv added some commits Jun 25, 2016
@rajithv rajithv Merge branch 'master' of https://github.com/symengine/symengine into …
…matrix
ba87f11
@rajithv rajithv Fixed issue with Inversion, Transpose and Submatrix
e5eebdc
@rajithv rajithv wrapped nrows and ncols f4384b2
@rajithv rajithv CWrappers for add_matrix, add_scalar, mul_matrix and mul_scalar
88125d4
@rajithv rajithv cwrappers for LU, LDL, LU_solve, FFLU, FFLDU and making arguments const
ab66e29
@rajithv rajithv Fixed issue with transpose
7e3d6c6
@rajithv rajithv Fixing LU_solve
acfd301
@certik
Contributor
certik commented Jun 27, 2016

(also remove the [WIP] once you want to review.)

@rajithv
Contributor
rajithv commented Jun 28, 2016

@certik will surely do. I've got to wrap the numpy-like functions and write the tests. Will ping you after completing those tasks.

@isuruf isuruf and 1 other commented on an outdated diff Jun 28, 2016
symengine/cwrapper.cpp
+CSparseMatrix *sparse_matrix_new()
+{
+ return new CSparseMatrix;
+}
+
+void dense_matrix_free(CDenseMatrix *self)
+{
+ delete self;
+}
+
+void sparse_matrix_free(CSparseMatrix *self)
+{
+ delete self;
+}
+
+void dense_matrix_init(CDenseMatrix *s)
@isuruf
isuruf Jun 28, 2016 Member

What's the use of this function?

@rajithv
rajithv Jun 28, 2016 Contributor

wrapping DenseMatrix::DenseMatrix(const DenseMatrix &x) this constructor.
using it for: SymEngine::DenseMatrix(SymEngine::DenseMatrix) in the ruby wrapper

@isuruf
isuruf Jun 28, 2016 Member

DenseMatrix::DenseMatrix(const DenseMatrix &x) is used for copying. What's the difference with dense_matrix_set?

@rajithv
rajithv Jun 28, 2016 Contributor

Yes, I see it now. I will fix it. Thanks for pointing it out :)

rajithv added some commits Jun 28, 2016
@rajithv rajithv cwrappers for numpy-like functions - DenseMatrix
83ac221
@rajithv rajithv SparseMatrix get and set
f508543
@rajithv rajithv Formatting and setting up testing for matrices in tests_cwrapper f442391
@rajithv rajithv Merge branch 'master' of https://github.com/symengine/symengine into …
…matrix
20e712b
@isuruf isuruf commented on an outdated diff Jul 4, 2016
symengine/cwrapper.cpp
+}
+
+void dense_matrix_init(CDenseMatrix *s)
+{
+ s->m = SymEngine::DenseMatrix();
+}
+
+void sparse_matrix_init(CSparseMatrix *s)
+{
+ s->m = SymEngine::CSRMatrix();
+}
+
+void dense_matrix_rows_cols(CDenseMatrix *s, unsigned long int rows,
+ unsigned long int cols)
+{
+ s->m = SymEngine::DenseMatrix(rows, cols);
@isuruf
isuruf Jul 4, 2016 edited Member

This shouldn't create a new Matrix and assign, but rather change s->m

@isuruf isuruf commented on an outdated diff Jul 4, 2016
symengine/cwrapper.cpp
+
+void dense_matrix_rows_cols(CDenseMatrix *s, unsigned long int rows,
+ unsigned long int cols)
+{
+ s->m = SymEngine::DenseMatrix(rows, cols);
+}
+
+void sparse_matrix_rows_cols(CSparseMatrix *s, unsigned long int rows,
+ unsigned long int cols)
+{
+ s->m = SymEngine::CSRMatrix(rows, cols);
+}
+
+void dense_matrix_set(CDenseMatrix *s, const CDenseMatrix *d)
+{
+ s->m = SymEngine::DenseMatrix(d->m);
@isuruf
isuruf Jul 4, 2016 Member

s->m = d->m is enough here

@isuruf isuruf commented on an outdated diff Jul 4, 2016
symengine/cwrapper.cpp
+ s->m = mat->m.get(r, c);
+}
+
+void sparse_matrix_set_basic(CSparseMatrix *mat, unsigned long int r,
+ unsigned long int c, basic s)
+{
+ mat->m.set(r, c, s->m);
+}
+
+void dense_matrix_det(basic s, const CDenseMatrix *mat)
+{
+ s->m = mat->m.det();
+}
+void dense_matrix_inv(CDenseMatrix *s, const CDenseMatrix *mat)
+{
+ s->m = SymEngine::DenseMatrix();
@isuruf
isuruf Jul 4, 2016 Member

This line is unnecessary

@isuruf isuruf commented on an outdated diff Jul 4, 2016
symengine/cwrapper.cpp
+ mat->m.set(r, c, s->m);
+}
+
+void dense_matrix_det(basic s, const CDenseMatrix *mat)
+{
+ s->m = mat->m.det();
+}
+void dense_matrix_inv(CDenseMatrix *s, const CDenseMatrix *mat)
+{
+ s->m = SymEngine::DenseMatrix();
+ dense_matrix_rows_cols(s, mat->m.nrows(), mat->m.ncols());
+ mat->m.inv(s->m);
+}
+void dense_matrix_transpose(CDenseMatrix *s, const CDenseMatrix *mat)
+{
+ s->m = SymEngine::DenseMatrix();
@isuruf
isuruf Jul 4, 2016 Member

Same here

@isuruf
Member
isuruf commented Jul 4, 2016

s->m = SymEngine::DenseMatrix(); at the beginning of most functions is unnecessary

rajithv added some commits Jul 4, 2016
@rajithv rajithv Changed eye, ones and zeros to reflect changes in C++ code; wrapped =…
…= and != operators
3c2bdbf
@rajithv rajithv Adding cwrapper tests 115a3ff
@rajithv rajithv Fix in the Cwrapper tests
36e5183
@isuruf isuruf and 1 other commented on an outdated diff Jul 5, 2016
symengine/tests/cwrapper/test_cwrapper.c
@@ -1013,7 +1118,8 @@ int main(int argc, char *argv[])
#endif // HAVE_SYMENGINE_MPFR
#ifdef HAVE_SYMENGINE_MPC
test_complex_mpc();
-#endif // HAVE_SYMENGINE_MPC
-
+#endif // HAVE_SYMENGINE_MPC*/
+ symengine_print_stack_on_segfault();
+ test_matrix();
@isuruf
isuruf Jul 5, 2016 Member

You can move matrix tests to a new test file if you want.

@rajithv
rajithv Jul 5, 2016 Contributor

I'll uncomment the lines, once all the tests are done.
Commented them out, so running the tests would be easier locally.

rajithv added some commits Jul 5, 2016
@rajithv rajithv CWrapper Tests done
cb59d41
@rajithv rajithv resize method added for DenseMatrix
9c370c8
@isuruf isuruf commented on an outdated diff Jul 6, 2016
symengine/matrix.h
@@ -87,6 +87,9 @@ class DenseMatrix : public MatrixBase
// Should implement all the virtual methods from MatrixBase
// and throw an exception if a method is not applicable.
+ // Resize
+ virtual void resize(unsigned i, unsigned j);
+
@isuruf
isuruf Jul 6, 2016 Member

Can you remove virtual from this and move it above Line 87?

@rajithv rajithv Tests for numpy-like functions
9813394
@rajithv rajithv changed the title from [WIP] Matrix CWrappers to Matrix CWrappers Jul 6, 2016
@rajithv
Contributor
rajithv commented Jul 6, 2016

@isuruf @abinashmeher999 @certik ready for review

@isuruf isuruf commented on the diff Jul 7, 2016
symengine/cwrapper.cpp
+}
+
+void sparse_matrix_init(CSparseMatrix *s)
+{
+ s->m = SymEngine::CSRMatrix();
+}
+
+void sparse_matrix_rows_cols(CSparseMatrix *s, unsigned long int rows,
+ unsigned long int cols)
+{
+ s->m = SymEngine::CSRMatrix(rows, cols);
+}
+
+void dense_matrix_set(CDenseMatrix *s, const CDenseMatrix *d)
+{
+ s->m = d->m;
@isuruf
isuruf Jul 7, 2016 Member

Can you add a test for this?

@isuruf isuruf commented on the diff Jul 7, 2016
symengine/tests/cwrapper/test_cwrapper.c
+ expected = "[0, 0]\n[0, 0]\n[0, 0]\n";
+ SYMENGINE_C_ASSERT(strcmp(result, expected) == 0);
+
+ // Diag
+ dense_matrix_diag(D, vec, 0);
+ result = dense_matrix_str(D);
+ expected = "[-2, 0, 0, 0]\n[0, 3, 0, 0]\n[0, 0, 3, 0]\n[0, 0, 0, -4]\n";
+ SYMENGINE_C_ASSERT(strcmp(result, expected) == 0);
+
+ //det
+ dense_matrix_det(i1, D);
+ SYMENGINE_C_ASSERT(integer_get_ui(i1) == 72);
+
+ // eye
+ dense_matrix_eye(D, 3, 4, 1);
+ result = dense_matrix_str(D);
@isuruf
isuruf Jul 7, 2016 Member

result should be deallocated every time you use dense_matrix_str

rajithv added some commits Jul 6, 2016
@rajithv rajithv Tests for numpy-like functions dd097a2
@rajithv rajithv freeing the strings in test_matrix() in test_cwrapper.c and adding te…
…sts for dense_matrix_set
ed714cc
@rajithv rajithv Merge branch 'matrix' of https://github.com/rajithv/symengine into ma…
…trix
1de461b
@isuruf isuruf commented on the diff Jul 7, 2016
symengine/tests/cwrapper/test_cwrapper.c
result = dense_matrix_str(D);
expected = "[4, 0]\n[0, -1/4]\n";
SYMENGINE_C_ASSERT(strcmp(result, expected) == 0);
+ basic_str_free(result);
+
+ // set matrix
+ dense_matrix_set(C, D);
+ SYMENGINE_C_ASSERT(dense_matrix_eq(D, C) == 1);
@isuruf
isuruf Jul 7, 2016 Member

Can you change D after this line and check that this doesn't affect C ?

@isuruf isuruf commented on an outdated diff Jul 7, 2016
symengine/tests/cwrapper/test_cwrapper.c
@@ -1174,6 +1174,11 @@ void test_matrix()
dense_matrix_set(C, D);
SYMENGINE_C_ASSERT(dense_matrix_eq(D, C) == 1);
+ dense_matrix_LDL(D, E, B);
+
+ // now E should be equal to C
+ SYMENGINE_C_ASSERT(dense_matrix_eq(C, E) == 1);
+
@isuruf
isuruf Jul 7, 2016 Member

Can you add a check that C != D?

@rajithv rajithv improvement in test_cwrapper's test_matrix()
14747c7
@isuruf
Member
isuruf commented Jul 7, 2016

+1 to merge when tests pass

@isuruf isuruf merged commit 8270d81 into symengine:master Jul 7, 2016

3 checks passed

codecov/project 69.31% (+0.35%) compared to 0340d47
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@certik
Contributor
certik commented Jul 7, 2016

Thanks @rajithv for wrapping these!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment