Improvements for ImmutableMatrix #148
Conversation
|
It's better to fix |
ShikharJ
force-pushed
the
_set
branch
2 times, most recently
from
2fef6d3 to
cbb74fc
Compare
ShikharJ
changed the title
symengine/lib/symengine_wrapper.pyx
Outdated
| self._set(i, j, f(self._get(i, j))) | ||
| e_ = _sympify(f(self._get(i, j))) | ||
| if e_ is not None: | ||
| deref(self.thisptr).set(i, j, e_.thisptr) |
There was a problem hiding this comment.
Above method would still allow an ImmutableMatrix to be mutated from python. You can copy the code from here and move it to applyfunc and move _applyfunc to MutableDenseMatrix.
symengine/lib/symengine_wrapper.pyx
Outdated
| raise ShapeError("`self` and `rhs` must have the same number of columns.") | ||
| cdef DenseMatrixBase result = zeros(self.rows + o.rows, self.cols) | ||
| cdef DenseMatrixBase result = self.__class__(self) | ||
| deref(symengine.static_cast_DenseMatrix(result.thisptr)).resize(self.rows + o.rows, self.cols) |
There was a problem hiding this comment.
Change the above 2 lines to cdef DenseMatrixBase result = self.__class__(self.rows, self.cols). This avoids the copy in the first line.
ShikharJ
force-pushed
the
_set
branch
3 times, most recently
from
246cfc1 to
2a56b82
Compare
|
Ping @isuruf. |
ShikharJ
changed the title
symengine/lib/symengine_wrapper.pyx
Outdated
| for k in range(0, self.cols): | ||
| self[i, k], self[j, k] = self[j, k], self[i, k] | ||
|
|
||
| def applyfunc(self, f): |
There was a problem hiding this comment.
What's the difference between this function and the one above?
We'd also need a _applyfunc in MutableDenseMatrix that modifies it in place.
There was a problem hiding this comment.
This one uses the python defined method _set. The implementation above directly uses C++ method.
There was a problem hiding this comment.
Okay, but what's the difference in the functionality?
There was a problem hiding this comment.
None other than that, I'll change this to an in-place _applyfunc.
symengine/lib/symengine_wrapper.pyx
Outdated
| for i in range(self.rows): | ||
| for j in range(self.cols): | ||
| result[i, j] = self[i, j] | ||
| e_ = _sympify(self._get(i, j)) |
|
Thanks for the PR |
2 participants
Relevant: #147