New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Express in vector module #13191

Open
wants to merge 21 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@szymag
Contributor

szymag commented Aug 24, 2017

This PR adapt express for transformation equations.

@szymag

This comment has been minimized.

Show comment
Hide comment
@szymag

szymag Aug 24, 2017

Contributor

ping @Upabjojr

Contributor

szymag commented Aug 24, 2017

ping @Upabjojr

Show outdated Hide outdated sympy/vector/functions.py Outdated
Show outdated Hide outdated sympy/vector/functions.py Outdated
@Upabjojr

This comment has been minimized.

Show comment
Hide comment
@Upabjojr

Upabjojr Aug 25, 2017

Contributor

We need tests of express with the new coordinate transformations.

Contributor

Upabjojr commented Aug 25, 2017

We need tests of express with the new coordinate transformations.

Show outdated Hide outdated sympy/vector/coordsysrect.py Outdated
Show outdated Hide outdated sympy/vector/coordsysrect.py Outdated
Show outdated Hide outdated sympy/vector/coordsysrect.py Outdated
@Upabjojr

This comment has been minimized.

Show comment
Hide comment
@Upabjojr

Upabjojr Aug 28, 2017

Contributor

At the first glance, it looks like you have inverted the transformation functions twice, so that the effect has cancelled.

Contributor

Upabjojr commented Aug 28, 2017

At the first glance, it looks like you have inverted the transformation functions twice, so that the effect has cancelled.

@szymag

This comment has been minimized.

Show comment
Hide comment
@szymag

szymag Aug 28, 2017

Contributor

At the first glance, it looks like you have inverted the transformation functions twice, so that the effect has cancelled.

Can you show me the place which suggest that?

Contributor

szymag commented Aug 28, 2017

At the first glance, it looks like you have inverted the transformation functions twice, so that the effect has cancelled.

Can you show me the place which suggest that?

@Upabjojr

This comment has been minimized.

Show comment
Hide comment
@Upabjojr

Upabjojr Aug 28, 2017

Contributor

Can you undo the edits to the transformation functions?

Contributor

Upabjojr commented Aug 28, 2017

Can you undo the edits to the transformation functions?

@szymag

This comment has been minimized.

Show comment
Hide comment
@szymag

szymag Aug 28, 2017

Contributor

Yes

Contributor

szymag commented Aug 28, 2017

Yes

@Upabjojr

This comment has been minimized.

Show comment
Hide comment
@Upabjojr

Upabjojr Aug 28, 2017

Contributor

I think it's simple, we are using the transformation input as transformation from the parent, while the code of vector used to assume that it is the opposite.

@moorepants hasn't yet answered to my email.

Contributor

Upabjojr commented Aug 28, 2017

I think it's simple, we are using the transformation input as transformation from the parent, while the code of vector used to assume that it is the opposite.

@moorepants hasn't yet answered to my email.

@Upabjojr

This comment has been minimized.

Show comment
Hide comment
@Upabjojr

Upabjojr Aug 28, 2017

Contributor

One possible interpretation is that the vector module is tought to manage transformations as active transformations: instead of rotating the coordinate system, you are rotating all objects in the coordinate system. Therefore the result is like having the coordinate system rotated by the opposite transformations.

Contributor

Upabjojr commented Aug 28, 2017

One possible interpretation is that the vector module is tought to manage transformations as active transformations: instead of rotating the coordinate system, you are rotating all objects in the coordinate system. Therefore the result is like having the coordinate system rotated by the opposite transformations.

@szymag

This comment has been minimized.

Show comment
Hide comment
@szymag

szymag Aug 28, 2017

Contributor

http://docs.sympy.org/latest/modules/vector/coordsys.html#orienting-new-systems
There is a picture which suggest that we have here passive rotation. Also for active we have separate method orient_new_body

Contributor

szymag commented Aug 28, 2017

http://docs.sympy.org/latest/modules/vector/coordsys.html#orienting-new-systems
There is a picture which suggest that we have here passive rotation. Also for active we have separate method orient_new_body

@szymag szymag changed the title from [WIP] Express in vector module to Express in vector module Aug 29, 2017

Show outdated Hide outdated sympy/vector/coordsysrect.py Outdated
r * sin(theta) * cos(phi),
r * sin(theta) * sin(phi),
r * cos(theta)))
# assert raises(ValueError, lambda: G.transformation_from_parent())

This comment has been minimized.

@szymag

szymag Aug 30, 2017

Contributor

This test fails on my PC. I don't know why because ValueError is raised

@szymag

szymag Aug 30, 2017

Contributor

This test fails on my PC. I don't know why because ValueError is raised

This comment has been minimized.

@Upabjojr

Upabjojr Aug 30, 2017

Contributor

Do not comment if it's the expected value. So we can see Travis.

@Upabjojr

Upabjojr Aug 30, 2017

Contributor

Do not comment if it's the expected value. So we can see Travis.

This comment has been minimized.

@Upabjojr

Upabjojr Aug 30, 2017

Contributor

You don't need assert if you use raises

@Upabjojr

Upabjojr Aug 30, 2017

Contributor

You don't need assert if you use raises

def test_transformation_functions():
C = CoordSys3D("C")
D = C.locate_new("D", 3 * C.i)
assert D.transformation_from_parent() == (C.x + 3, C.y, C.z)

This comment has been minimized.

@Upabjojr

Upabjojr Aug 30, 2017

Contributor

didn't we conclude that D.x == C.x - 3? Express replaces C.x with D.x + 3 in line 492...

@Upabjojr

Upabjojr Aug 30, 2017

Contributor

didn't we conclude that D.x == C.x - 3? Express replaces C.x with D.x + 3 in line 492...

This comment has been minimized.

@szymag

szymag Aug 30, 2017

Contributor

If we have passive transformation, the parent coordinate system is shifted. Here is shifted in left direction, I mean -3.
So if we have from_parent we're gong to the right direction, +3.
It's my explanation. I might be wrong

@szymag

szymag Aug 30, 2017

Contributor

If we have passive transformation, the parent coordinate system is shifted. Here is shifted in left direction, I mean -3.
So if we have from_parent we're gong to the right direction, +3.
It's my explanation. I might be wrong

This comment has been minimized.

@Upabjojr

Upabjojr Aug 30, 2017

Contributor

Well, my purpose is to make transformation_from_parent return the combination of the parent's base scalars such that will be equivalent to the child's base scalars. Maybe the name was a bit unfortunate.

The idea is that you can use the resulting tuple and replace the value for D.x in express.

Maybe we should rename it to something more intuitive, like base_scalars_from_parent.

@Upabjojr

Upabjojr Aug 30, 2017

Contributor

Well, my purpose is to make transformation_from_parent return the combination of the parent's base scalars such that will be equivalent to the child's base scalars. Maybe the name was a bit unfortunate.

The idea is that you can use the resulting tuple and replace the value for D.x in express.

Maybe we should rename it to something more intuitive, like base_scalars_from_parent.

@gxyd gxyd added the vector label Sep 26, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment