# LagrangesMethod class for sympy.physics.mechanics. #1460

Merged
merged 6 commits into from Aug 13, 2012

None yet

### 9 participants

Contributor
commented Aug 4, 2012
 The docstrings will likely need to be revamped.
Contributor
commented Aug 4, 2012
commented on the diff Aug 4, 2012
sympy/physics/mechanics/lagrange.py
 + arguments. The Lagrangian multipliers are automatically generated and are + equal in number to the constraint equations.Similarly if there are any + non-conservative forces, they can be supplied in a list along with a + ReferenceFrame. This is discussed further in the __init__ method. + + Attributes + ========== + + mass_matrix : Matrix + The system's mass matrix + + forcing : Matrix + The system's forcing vector + + mass_matrix_full : Matrix + The "mass matrix" for the qdot's, qdoubledot's, and the
 smichr Member in my experience, mixing single and double quotes in a docstring generates sphinx errors...let's see if this happens with your request. angadhn Contributor This hasn't been an issue in the "Kane" class, so I presumed it wouldn't be a problem here but that could've been an erroneous assumption.
commented on an outdated diff Aug 4, 2012
sympy/physics/mechanics/lagrange.py
 + + self._L = sympify(Lagrangian) + self.eom = None #initializing the eom Matrix + self._m_cd = Matrix([]) #Mass Matrix of differentiated coneqs + self._m_d = Matrix([]) #Mass Matrix of dynamic equations + self._f_cd = Matrix([]) #Forcing part of the diff coneqs + self._f_d = Matrix([]) #Forcing part of the dynamic equations + self.lam_coeffs = Matrix([]) #Initializing the coeffecients matrix of lams + + self.forcelist = forcelist + self.inertial = frame + + self.lam_vec = Matrix([]) + + + #What used to be the coords method
 smichr Member if it's not there anymore then this comment will have no meaning for anyone looking at this in the future, so it can either be deleted or changed to indicate what you are doing, perhaps?
and 1 other commented on an outdated diff Aug 4, 2012
sympy/physics/mechanics/lagrange.py
 + self.forcelist = forcelist + self.inertial = frame + + self.lam_vec = Matrix([]) + + + #What used to be the coords method + + q_list = list(q_list) + if not isinstance(q_list, list): + raise TypeError('Generalized coords. must be supplied in a list') + self._q = q_list + self._qdots = [diff(i, dynamicsymbols._t) for i in self._q] + self._qdoubledots = [diff(i, dynamicsymbols._t) for i in self._qdots] + + #What used to be the constraints method
 smichr Member ditto about what used to be angadhn Contributor Thanks! I forgot to edit those lines. I had left those in as notes for myself.
and 1 other commented on an outdated diff Aug 4, 2012
sympy/physics/mechanics/lagrange.py
 + raise TypeError('First entry in force pair is a point' + ' or frame') + + else: + term4 = zeros(n,1) + + self.eom = term1 - term2 -term3 -term4 + + #The mass matrix is generated by the following + self._m_d = (self.eom).jacobian(qdd) + + return self.eom + + @property + def mass_matrix(self): + # Returns the mass matrix, which is augmented by the Lagrange
 smichr Member convert to docstring instead of comments angadhn Contributor Once again, I was replicating the presentation of "Kane". I have changed this now though. Thanks.
and 1 other commented on an outdated diff Aug 4, 2012
sympy/physics/mechanics/lagrange.py
 + # an n X n matrix is returned. + # If there are 'n' generalized coordinates and 'm' constraint equations + # have been supplied during initialization then an n X (n+m) matrix is + # returned. The (n + m - 1)th and (n + m)th columns contain the + # coeffecients of the lagrange multipliers. + + if self.eom == None: + raise ValueError('Need to compute the equations of motion first') + if len(self.lam_coeffs) != 0: + return (self._m_d).row_join((self.lam_coeffs).transpose()) + else: + return self._m_d + + @property + def mass_matrix_full(self): + n = len(self._q)
 smichr Member docstring? angadhn Contributor The docstring I had written was a little contorted so I decided to leave it blank. I have added a docstring now though. Just waiting for the decision on the '*'/PEP 8 comments to push again.
commented on the diff Aug 4, 2012
sympy/physics/mechanics/lagrange.py
 + #THE FIRST TWO ROWS OF THE MATRIX + row1 = eye(n).row_join(zeros(n,n)) + row2 = zeros(n,n).row_join(self.mass_matrix) + if self.coneqs != None: + m = len(self.coneqs) + I = eye(n).row_join(zeros(n,n+m)) + below_eye = zeros(n+m,n) + A = (self.mass_matrix).col_join((self._m_cd).row_join(zeros(m,m))) + below_I = below_eye.row_join(A) + return I.col_join(below_I) + else: + A = row1.col_join(row2) + return A + + @property + def forcing(self):
 smichr Member docstring? gilbertgede Contributor @angadhn Check the formatting for single line docstrings that shows up elsewhere and try to match that: """Some descriptive sentence."""
commented on the diff Aug 4, 2012
sympy/physics/mechanics/lagrange.py
 + + if self.coneqs != None: + lam = self.lam_vec + lamzero = dict(zip(lam, [0] * len(lam))) + + #The forcing terms from the eoms + self._f_d = -((self.eom).subs(qddzero)).subs(lamzero) + + else: + #The forcing terms from the eoms + self._f_d = -(self.eom).subs(qddzero) + + return self._f_d + + @property + def forcing_full(self):
 smichr Member docstring?
commented on an outdated diff Aug 4, 2012
sympy/physics/mechanics/lagrange.py
 + else: + #The forcing terms from the eoms + self._f_d = -(self.eom).subs(qddzero) + + return self._f_d + + @property + def forcing_full(self): + if self.eom == None: + raise ValueError('Need to compute the equations of motion first') + if self.coneqs != None: + return (Matrix(self._qdots)).col_join((self.forcing).col_join(self._f_cd)) + else: + return (Matrix(self._qdots)).col_join(self.forcing) + + def rhs(self, method):
 smichr Member convert comments to docstring
commented on the diff Aug 4, 2012
sympy/physics/mechanics/lagrange.py
 + below_eye = zeros(n+m,n) + A = (self.mass_matrix).col_join((self._m_cd).row_join(zeros(m,m))) + below_I = below_eye.row_join(A) + return I.col_join(below_I) + else: + A = row1.col_join(row2) + return A + + @property + def forcing(self): + # Returns the forcing vector + if self.eom == None: + raise ValueError('Need to compute the equations of motion first') + + qdd = self._qdoubledots + qddzero = dict(zip(qdd, [0] * len(qdd)))
 smichr Member PEP8, I believe, would suggest removing spaces around `*` moorepants Member Are you sure? http://www.python.org/dev/peps/pep-0008/#other-recommendations Seems like spaces around these operators is preferred. certik Member Yes, it seems to me as well, that PEP8 says you should have spaces around "*". In either case, I would use whatever looks more readable on the case by case basis, in this PR it seems that having spaces is more readable. asmeurer Member We usually differ from PEP 8 in SymPy for `*` and `**` because it makes reading expressions like `3*x**2 + 2*x**4` easier. Here I would say it's justified, though really either way looks fine. smichr Member I reread PEP8 and see that I was wrong about the `*` -- I was running under the sympy-modifed version (that Aaron cites). So although we are doing exactly what PEP8 says not to do I think it looks better, too.
commented on an outdated diff Aug 4, 2012
sympy/physics/mechanics/lagrange.py
 + ' or tuples') + term4 = zeros(len(qd), 1) + for i,v in enumerate(qd): + for j,w in enumerate(forcelist): + if isinstance(w[0], ReferenceFrame): + speed = w[0].ang_vel_in(N) + term4[i] += speed.diff(v, N) & w[1] + if isinstance(w[0], Point): + speed = w[0].vel(N) + term4[i] += speed.diff(v, N) & w[1] + else: + raise TypeError('First entry in force pair is a point' + ' or frame') + + else: + term4 = zeros(n,1)
 smichr Member space afater comma
commented on an outdated diff Aug 4, 2012
sympy/physics/mechanics/lagrange.py
 + if not isinstance(coneqs, list): + raise TypeError('Enter the constraint equations in a list') + + o = len(coneqs) + + #Creating the multipliers + self.lam_vec = Matrix(dynamicsymbols('lam1:' + str(o+1))) + + #Extracting the coeffecients of the multipliers + coneqs_mat = Matrix(coneqs) + qd = self._qdots + self.lam_coeffs = -coneqs_mat.jacobian(qd) + + #Determining the third term in Lagrange's EOM + #term3 = ((self.lam_vec).transpose() * self.lam_coeffs).transpose() + term3 = self.lam_coeffs.transpose() * self.lam_vec
 smichr Member remove here (and throughout) spaces around `*` as per PEP8
commented on an outdated diff Aug 4, 2012
sympy/physics/mechanics/lagrange.py
 + #Determining the second term in Lagrange's EOM + term2 = (L.jacobian(q)).transpose() + + #term1 and term2 will be there no matter what so leave them as they are + + if self.coneqs != None: + coneqs = self.coneqs + #If there are coneqs supplied then the following will be created + coneqs = list(coneqs) + if not isinstance(coneqs, list): + raise TypeError('Enter the constraint equations in a list') + + o = len(coneqs) + + #Creating the multipliers + self.lam_vec = Matrix(dynamicsymbols('lam1:' + str(o+1)))
 smichr Member add space around `+` in `o+1`
 This pull request fails (merged b3a58b0a into 9625918).
Member
 SymPy Bot Summary: ✳️ All tests have passed. Test command: setup.py test master hash: 9625918 branch hash: b3a58b0a0b6efe5025feedf214575df54ef9b002 Interpreter 1: ✳️ All tests have passed. Interpreter: /usr/local/bin/python2.5 (2.5.6-final-0) Architecture: Linux (64-bit) Cache: yes Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sYtLYiDA Interpreter 2: ✳️ All tests have passed. Interpreter: /usr/bin/python2.7 (2.7.3-candidate-2) Architecture: Linux (64-bit) Cache: yes Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sYxqMjDA Interpreter 3: ✳️ All tests have passed. Interpreter: /usr/bin/python3.2 (3.2.3-candidate-2) Architecture: Linux (64-bit) Cache: yes Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sYs7YiDA Build HTML Docs: ✳️ All tests have passed. Docs build command: make html-errors Sphinx version: 1.1.3 Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sYq48iDA Automatic review by SymPy Bot.
Member
commented Aug 4, 2012
 Thanks for opening the pull request. So the first priority is regular tests. Either add them to some file in sympy/physics/mechanics/tests, or create a new one. Then some doctests showing how to use it in the docstring of the various methods and also what Chris mentioned above. The regular tests are the most important.
commented on the diff Aug 4, 2012
sympy/physics/mechanics/lagrange.py
 @@ -0,0 +1,253 @@ +__all__ = ['LagrangesMethod'] + +from sympy import diff, zeros, Matrix, eye, sympify +from sympy.physics.mechanics import (dynamicsymbols, ReferenceFrame, Point) + +class LagrangesMethod(object):
 moorepants Member the Kane method class isn't called KanesMethod, so why do that here? It's just more to type. There are no other naming conflicts in this module are there? angadhn Contributor So, @hazelnusse, @gilbertgede and I talked about this in the lab yesterday. We talked about how other methods of deriving EOMs are referred to i.e. 'Kane's method', 'Hamilton's method', etc and how maybe the "Kane" class could be renamed to "KanesMethod" possibly. Also, just calling it "Lagrange" might be ambiguous. moorepants Member Ok great, we should change the name of the Kane Class too then. gilbertgede Contributor @moorepants I will take care of that in another PR that I am going to open soon.
 This pull request fails (merged 8c26b504 into 9625918).
Member
 SymPy Bot Summary: 🔴 There were test failures. @angadhn: Please fix the test failures. Test command: setup.py test master hash: 7d92368 branch hash: 8c26b50426c7150788f999ca2b17a2db9cca4c24 Interpreter 1: 🔴 There were test failures. Interpreter: /usr/local/bin/python2.5 (2.5.6-final-0) Architecture: Linux (64-bit) Cache: yes Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sYsKsjDA Interpreter 2: 🔴 There were test failures. Interpreter: /usr/bin/python2.7 (2.7.3-candidate-2) Architecture: Linux (64-bit) Cache: yes Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sYsd0iDA Interpreter 3: ✳️ All tests have passed. Interpreter: /usr/bin/python3.2 (3.2.3-candidate-2) Architecture: Linux (64-bit) Cache: yes Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sYr6sjDA Build HTML Docs: 🔴 There were test failures. Docs build command: make html-errors Sphinx version: 1.1.3 Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sY580iDA Automatic review by SymPy Bot.
 This pull request fails (merged 3092f5e1 into 7d92368).
Member
 SymPy Bot Summary: 🔴 There were test failures. @angadhn: Please fix the test failures. Test command: setup.py test master hash: 1627b32 branch hash: 3092f5e14af92de40c7c14496902ebfe46f15641 Interpreter 1: 🔴 There were test failures. Interpreter: /usr/local/bin/python2.5 (2.5.6-final-0) Architecture: Linux (64-bit) Cache: yes Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sYz6MjDA Interpreter 2: 🔴 There were test failures. Interpreter: /usr/bin/python2.7 (2.7.3-candidate-2) Architecture: Linux (64-bit) Cache: yes Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sYle0iDA Interpreter 3: ✳️ All tests have passed. Interpreter: /usr/bin/python3.2 (3.2.3-candidate-2) Architecture: Linux (64-bit) Cache: yes Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sYoIwjDA Build HTML Docs: ✳️ All tests have passed. Docs build command: make html-errors Sphinx version: 1.1.3 Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sYjvAhDA Automatic review by SymPy Bot.
 This pull request fails (merged 619dcc6d into 65b6582).
 This pull request fails (merged 84a5ea6f into 65b6582).
Member
 SymPy Bot Summary: 🔴 There were test failures. @angadhn: Please fix the test failures. Test command: setup.py test master hash: 98cc80f branch hash: 84a5ea6f2899b4dbce6df52b3c0b6f01ad4497f9 Interpreter 1: 🔴 There were test failures. Interpreter: /usr/local/bin/python2.5 (2.5.6-final-0) Architecture: Linux (64-bit) Cache: yes Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sY8P8hDA Interpreter 2: 🔴 There were test failures. Interpreter: /usr/bin/python2.7 (2.7.3-candidate-2) Architecture: Linux (64-bit) Cache: yes Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sYjbMjDA Interpreter 3: ✳️ All tests have passed. Interpreter: /usr/bin/python3.2 (3.2.3-candidate-2) Architecture: Linux (64-bit) Cache: yes Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sY7_8hDA Build HTML Docs: ✳️ All tests have passed. Docs build command: make html-errors Sphinx version: 1.1.3 Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sYu6sjDA Automatic review by SymPy Bot.
and 1 other commented on an outdated diff Aug 8, 2012
sympy/physics/mechanics/tests/test_lagrange.py
 + # of the string fixed frame is redefined as 'LagrangesMethod' doesn't require + # generalized speeds, per se. (Lagrangian mechanics requires 'simple' + # generalized speeds) + A.orientnew('A', 'Axis', [q, N.z]) + A.set_ang_vel(N, qd *A.z) + P.v2pt_theory(O,N,A) + T = 1/2.0 * m * P.vel(N) & P.vel(N) # T is the kinetic energy of the system + V = - m * g * l * cos(q) # V is the potential energy of the system + L = T - V # L is the Lagrangian + + # The 'LagrangesMethod' class is invoked and the equations of motion are generated. + l = LagrangesMethod(L, [q]) + l.lagranges_equations() + l.rhs("GE") + + # Finally, we the results of both methods and it's seen that they are
 certik Member the word "compare" is missing? angadhn Contributor thanks!
and 1 other commented on an outdated diff Aug 8, 2012
sympy/physics/mechanics/lagrange.py
 + The system's mass matrix + + forcing : Matrix + The system's forcing vector + + mass_matrix_full : Matrix + The "mass matrix" for the qdot's, qdoubledot's, and the + lagrange multipliers (lam) + + forcing_full : Matrix + The forcing vector for the qdot's, qdoubledot's and + lagrange multipliers (lam) + + rhs : Matrix + Solves for the states (i.e. q's, qdot's, and multipliers) +
 certik Member Can you put a simple doctest here showing how to use the class? angadhn Contributor I've added it now.
Member
commented Aug 8, 2012
 You seem to have some trailing whitespace problems there: http://travis-ci.org/#!/sympy/sympy/jobs/2056371
commented on an outdated diff Aug 8, 2012
sympy/physics/mechanics/lagrange.py
 + + self.lam_vec = Matrix([]) + + + # Creating the qs, qdots and qdoubledots + + q_list = list(q_list) + if not isinstance(q_list, list): + raise TypeError('Generalized coords. must be supplied in a list') + self._q = q_list + self._qdots = [diff(i, dynamicsymbols._t) for i in self._q] + self._qdoubledots = [diff(i, dynamicsymbols._t) for i in self._qdots] + + self.coneqs = coneqs + + def lagranges_equations(self):
commented on an outdated diff Aug 8, 2012
sympy/physics/mechanics/lagrange.py
 + self.lam_coeffs = -coneqs_mat.jacobian(qd) + + #Determining the third term in Lagrange's EOM + #term3 = ((self.lam_vec).transpose() * self.lam_coeffs).transpose() + term3 = self.lam_coeffs.transpose() * self.lam_vec + + #Taking the time derivative of the constraint equations + diffconeqs = [diff(i, dynamicsymbols._t) for i in coneqs] + + #Extracting the coeffecients of the qdds from the diff coneqs + diffconeqs_mat = Matrix(diffconeqs) + qdd = self._qdoubledots + self._m_cd = diffconeqs_mat.jacobian(qdd) + + #The remaining terms i.e. the 'forcing' terms in diff coneqs + qddzero = dict(zip(qdd, [0] * len(qdd)))
 gilbertgede Contributor Isn't len(qdd) equal to n, which you use elsewhere?
Contributor
commented Aug 8, 2012
 So it appears that I have mistakenly rebased a couple of my branches and so my commits (including the most recent one on this page) aren't showing up correctly. Any way I can fix this?
Contributor
commented Aug 8, 2012
 Actually, it's fine here. Never mind.
 This pull request fails (merged 758c380b into 0d88b25).
Member
commented Aug 8, 2012
 You still have a trailing whitespace in `lagrange.py` line 49, see the Travis test results: ``````AssertionError: File contains trailing whitespace: /home/vagrant/virtualenv/python2.6/lib/python2.6/site-packages/sympy/physics/mechanics/lagrange.py, line 49. ``````
Member
 SymPy Bot Summary: 🔴 There were test failures. @angadhn: Please fix the test failures. Test command: setup.py test master hash: 0d88b25 branch hash: 758c380b3b2d871776782ceb5da985b936e58fb4 Interpreter 1: 🔴 There were test failures. Interpreter: /usr/local/bin/python2.5 (2.5.6-final-0) Architecture: Linux (64-bit) Cache: yes Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sYwqsjDA Interpreter 2: 🔴 There were test failures. Interpreter: /usr/bin/python2.7 (2.7.3-candidate-2) Architecture: Linux (64-bit) Cache: yes Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sYou0iDA Interpreter 3: ✳️ All tests have passed. Interpreter: /usr/bin/python3.2 (3.2.3-candidate-2) Architecture: Linux (64-bit) Cache: yes Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sY-JsjDA Build HTML Docs: ✳️ All tests have passed. Docs build command: make html-errors Sphinx version: 1.1.3 Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sYkZQjDA Automatic review by SymPy Bot.
 This pull request passes (merged 9d7d8b3b into 0d88b25).
 This pull request passes (merged e4d9026f into 0d88b25).
Member
 So this pull request may take a while to get in. It implements a complicated method and the testing is still weak. @angadhn pulled earlier than usual so that we can comment on the code easily and discuss some of the issues. Is there anyway not to get bombarded by the bots constantly? I personally find it stressful. We probably only need the bot testing when the PR is closer to being mergable.
Member
 SymPy Bot Summary: ✳️ All tests have passed. Test command: setup.py test master hash: 0d88b25 branch hash: e4d9026f4f3ad212df580540d3018c28e81fdfda Interpreter 1: ✳️ All tests have passed. Interpreter: /usr/local/bin/python2.5 (2.5.6-final-0) Architecture: Linux (64-bit) Cache: yes Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sYoPAhDA Interpreter 2: ✳️ All tests have passed. Interpreter: /usr/bin/python2.7 (2.7.3-candidate-2) Architecture: Linux (64-bit) Cache: yes Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sYjcYiDA Interpreter 3: ✳️ All tests have passed. Interpreter: /usr/bin/python3.2 (3.2.3-candidate-2) Architecture: Linux (64-bit) Cache: yes Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sY3KMjDA Build HTML Docs: ✳️ All tests have passed. Docs build command: make html-errors Sphinx version: 1.1.3 Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sY5tUiDA Automatic review by SymPy Bot.
Contributor
commented Aug 9, 2012
 So the docs commit came into Lagrange because I thought that the Lagrange class was complete w.r.t. the tests; they are identical in number to those in Kane (apart from that I will also be adding the rolling disc). The two degree of freedom test in my case is the double pendulum. Like Kane, I also have the simple pendulum. Lagrange's equations don't need the test for auxiliary equations and ditto for the parallel axis theorem.
 This pull request passes (merged e5d912a2 into 15c2c89).
Member
 Angadh, your current tests don't really cover the use cases of the class. An ideal set of tests would probably include a test for each problem case individually and at least one test case problem that covers a problem that has all dynamics features. This way you know that your class plays well with whatever the user may throw at it. It will also prove that your class gives the correct answer for a robust dynamics problem and if it does that then we would all be convinced that it can probably handle any lesser problems. You'd hate to send out a class into the world that incorrectly derives equations of certain types of problems. I think you need at least a 3D problem with multiple degrees of freedom, both holonomic and non-holonomic constraints, and analytically unsolvable kinematic loop. The nice thing is that Gilbert already has problems like this in his Kane test suite. The only thing you have to do is to write out the Lagrangian for those problems and then see if your Lagrange class gives the same answer as Gilbert's. In fact, both Kane and Lagrange classes should be tested by the same problem test suite as they are both just different ways to crack the coconut and should give the exact same answer.
commented on an outdated diff Aug 9, 2012
sympy/physics/mechanics/tests/test_lagrange.py
 + N = ReferenceFrame('N') + A = N.orientnew('A', 'Axis', [q, N.z]) + A.set_ang_vel(N, qd * N.z) + + # Next, we create the point O and fix it in the inertial frame. We then + # locate the point P to which the bob is attached. Its corresponding + # velocity is then determined by the 'two point formula'. + O = Point('O') + O.set_vel(N, 0) + P = O.locatenew('P', l * A.x) + P.v2pt_theory(O, N, A) + + # The 'Particle' which represents the bob is then created. + Pa = Particle('Pa', P, m) + + T = 1/2.0 * m * P.vel(N) & P.vel(N) # T is the kinetic energy of the system
 moorepants Member Whenever you use the & and ^ for the dot and cross products it is probably good practice to always enclose the operation in parentheses because these two operators have special order of operation rules that don't necessarily act like you think they do. In this case it is fine, but I think all example code that a user may possibly look at to figure out how to write their own problems should have this explicit. New users may spend forever trying to figure out the bug if they type something like ``````A & B ^ C ^ D `````` and expect the order of operations to match they way they think about it on paper. Look back on the sympy mailing list, the online docs, or old pull requests to see how these operators work (or you may already know). http://docs.sympy.org/dev/modules/physics/mechanics/vectors.html#vector-algebra-in-mechanics only has a small warning about this.
 This pull request passes (merged 85ac0417 into 15c2c89).
and 1 other commented on an outdated diff Aug 9, 2012
sympy/physics/mechanics/lagrange.py
 + + mass_matrix : Matrix + The system's mass matrix + + forcing : Matrix + The system's forcing vector + + mass_matrix_full : Matrix + The "mass matrix" for the qdot's, qdoubledot's, and the + lagrange multipliers (lam) + + forcing_full : Matrix + The forcing vector for the qdot's, qdoubledot's and + lagrange multipliers (lam) + + rhs : Matrix
 moorepants Member This is a method, so you should have a Methods section to accompany your Attributes section. Also, don't forget the form_equations method. gilbertgede Contributor Jason, I don't think that Methods are usually listed in the class docstring, although rhs should be taken out of here. moorepants Member Ok, here is some potential reference material for that: https://github.com/numpy/numpy/blob/master/doc/HOWTO_DOCUMENT.rst.txt#documenting-classes Although, I don't know the standard in SymPy is.
 This pull request fails (merged cdaf29ba into 15c2c89).
Contributor
 @moorepants I have to disagree with you on some of these points Jason. We just realized that the Kane class actually returns a negative mass matrix and a negative forcing vector. In all calculations that have been done, these cancel out and it goes unnoticed. Angadh and I discovered it today though. I suggested that he ensure that his results check out against hand/reference calculations rather than what the Kane class gives (also, if any other changes ever break Kane, Lagrange won't fail it's tests). Not all of the test cases go between the 2 classes either - if the generalized speeds are not just the derivatives of the coordinates, things get messy. That being said, storing some of these reference results that could be used for both cases in 1 place seems like a good idea. Also, Angadh is working to get reference results for a few more tests. Note that the Kane class does not have a test within SymPy for holonomic and nonholonomic constraints.
 This pull request fails (merged 0fc013fe into 15c2c89).
Member
 I'm not sure what you are disagreeing with, seems more like agreeing. I'm glad you found the bug in the Kane class. Yes, results should always check against well known results that are in literature as the de facto test. I'm suggesting that both classes can test to the same benchmark problems from literature. i.e. setup up a benchmark problem with known canonical results and then run the two classes to see if you get the same answers as literature. There seems to be little reason to make different test problems for both of the classes. These test cases certainly need to be designed such that the resulting equations of motion use the same coordinates and such, but they certainly should both be able to solve the same problems and give the same EoMs. Secondly, maintaining different sets of benchmark tests for many automated EoM derivation methods will ultimately be a nightmare. I can imagine that all methods test to the same benchmarks, but then there may be some special test cases for each Method class that test the particularities and advantages of one method or an another. I thought you have the Bicycle example as a test for a "complete" dynamics problem. This test could probably be run in the test suite even though it is slower, especially since the tests are mostly run by the bots in the background. Also a simple four bar linkage or a 3D version of one could be a nice holonomic/kinematic loop test. I think the problem in Kane's online dynamics in Chapter Zero is a good one for that. Lastly, we should find a problem that is "complete" but which is simpler than the bicycle for computation time reasons. We've talked about this in the past and I vaguely remember suggesting one. I'll dig around for that.
Member
 SymPy Bot Summary: ✳️ All tests have passed. Test command: setup.py test master hash: 15c2c89 branch hash: 0fc013fe86fdfd38d84a04d1b1d611e4250acac2 Interpreter 1: ✳️ All tests have passed. Interpreter: /usr/local/bin/python2.5 (2.5.6-final-0) Architecture: Linux (64-bit) Cache: yes Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sYxt0iDA Interpreter 2: ✳️ All tests have passed. Interpreter: /usr/bin/python2.7 (2.7.3-candidate-2) Architecture: Linux (64-bit) Cache: yes Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sYgpwjDA Interpreter 3: ✳️ All tests have passed. Interpreter: /usr/bin/python3.2 (3.2.3-candidate-2) Architecture: Linux (64-bit) Cache: yes Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sY_P8hDA Build HTML Docs: ✳️ All tests have passed. Docs build command: make html-errors Sphinx version: 1.1.3 Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sYgZwjDA Automatic review by SymPy Bot.
Member
 I asked Mont if he knew of a simpler problem than the bicycle which was full featured and he suggested the unicycle with the riders leg(s). The leg and crank form a four bar linkage. He doubted than anyone had the solution to this problem written down, but it may be possible. We could compute the EoM's using Autolev and then compare the results to what the classes in SymPy give if writing the solution by hand is a mess. I was think of something similar today too that added a four bar linkage to the the rolling disc, but it doesn't have as nice of a physical interpretation. I'm willing to make this test up, but can't do it till after he 21st (my dissertation due date).
Contributor
 Why not the rolling disc with non-minimal choice of coordinates and speeds? Though the constraints are somewhat artificial with this approach, they have the same form as more complicated problems and the benefit is that there are well established solutions for this system. Computation time running the tests, but also time implementing them correctly is definitely a factor to consider. Luke
Member
commented Aug 10, 2012
 @gilbertgede, @moorepants, are you ok with pusing this in? It looks good to me. @angadhn, for the future, see here how to write good commit messages: https://plus.google.com/u/0/104039945248245758823/posts/GcJmpxCVDfg So for example instead of "More edits." it's better to explain what exactly you did and why. See the blog post, it can be just one line, or more paragraphs if needed.
Member
 I think we should make sure at least the rolling disc test works before this goes in and ideally we test a more robust problem. @angadhn Luke suggested over defining the rolling disc problem so that you have more constraints, thus making the problem a little more robust at testing the method. Jason On Fri, Aug 10, 2012 at 9:53 AM, Ondřej Čertík notifications@github.comwrote: @gilbertgede https://github.com/gilbertgede, @moorepantshttps://github.com/moorepants, are you ok with pusing this in? It looks good to me. @angadhn https://github.com/angadhn, for the future, see here how to write good commit messages: https://plus.google.com/u/0/104039945248245758823/posts/GcJmpxCVDfg So for example instead of "More edits." it's better to explain what exactly you did and why. See the blog post, it can be just one line, or more paragraphs if needed. — Reply to this email directly or view it on GitHubhttps://github.com/sympy/sympy/pull/1460#issuecomment-7650324. Personal Website http://biosport.ucdavis.edu/lab-members/jason-moore Sports Biomechanics Lab http://biosport.ucdavis.edu, UC Davis Davis Bike Collective http://www.davisbikecollective.org Minister, Davis, CA BikeDavis.info Google Voice: +01 530-601-9791 Home: +01 530-753-0794 Office: +01 530-752-2163 Lab: +01 530-752-2235
Contributor
commented Aug 10, 2012
 @moorepants Yeah. I'm redoing the rolling disc.
Contributor
 I agree with @moorepants , we should add at least 1 test which is more complex, but I am not sure how complex we should make it. @angadhn is working on a test right now though, so something should be up soon.
Member
commented Aug 10, 2012
Contributor
commented Aug 10, 2012
 Everyone, I just wanted to give a heads up that I will be rebasing this branch; I need to have access to the energy functions that just got merged into master. as I will need to use this for the 'LagrangesMethod' tests. And I won't amend the commits this time around ;)
added some commits Aug 4, 2012
 angadhn `Revised6: LagrangesMethod class for sympy.physics.mechanics.` `a054386` angadhn `Minor edits.` `9f5999f` angadhn `ADDED ROLLING DISC TEST AND MINOR EDITS.` ```The rolling disc test has been included. Two minor edits have been included: - addition of the energy function to determine the Lagrangian. - slight modification of the 'rhs' method in the LagrangesMethod class.``` `1171a1c`
Contributor
commented Aug 10, 2012
 @certik I have added the test for the rolling disc. @hazelnusse @moorepants I tried to run the rolling disc with the non-minimal coordinates and so did @gilbertgede. It took forever to run. 12 mins into running the test, there was still no output so we decided to stick with the three generalized coordinates. I guess having trivial generalized (speeds as defined by Lagrange) slows things down drastically when we have 6 generalized coordinates AND three constraint equations.
 This pull request passes (merged 1171a1c into 2797590).
sympy/physics/mechanics/tests/test_lagrange.py
 + C.set_vel(N, 0) + Dmc = C.locatenew('Dmc', r * L.z) + Dmc.v2pt_theory(C, N, R) + + # Forming the inertia dyadic. + I = inertia(L, m / 4 * r**2, m / 2 * r**2, m / 4 * r**2) + BodyD = RigidBody('BodyD', Dmc, R, m, (I, Dmc)) + + # Finally we form the equations of motion, using the same steps we did + # before. Supply the Lagrangian, the generalized speeds. + T = kinetic_energy(N, BodyD) + BodyD.set_potential_energy(- m * g * r * cos(q2)) + V = potential_energy(BodyD) + Lag = T - V + q = [q1, q2, q3] + l = LagrangesMethod(Lag, q)
 moorepants Member I'm wondering if this would flow better like: ```BodyD.set_potential_energy(- m * g * r * cos(q2)) q = [q1, q2, q3] l = LagrangesMethod(BodyD, (q1, q2, q3))``` For 99% of problems the Lagrangian is T - V. I'm not sure why it would be otherwise. So why force all this extra typing? The above will mean that the kinetic energy and potential energy calls happen inside the Lagrange class. So for that 1% where you might want to define a different Lagrangian, why not just have either an optional keyword argument or check the first object to see if it is a Body, Particle, or an expression. So you can either type: `LagrangesMethod(body1, body2, particle1, particle2, (q1, q2, q3))` or `LagrangesMethod(L, (q1, q2, q3))`
Member
 I fine with the tests as is. Nice job on this! I've only the one comment above that I'm curious about the design of the code and why the Lagrangian can't be formed in the class.
Contributor
commented Aug 11, 2012
 How about adding a 'Lagrangian' function in the functions.py instead. That way users can feed in a Lagrangian however they desire. I'm thinking of something like this in 'functions.py'- ``````def Lagrangian(frame, *body): "compute Lagrangian and return it here" `````` The frame would still be needed to determine the kinetic energy. What do you think?
Contributor
commented Aug 11, 2012
 Also a git question- After having rebased and committed once (without amending), will it be okay to 'git commit --amend'?
Member
 SymPy Bot Summary: 🔴 There were test failures. @angadhn: Please fix the test failures. Test command: setup.py test master hash: 2797590 branch hash: 1171a1c Interpreter 1: 🔴 There were test failures. Interpreter: /usr/local/bin/python2.5 (2.5.6-final-0) Architecture: Linux (64-bit) Cache: yes Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sYjvUiDA Interpreter 2: 🔴 There were test failures. Interpreter: /usr/bin/python2.7 (2.7.3-candidate-2) Architecture: Linux (64-bit) Cache: yes Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sY1Y8iDA Interpreter 3: ✳️ All tests have passed. Interpreter: /usr/bin/python3.2 (3.2.3-candidate-2) Architecture: Linux (64-bit) Cache: yes Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sY2bYiDA Build HTML Docs: ✳️ All tests have passed. Docs build command: make html-errors Sphinx version: 1.1.3 Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sYr_AhDA Automatic review by SymPy Bot.
Contributor
 I'm concerned about the test taking 12 minutes. Where was the majority of the time spent? Changing back to the minimal coordinates and speeds seems to imply you won't be testing functionality that is really important to this class. Maybe I'm missing something but to me, 12 minutes seems to be an indication that your algorithm needs improvement and/or the way you are using core sympy functionality is less than optimal. Can you explain why it was taking so long?
Member
 `git commit --amend` is essentially the same as `git rebase HEAD~1`, as far as changing the history goes.
Contributor
commented Aug 11, 2012
 @hazelnusse Things get slow at the mass matrix inversion. The 'form_lagranges_equations' method doesn't stall so I'm wondering if maybe the issue lies in inverting a huge mass matrix, in this case, it was a 15 by 15 matrix.
Member
commented Aug 11, 2012
 If you use `git commit -a` (a=all, not ammend) it will do what you are thinking, I believe: just add the changed files and put you in the editor to add a message. If you do `git commit -a -m "a title"` for a title-only commit and it will bypass the editing step.
Contributor
 @angadhn Is the mass matrix inversion for solving for the q double dot's and the lagrange multipliers? Is this done by the class, or in user code after the mass matrix and forcing function has been obtained? If the former, think this is a pretty clear example of why coupling your class with a linear system solver is dicey. Also, what technique were you using to do the matrix inverse? I think in my experience, doing something like M.adjugate() / M.det(method='...') was pretty fast, even for relatively big matrices.
Contributor
commented Aug 11, 2012
 @hazelnusse yes. and also solves for the qdots (which is of course trivial.) . And yes, this is done within clsas in the 'rhs' method. The inversion of the mass matrix MM is done by ``````MM.inv("GE", try_block_diag = True) `````` I will try it out the way you have suggested and see how it works.
Contributor
commented Aug 11, 2012
 @hazelnusse no dice. it seems to be taking forever to compute the determinant as well as the adjoint of the mass matrix.
Contributor
commented Aug 11, 2012
 I stand corrected. so the issue seems to be with printing the adjugate and not calculating it. As for just using the the method buil in to the class, that does take forever.
Contributor
 "the method" == ? Are you using dummy symbols for this matrix? On Fri, Aug 10, 2012 at 8:58 PM, Angadh Nanjangud notifications@github.comwrote: I stand corrected. so the issue seems to be with printing the adjugate and not calculating it. As for just using the the method buil in to the class, that does take forever. — Reply to this email directly or view it on GitHubhttps://github.com/sympy/sympy/pull/1460#issuecomment-7663710. “People call me a perfectionist, but I'm not. I'm a rightist. I do something until it's right, and then I move on to the next thing.” ― James Cameron
Contributor
commented Aug 11, 2012
 the method == rhs method to solve for the states.
 angadhn `ADDED LAGRANGIAN AND SOME OTHER CORRECTIONS` ```A Lagrangian function has been added to 'functions.py' and appropriately used now in the tests. Corrected an error in the docstring of 'kinetic_energy'. Modified the 'potential_energy' methods for both Particle and RigidBody; the default value is now zero thus preventing the user from having to set potential energy when there is none.``` `ba38ebe`
Contributor
commented Aug 11, 2012
 @smichr @asmeurer Thanks for the information. Very helpful. @hazelnusse Thanks for showing the path to faster inversions. Very insightful. Nonetheless, I have spent a good few hours since we met trying to decipher the gargantuan 'rhs' terms and I didn't find a feasible way to make those terms any simpler. It's quite horrendous as you may have seen. @hazelnusse @moorepants @gilbertgede I will try to find a "simpler" test case for nonholnomicity later if you are okay with it as I personally feel this doesn't fit the bill in the case of using Lagrange's method.
 This pull request passes (merged ba38ebe into 2797590).
Member
 SymPy Bot Summary: ✳️ All tests have passed. Test command: setup.py test master hash: 3b77c94 branch hash: ba38ebe Interpreter 1: ✳️ All tests have passed. Interpreter: /usr/local/bin/python2.5 (2.5.6-final-0) Architecture: Linux (64-bit) Cache: yes Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sY148iDA Interpreter 2: ✳️ All tests have passed. Interpreter: /usr/bin/python2.7 (2.7.3-candidate-2) Architecture: Linux (64-bit) Cache: yes Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sY2rYiDA Interpreter 3: ✳️ All tests have passed. Interpreter: /usr/bin/python3.2 (3.2.3-candidate-2) Architecture: Linux (64-bit) Cache: yes Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sYiYAiDA Build HTML Docs: ✳️ All tests have passed. Docs build command: make html-errors Sphinx version: 1.1.3 Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sY7qMjDA Automatic review by SymPy Bot.
Member
commented Aug 11, 2012
 It's definitely ok with me to merge this now, if you want. Or should I wait?
 angadhn `ADDED WHITESPACE AFTER A COMMA.` ```I had forgotten to address one of Ondrej's comments on PR 1407 so I have taken care of that here.``` `ce17e7b`
 This pull request passes (merged ce17e7b into 2797590).
Member
 SymPy Bot Summary: ✳️ All tests have passed. Test command: setup.py test master hash: b0a9866 branch hash: ce17e7b Interpreter 1: ✳️ All tests have passed. Interpreter: /usr/local/bin/python2.5 (2.5.6-final-0) Architecture: Linux (64-bit) Cache: yes Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sYvowjDA Interpreter 2: ✳️ All tests have passed. Interpreter: /usr/bin/python2.7 (2.7.3-candidate-2) Architecture: Linux (64-bit) Cache: yes Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sYocYiDA Interpreter 3: ✳️ All tests have passed. Interpreter: /usr/bin/python3.2 (3.2.3-candidate-2) Architecture: Linux (64-bit) Cache: yes Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sY8aMjDA Build HTML Docs: ✳️ All tests have passed. Docs build command: make html-errors Sphinx version: 1.1.3 Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sY0t0iDA Automatic review by SymPy Bot.
Contributor
 @certik I'm OK with this going in - the tests aren't as complex as they could be, but I believe that all the functionality is covered.
 angadhn `ADDED USAGE OF LAGRANGIAN IN DOCSTRING EXAMPLE.` `d04c599`
Contributor
commented Aug 12, 2012
 @certik With that last commit I'm OK with this going in too.
 This pull request passes (merged d04c599 into 2797590).
Member
 SymPy Bot Summary: ✳️ All tests have passed. Test command: setup.py test master hash: b0a9866 branch hash: d04c599 Interpreter 1: ✳️ All tests have passed. Interpreter: /usr/local/bin/python2.5 (2.5.6-final-0) Architecture: Linux (64-bit) Cache: yes Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sY_dUiDA Interpreter 2: ✳️ All tests have passed. Interpreter: /usr/bin/python2.7 (2.7.3-candidate-2) Architecture: Linux (64-bit) Cache: yes Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sYwYwjDA Interpreter 3: ✳️ All tests have passed. Interpreter: /usr/bin/python3.2 (3.2.3-candidate-2) Architecture: Linux (64-bit) Cache: yes Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sY_NUiDA Build HTML Docs: ✳️ All tests have passed. Docs build command: make html-errors Sphinx version: 1.1.3 Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sYrbMjDA Automatic review by SymPy Bot.
merged commit `86696a5` into sympy:master Aug 13, 2012

#### 1 check passed

default The Travis build passed
Details
Member
commented Aug 13, 2012
 Excellent, it's in!
Contributor
commented Aug 13, 2012
 Everyone- thanks a lot for all the input.
Member
 This has led to some test failures in master. See #1370 (comment).
Member
 Actually, I can't reproduce those. They might be related to the copy_py3k_sympy feature of sympy-bot.
Contributor
 Yeah, it appears to only be with the python3 tests. Is this just a random issue, or will this code keep popping up as an issue?
Member
 Hopefully it will go away. I think it was an issue of py3k-sympy not being updated correctly.
Member
commented Aug 15, 2012
 The Python 3.2 tests are well tested by travis: http://travis-ci.org/#!/sympy/sympy/builds you can see that there are no failures in master, except some bugs in Travis itself.
Contributor
commented Aug 15, 2012
 That's a relief! Thanks!