[Lang] [refactor] Deprecate "as_vector=True" in Matrix.to_numpy/to_torch

[archibate]

Related issue = #940 #833 #1016

This PR also makes the stage 3 of #923 easier to implement.

[Click here for the format server]

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’t exceed 5-10, or you’re 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’re brilliant, but maybe you’d like to understand what you did 2 weeks from now.

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

---

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?

Note that we should always try our best to make a well-polished matrix/vector API before Taichi v1.0.0 comes.

---

I'm putting a lot of lines of text just to convey you trust my refactor decision, so please take a good look and tl;dr's are unwanted.

[archibate]

Hello? Please feel free to say No if you don't like this change.

[yuanming-hu]

I actually like your idea of having multiple initializers!

Whether we should separate vectors from matrices still needs discussions. I personally prefer not to do it:

• We want to hold 1x1 matrices with the class Matrix only, instead of either Matrix or Vector. The latter will lead to a lot of confusion.
• Using a single Matrix class can hold both row vectors and column vectors. If we create an extra Vector class, we have to further distinguish row vectors from column vectors
• If we separate Vector from Matrix, can Matrix hold nx1 or 1xn matrices (vectors)?
  • If it can, then users might misuse Matrix to hold vectors
  • If it cannot, then a lot of branching will be needed for functions returning nxm matrices where n or m might be 1

Generally speaking, for regularity, I prefer not to do the separation.

[archibate]

If it can, then users might misuse Matrix to hold vectors

They don't want to use Matrix as vector, since Matrix.dot: Name not defined.

My worry is, will Matrix.dot and Matrix.outer_product be confusing to people? They are actually only for Vectors.

Another issue is, as_vector=True sounds so strange, why I defined as ti.Vector while still needs to specify this? And what if we add more taichi_classes like 四元数.to_numpy()?

[yuanming-hu]

Good points.

They don't want to use Matrix as vector, since Matrix.dot: Name not defined.
My worry is, will Matrix.dot and Matrix.outer_product be confusing to people? They are actually only for Vectors.

Maybe we can solve this by moving dot and outer_product to ti?

Another issue is, as_vector=True sounds so strange, why I defined as ti.Vector while still needs to specify this? And what if we add more taichi_classes like 四元数.to_numpy()?

as_vector actually brings you consistency. We don't want dimensionality (shape) of the returned numpy array to depend on how many rows/columns you have in Matrix.

[archibate]

as_vector actually brings you consistency. We don't want dimensionality (shape) of the returned numpy array to depend on how many rows/columns you have in Matrix.

Not this point. My point is ti.Vector is a complete different class from ti.Matrix, just like ti.Quaternion.to_numpy doesn't have to be the same shape with ti.Matrix.to_numpy.

[archibate]

Also note that we will have to use [[1, 2]] or [[1], [2]] for printing row/col vectors in #923.
So we want:

print('u =', u, as_vector=True)

for that? See? as_vector is really a bad design.

[yuanming-hu]

Thanks for the inputs.

If you really hate as_vector, how about this: let's make to_numpy sets as_vector as True by default when the Matrix with only one column.

as_vector actually brings you consistency. We don't want dimensionality (shape) of the returned numpy array to depend on how many rows/columns you have in Matrix.

By changing the default as_vector we give up consistency here for convenience, which is fine. There's no perfect solution here.

Not this point. My point is ti.Vector is a complete different class from ti.Matrix, just like ti.Quaternion.to_numpy doesn't have to be the same shape with ti.Matrix.to_numpy.

I really like your idea of having more constructors, but I still don't think Vector is completely different from Matrix. Quaternion should clearly be a different class since it has different arithmetic rules from Vector and Matrix.

So, does removing as_vector but not separating Vector from Matrix sound reasonable to you? We can, of course, deprecate ti.Matrix.dot and use ti.dot instead.

[yuanming-hu]

@FantasyVR please feel free to share your inputs given you are interested in this topic :-)

[archibate]

@FantasyVR please feel free to share your inputs given you are interested in this topic :-)

Yes, we need more discussion on this, to determine this ultra-critical design by just you-and-me can be biased.

If you really hate as_vector, how about this: let's make to_numpy sets as_vector as True by default when the Matrix with only one column.

So we also print as [0, 1] in #923 if self.m == 1? What if self.n == self.m == 1? Is that printed as a scalar or 1D-col-vector or 1D-row-vector?

[archibate]

I just come up with an perfect idea to keep both consistency and simplicity:

def Vector(n):
  return Matrix(n, 1, is_vector=True)

...
def dot(u, v):
  assert u.is_vector

[yuanming-hu]

Thank you so much! Most changes look great. Just two minor places to fix.

[yuanming-hu]

Let's skip self.m as well if it value is 1. len(dim_ext)=

• 0 for 1x1 matrices
• 1 for nx1 or 1xn matrices (n != 1)
• 2 for nxm matrices (n, m != 1)

[archibate]

Then I should change matrix_to_ext_arr to treat the useless 1x1 matrices?

[yuanming-hu]

Yes, it seems that we need a keep_dims parameter in matrix_to_ext_arr as well.

Thanks!

[archibate]

Special treating 1x1 mats sounds unreasonable to me..

[yuanming-hu]

Few people would use 1x1 matrices anyway. Let's just make the behavior consistent: dimensionality of size 1 should be skipped.

[archibate]

Wdym? Do we consider 1x1 mat as 1D vector?

[yuanming-hu]

I mean, if the matrix is 1x1, then the numpy array, when keep_dims=False, should have the same shape as the matrix shape, instead of the matrix shape extended by (1, 1).

[yuanming-hu]

Thanks! Let's make keep_dims behave as what most people would expect.

[yuanming-hu]

Suggested change

	new_v = ti.Matrix.zero(ti.f32, 2)
	new_v = ti.Vector.zero(ti.f32, 2)

[archibate]

>           new_v = ti.Vector.zero(ti.f32, 2)
E           AttributeError: 'function' object has no attribute 'zero'

Thanks to the fact that Vector is not a subclass of Matrix, but a initializer wrapper function.

[archibate]

Now, I use Vector.zero = Matrix.zero solved the problem for now before we finally come to ti.linalg.zero.

[yuanming-hu]

Sounds good. We should not use the subclass approach. Let's use add different initializers later.

[archibate]

Hello? We should mt asap, 2pvcw/ #1051, & mk GAMES201 happy.

[yuanming-hu]

I think we can merge this for now, given the changeset is getting large, but we should fully implement keep_dims later. Thank you!

[xumingkuan]

taichi/python/taichi/misc/image.py

Line 7 in 6751bd5

img = img.to_numpy(as_vector=True)

@archibate This line raises an error now...