[Lang] ti.cross now supports 2D vectors #1068

Eydcao · 2020-05-28T06:53:59Z

Related issue = 1 or 2 of those in #1016

This PR aimed to make ''ti.matrix.cross'' function work for 2D vectors. The result should be a 1*1 matrix(equivalent to scalar. If I were wrong, please feel free to correct). While reading the document for scalar tensor, I also noticed some issues in the rst, so fixed it BTW.

There are two more items todo:

test if the function is working well
improve the test for linear algebra
mention cross 2d in vector doc

[Click here for the format server]

archibate

Thanks for the implementation! LGTM in general except for a few nits.

python/taichi/lang/matrix.py

archibate · 2020-05-28T07:08:22Z

python/taichi/lang/matrix.py

+
+        else:
+            raise Exception(
+                "CrossProduct only supports 3D vector and 2D vector")


Suggested change

"CrossProduct only supports 3D vector and 2D vector")

"cross product is only supported between 3D or 2D vectors")

Hi Yubing,
Thanks for the quick suggestion. But here I assume "cross product is only supported between pairs of 3D vectors or those of 2D vectors" is more accurate. Just in case people are confused about if they ever tried vec2D.cross(vec3D).
What do you think~

Thank for clarify, that sounds like a better expression.

docs/scalar_tensor.rst

return true scalar instead of a 1x1 mat Co-authored-by: 彭于斌 <[email protected]>

archibate · 2020-05-28T08:26:32Z

Btw, would you like to also update this usage in docs/vector.rst?

Eydcao · 2020-05-28T08:49:37Z

Btw, would you like to also update this usage in docs/vector.rst?

No problem I will add it to the todo list here

tests/python/test_linalg.py

update to underline code style Co-authored-by: 彭于斌 <[email protected]>

docs/vector.rst

include var name Co-authored-by: 彭于斌 <[email protected]>

avoid making people confused about return type of cross(vec2d,vec2d) Co-authored-by: 彭于斌 <[email protected]>

yuanming-hu

Awesome! Thank you so much for implementing 2D cross-product and all the tests!

Just a few writing nits. I think this is ready to merge once they are applied.

docs/vector.rst

python/taichi/lang/matrix.py

writing nits Co-authored-by: Yuanming Hu <[email protected]>

another writing nits Co-authored-by: Yuanming Hu <[email protected]>

last writing nit! Co-authored-by: Yuanming Hu <[email protected]>

Last writing nits in vector.rst Co-authored-by: Yuanming Hu <[email protected]>

Capital letter at the beginning of a sentence. Co-authored-by: Yuanming Hu <[email protected]>

yuanming-hu

LGTM now. Thank you! Btw, "Add suggestion to batch" looks like a nice feature for you: https://help.github.com/en/github/collaborating-with-issues-and-pull-requests/incorporating-feedback-in-your-pull-request

Eydcao · 2020-06-10T02:44:26Z

LGTM now. Thank you! Btw, "Add suggestion to batch" looks like a nice feature for you: https://help.github.com/en/github/collaborating-with-issues-and-pull-requests/incorporating-feedback-in-your-pull-request

Thanks, I never notice this reply until today when I am about to work on a new PR. This is very helpful!

Eydcao added 2 commits May 27, 2020 23:16

[skip ci][doc] improve a bit in scalar_tensor.rst

69b29e6

[lang] make cross function work for vec2d

3ad3ddf

archibate changed the title ~~[Lang][Doc] make cross product work for 2D vectors~~ [Lang] make cross product work for 2D vectors May 28, 2020

archibate approved these changes May 28, 2020

View reviewed changes

Update python/taichi/lang/matrix.py

f8968ac

return true scalar instead of a 1x1 mat Co-authored-by: 彭于斌 <[email protected]>

archibate requested a review from taichi-gardener May 28, 2020 07:44

[skip ci] enforce code format

c6772cb

yuanming-hu mentioned this pull request May 28, 2020

GAMES 201 TODO items #1016

Closed

18 tasks

archibate added the GAMES201 GAMES 201 students' wishlist label May 29, 2020

Eydcao and others added 4 commits May 28, 2020 19:24

[skip ci] clearer description of cross's exception

a198ab0

[Test] improve basic ults test of matrix/linalg

3ee305b

resolve merge conflict

fd164a5

[skip ci] enforce code format

1393b99

archibate reviewed May 29, 2020

View reviewed changes

tests/python/test_linalg.py Outdated Show resolved Hide resolved

[skip ci] mention cross2D in vector.rst

ec86c16

Eydcao marked this pull request as ready for review May 29, 2020 03:38

Update tests/python/test_linalg.py

9964be7

update to underline code style Co-authored-by: 彭于斌 <[email protected]>

archibate reviewed May 29, 2020

View reviewed changes

docs/vector.rst Outdated Show resolved Hide resolved

docs/vector.rst Outdated Show resolved Hide resolved

Eydcao and others added 2 commits May 29, 2020 12:05

Update docs/vector.rst

6d828dd

include var name Co-authored-by: 彭于斌 <[email protected]>

Update docs/vector.rst

4d73612

avoid making people confused about return type of cross(vec2d,vec2d) Co-authored-by: 彭于斌 <[email protected]>

archibate requested review from yuanming-hu and k-ye May 29, 2020 05:24

yuanming-hu reviewed May 29, 2020

View reviewed changes

docs/vector.rst Outdated Show resolved Hide resolved

docs/vector.rst Outdated Show resolved Hide resolved

docs/vector.rst Outdated Show resolved Hide resolved

docs/vector.rst Outdated Show resolved Hide resolved

python/taichi/lang/matrix.py Outdated Show resolved Hide resolved

Eydcao and others added 5 commits May 29, 2020 22:16

Update docs/vector.rst

487fc9a

writing nits Co-authored-by: Yuanming Hu <[email protected]>

Update docs/vector.rst

44617eb

another writing nits Co-authored-by: Yuanming Hu <[email protected]>

Update docs/vector.rst

70587f1

last writing nit! Co-authored-by: Yuanming Hu <[email protected]>

Update docs/vector.rst

b54dfcc

Last writing nits in vector.rst Co-authored-by: Yuanming Hu <[email protected]>

[skip ci] enforce code format

c2aea7b

Update python/taichi/lang/matrix.py

99cbb45

Capital letter at the beginning of a sentence. Co-authored-by: Yuanming Hu <[email protected]>

yuanming-hu changed the title ~~[Lang] make cross product work for 2D vectors~~ [Lang] Make cross product work for 2D vectors May 29, 2020

archibate added the skip ci label May 29, 2020

yuanming-hu changed the title ~~[Lang] Make cross product work for 2D vectors~~ [Lang] ti.cross now supports 2D vectors May 29, 2020

yuanming-hu approved these changes May 29, 2020

View reviewed changes

yuanming-hu merged commit 2d4ade4 into taichi-dev:master May 29, 2020

yuanming-hu mentioned this pull request May 31, 2020

[release] v0.6.7 #1094

Merged

Eydcao deleted the cross2D branch June 5, 2020 05:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Lang] ti.cross now supports 2D vectors #1068

[Lang] ti.cross now supports 2D vectors #1068

Eydcao commented May 28, 2020 •

edited

Loading

archibate left a comment

archibate May 28, 2020 •

edited

Loading

Eydcao May 28, 2020

archibate May 28, 2020

archibate commented May 28, 2020

Eydcao commented May 28, 2020

yuanming-hu left a comment

yuanming-hu left a comment

Eydcao commented Jun 10, 2020

	"CrossProduct only supports 3D vector and 2D vector")
	"cross product is only supported between 3D or 2D vectors")

[Lang] ti.cross now supports 2D vectors #1068

[Lang] ti.cross now supports 2D vectors #1068

Conversation

Eydcao commented May 28, 2020 • edited Loading

archibate left a comment

Choose a reason for hiding this comment

archibate May 28, 2020 • edited Loading

Choose a reason for hiding this comment

Eydcao May 28, 2020

Choose a reason for hiding this comment

archibate May 28, 2020

Choose a reason for hiding this comment

archibate commented May 28, 2020

Eydcao commented May 28, 2020

yuanming-hu left a comment

Choose a reason for hiding this comment

yuanming-hu left a comment

Choose a reason for hiding this comment

Eydcao commented Jun 10, 2020

Eydcao commented May 28, 2020 •

edited

Loading

archibate May 28, 2020 •

edited

Loading