[Lang] MatrixNdarray refactor part2: Remove redundant members in python-scope AnyArray #5885
Conversation
…uction in Ndarray and refactored the use of element_shape
…t to use TensorType
…to matrix_ndarray_pr2
✅ Deploy Preview for docsite-preview ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
…to matrix_ndarray_pr2
…on-scope AnyArray
…to matrix_ndarray_pr3
jim19930609
force-pushed
the
matrix_ndarray_pr3
branch
from
68b0d60 to
010dc7e
Compare
…to matrix_ndarray_pr3
…to matrix_ndarray_pr3
jim19930609
force-pushed
the
matrix_ndarray_pr3
branch
from
1bbe024 to
87f51da
Compare
jim19930609
requested review from
ailzhang and
strongoier
and removed request for
ailzhang
jim19930609
requested review from
ailzhang,
strongoier,
k-ye and
AD1024
and removed request for
strongoier and
ailzhang
| @@ -14,17 +14,26 @@ class AnyArray: | |||
| element_shape (Tuple[Int]): () if scalar elements (default), (n) if vector elements, and (n, m) if matrix elements. | |||
| layout (Layout): Memory layout. | |||
| """ | |||
| def __init__(self, ptr, element_shape, layout): | |||
| def __init__(self, ptr): | |||
| assert ptr.is_external_var() | |||
| self.ptr = ptr | |||
There was a problem hiding this comment.
I wonder if it's easier to set self.element_shape & self.layout directly here. I can understand that we want to keep element_shape/layout in sync with the ptr but querying into C++ from python is also kinda expensive. So maybe we can add an assumption here (not using facing IIUC) and save some perf in small kernel cases?
There was a problem hiding this comment.
I think that sounds more like a caching mechanism, where we cache a bunch of C++ values at Python scope to minimize pybind performance loss. If we want to minimize the logics in Frontend language, then we'll need this cache sooner or later.
IMO, the self.element_shape and self.layout are essentially ill-implemented caches which easily causes mismatch between Python and C++ objects. Moreover, this cache also got no centralized management, and can spread all over the code base.
Maybe we can implement a better caching mechanism together with "pybind - ctypes" refactor?
Related issue = #5873, #5819
This PR belongs to "Part ②" in #5873.
After this PR,
[Python]AnyArrayonly keeps aptrto an underlying C++ objectExternalTensorExpression. AnyArray's attributes such asshapeandlayoutshould be directly fetched fromself.ptr, so that they stay in sync withExternalTensorExpressionall the time.