Taichi: [Lang] make Matrix.__init__ less response by introducing multiple initializers

Created on 1 Jun 2020  路  2Comments  路  Source: taichi-dev/taichi

Concisely describe the proposed feature
I strongly argue about the ultra-massive-centralized-multiplexer in ti.Matrix.__init__:

def __init__(self, n=1, m=1, dt=None, shape=None,
             empty=False, layout=None, needs_grad=False,
             keep_raw=False, rows=None, cols=None):
   ... # 100+ lines of if-else-if's, within just **one function**

Functions should be short and sweet, and do just one thing. They should fit on one or two screenfuls of text (the ISO/ANSI screen size is 80x24, as we all know), and do one thing and do that well.

... said the Linux Kernel Coding Style, and I believe the 80x24 rule also works in python too.

Another measure of the function is the number of local variables. They shouldn鈥檛 exceed 5-10, or you鈥檙e doing something wrong. Re-think the function, and split it into smaller pieces. A human brain can generally easily keep track of about 7 different things, anything more and it gets confused. You know you鈥檙e brilliant, but maybe you鈥檇 like to understand what you did 2 weeks from now.

Not to say lobal variables, just the arguments alone, there are 11.

Describe the solution you'd like (if any)

Also note that the API initializing a ti.Matrix is highly centralized, GAMES201 students can easily get confused by error messages when they're not using the correct argument specification:

ti.Matrix(n, m, dt, shape)  # matrix tensor
ti.Matrix(n, dt, shape)  # vector tensor
ti.Matrix([[a, b], [c, d]])  # taichi-scope matrix
ti.Matrix([a, b])  # taichi-scope vector
ti.Matrix(cols=[u, v])  # taichi-scope matrix, with col vectors
ti.Matrix(rows=[u, v])  # taichi-scope matrix, with row vectors
ti.Matrix(n, m, empty=True)  # empty matrix

Don't you think a better design could be:

ti.Matrix.var(n, m, dt, shape)  # matrix tensor
ti.Vector.var(n, dt, shape)  # vector tensor
ti.Matrix([[a, b], [c, d]])  # taichi-scope matrix
ti.Vector([a, b])  # taichi-scope vector
ti.Matrix.cols([u, v])  # taichi-scope matrix, with col vectors
ti.Matrix.rows([u, v])  # taichi-scope matrix, with row vectors
ti.Matrix.empty(n, m)  # empty matrix

to give ti.Matrix.__init__ less duty just like currently ti.Matrix.ones and ti.Matrix.zeros does?

Or at least we can make ti.Matrix.__init__ serve as a router just redirecting to the actual functions like ti.Matrix.init_as_local and ti.Matrix.init_as_var, instead of putting those implementations inside of ti.Matrix.__init__ if-else-if's?

Additional comments

As I mentioned in https://github.com/taichi-dev/taichi/pull/1046#issue-422369389.
Note that we should always try our best to make a well-polished matrix/vector API before Taichi v1.0.0 comes.

GAMES201 discussion feature request good first issue python

Most helpful comment

Yes, I think so, either done this refactor before 6.13 or after GAMES201 ends, or students will be confused with deprecated APIs...

Maybe we want to do ti.Matrix.cols/rows/.. refactor first, and do Matrix.var after GAMES201 in v0.7.0?

All 2 comments

I presume that this needs to be done before June 13th for GAMES201 ?

Yes, I think so, either done this refactor before 6.13 or after GAMES201 ends, or students will be confused with deprecated APIs...

Maybe we want to do ti.Matrix.cols/rows/.. refactor first, and do Matrix.var after GAMES201 in v0.7.0?

Was this page helpful?
0 / 5 - 0 ratings

Related issues

Xayahp picture Xayahp  路  3Comments

g1n0st picture g1n0st  路  3Comments

xumingkuan picture xumingkuan  路  3Comments

kazimuth picture kazimuth  路  4Comments

GeoffreyPlitt picture GeoffreyPlitt  路  4Comments