Skip to content
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

New matrices support in sympy.physics.matrices #140

Closed
wants to merge 4 commits into from
Closed

New matrices support in sympy.physics.matrices #140

wants to merge 4 commits into from

Conversation

tjl
Copy link
Member

@tjl tjl commented Mar 19, 2011

I've added support in sympy.physics.matrices for calculating the parallel axis theorem matrix for translating the inertia matrix. Currently, it uses individual parameters for the distances dx, dy, dz, but that could be changed if desired.

I've also added rotation matrices for rotation about individual axes. While I use them to rotate the inertia matrix, they could be used elsewhere. I wasn't sure if I should put them in sympy.physics.matrices or directly in matrices.

Tests for these are added to the tests_physics_matrices.py file in the tests subdirectory of physics. I'm not sure how to properly call these from the python prompt though, but all the tests pass when run separately.

rotation about a single axis to sympy.physics.matrices. Tests
for these were added to tests_physics_matrices.py.
st = sin(theta)
mat = ((1,0,0),
(0,ct,st),
(-st,ct,0))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

check this line, cell with 1, must not be intersected with cos and sin cells.

…ean up the associated tests which were needlessly verbose.
r2_minus = rotAxis1(-pi/3)
assert r3_minus*r3_plus*eye(3)*r3_plus.transpose()*r3_minus.transpose() == eye(3)
assert r2_minus*r2_plus*eye(3)*r2_plus.transpose()*r2_minus.transpose() == eye(3)
assert r1_minus*r1_plus*eye(3)*r1_plus.transpose()*r1_minus.transpose() == eye(3)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also it would be useful if you create test saying that trace of rotation matrix is equal 1 + 2*cos(theta)

@goodok
Copy link
Contributor

goodok commented Mar 19, 2011

I'm not sure how to properly call these from the python prompt though, but all the tests pass when run separately.

Try
./bin/test
and
./bin/doctest

To be sure that yours test are connected properly you can play with them (made them incorrect) and see reaction in this case (letters DO NOT COMMIT).

@tjl
Copy link
Member Author

tjl commented Mar 19, 2011

I want to know how to run just one set of tests, not the whole set. Running ./bin/test runs the whole set.

@goodok
Copy link
Contributor

goodok commented Mar 19, 2011

Taking into consideration that rotation can happen not only in physics, but in mathematics and geomentry too I think that this methods can be placed in matrices module.

What about ./bin/test, if you want to test one set of tests try

./bin/test sympy/concrete/tests/test_products.py

but at the same time you must be sure that yours tests are connected with './bin/test' properly too.

st = sin(theta)
mat = ((ct,0,st),
(0,1,0),
(st,0,-ct))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, check this line too.
So can be added another test: that when theta==0 then rotation is equal to identity matrix).

@tjl
Copy link
Member Author

tjl commented Mar 19, 2011

I've fixed this. All tests pass, but I'm going to move the rotation matrices to the matrices module and test again.

@tjl
Copy link
Member Author

tjl commented Mar 19, 2011

I've moved the rotation matrices over to matrices.py in the matrices module and removed them from physics.matrices. The pat_matrix function for parallel axis theorem is still in the physics.matrices module and running ./bin/test everything seems fine so I'll commit the new version.

…to the matrices module, made minor corrections and added new tests. The parallel axis theorem matrix remains in the physics.matrices module. All tests now pass and the functions have been re-named to match PEP8 expectations.
@vks
Copy link
Contributor

vks commented Mar 31, 2011

I want to know how to run just one set of tests, not the whole set. Running ./bin/test runs the whole set.

Just add an arbitrary string you want to be present in the test's name. In your case for instance:

$ bin/test matr

@@ -2203,3 +2204,31 @@ def symarray(prefix, shape):
arr[index] = Symbol('%s_%s' % (prefix, '_'.join(map(str, index))))
return arr

def rot_axis3(theta):
"""Returns a rotation matrix for a rotation of theta about the 3-axis."""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

doctest is missing

@certik
Copy link
Member

certik commented Apr 3, 2011

Otherwise it looks good, thanks!

@tjl
Copy link
Member Author

tjl commented Apr 3, 2011

I wrote them as separate tests, I was trying to follow the style that I saw in the sympy.physics.matrices. I could move a test or two for pat_matrix so they're doctests, but I think the other tests for the rot_axis functions should stay with the matrix tests. I guess I could add an example for each of the rot_axis functions. I'm guessing there really should be doctests for the other functions in sympy.physics.matrices.

…doctests for pat_matrix in sympy/physics/matrices.py.
@smichr
Copy link
Member

smichr commented Jun 18, 2011

This needs to be rebased and a test run on sympy-bot would be good.

@Krastanov
Copy link
Member

Test results html report: http://pastehtml.com/view/axlivyuqc.html

Automatic review by sympy-bot.

There are merge conflicts at the moment.

@plaes
Copy link
Contributor

plaes commented Jul 27, 2011

I have cleaned up this pull request - rebased, then squashed the commits into one commit because most of the stuff was implemented in first commit and then moved around and rewritten in later commits. I also made some extra cleanups and bunch of fixes (like wrong exception names) in another commit.

https://github.com/plaes/sympy/tree/pull-140

So, should I open a new pull request and this can be closed?

@vks
Copy link
Contributor

vks commented Jul 27, 2011

Yes, please open a new pull request.

@plaes
Copy link
Contributor

plaes commented Jul 27, 2011

Ok, new one is @ #515

@vks
Copy link
Contributor

vks commented Jul 27, 2011

Superseded by pull request #515.

@vks vks closed this Jul 27, 2011
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants