Mechanics improvements #7790

jcrist · 2014-07-25T23:15:08Z

Improvements to a pile of things in sympy.physics.mechanics. In order of importance:

Addition of msubs: a specialized subs function for use on the large expressions in mechanics
Pulled out common find_dynamicsymbols code into a function of that name. Finds all the dynamicsymbols in an expression. Optional exclude kwarg to ignore certain symbols.
Speed improvements to all Linearizer and related methods. Runs approximately twice as fast now.
Refactor of KanesMethod and LagrangesMethod

The refactoring was mainly to:

Remove dead code (there was some in both)
Improve readability
Make some code more pythonic
Improve efficiency.

The overall useage of each class is the same, no user facing changes.

- Removed `_mat_mul_inv` in favor of LUsolve. This provides a significant speed boost. - Added `msubs` function. This is a custom `subs` that replaces the old `_subs_keep_derivs` function, but does its work in only one pass, while providing checking for `nan` and `oo` in the expression tree.

Small change to utilize existing KanesMethod attributes in forming the general form representation. Also refactored __init__ of Linearizer to pull slow code out into separate function to be run on first call to Linearizer.linearize. This trades faster object creation for a slower first method call.

It's slightly slower to partition with msubs than using KM._f_d and KM._k_d, but results in a significantly smaller expression size - yielding overall gains.

Added missing call to _recurser in _smart_subs.

Refactoring to improve readability, remove dead code, and improve efficiency. What's done so far: - __init__ (and all _initialize* methods) rewritten in more readable style. - Some dead code/unused attribuetes removed - Internal state is no kept in matrices, not lists.

- Cleaned up code to be more readable, no major changes in algorithms. - Added option to msubs to accept several dicts as *args.

- Pulled out _find_dynamicsymbols into a separate function - Deleted unused _othersymbols function - Refactored out _partial_derivatives class method - Some code cleanup - Added tests for find_dynamicsymbols and msubs

Added doc page for linearization methods in physics/mechanics.

Conflicts: sympy/physics/mechanics/functions.py sympy/physics/mechanics/kane.py sympy/physics/mechanics/lagrange.py

Code cleanup of the LagrangesMethod class - Removed dead code - Refactored to be more efficient in places - Moved a common inner loop between KanesMethod and LagrangesMethod to the functions file. Function is _f_list_parser, and is internal only

jcrist · 2014-07-25T23:20:57Z

@moorepants : I also have the documentation for everything done so far, but it's a superset of this PR (contains the new functions). Should I also add those commits to this, or wait for this to merge?

moorepants · 2014-07-25T23:27:26Z

Either way. I won't have time to look at it this weekend because I'll be camping.

jcrist · 2014-07-25T23:28:35Z

That's fine. I'm just wondering if they should go in together or not.

moorepants · 2014-07-25T23:30:42Z

This is alot so maybe keep them separate.

btw, does the bicycle test run with these changes?

jcrist · 2014-07-25T23:32:55Z

It does not. But it doesn't run with what's in there right now either (at least not in reasonable time). Still working on that. This just results in a significant speedup. The fix for the bicycle test will come later.

Also fixed docstrings to pass doctests.

Added documentation for linearization methods, and all assorted changes that occurred along with them.

tarzzz · 2014-07-27T18:33:57Z

Travis doesn't seem happy:

File "/home/travis/virtualenv/python2.6.9/lib/python2.6/site->packages/sympy/physics/mechanics/functions.py", line 448
def msubs(expr, *sub_dicts, smart=False):
^
SyntaxError: invalid syntax

tarzzz · 2014-07-27T18:42:01Z

sympy/physics/mechanics/functions.py

+        if iterable(exclude):
+            exclude_set = set(exclude)
+        else:
+            raise TypeError("exclude must be an iterable")


small typo: "exclude must be iterable". "iterable" is used as a verb, rather than a noun,(as observed in errors in other core modules)

jcrist · 2014-07-28T02:28:38Z

Yeah. I never tested it on Python 2 (woops). Fix coming soon. There were also some issues with the doc tests, which were fixed in my docs branch. I think for ease of git I'm just going to push them all at the same time.

- Updates to docstrings to render well in sphinx docs - Added prose walkthrough of example sections - Spelling and grammar fixes

jcrist · 2014-07-28T06:20:25Z

So I pushed the documentation stuff in with this PR because the doctets errors were already fixed in that branch, and this just seemed easier. However, this is no a lot to review. Most of it is just code cleanup, with only a few new, user facing changes. For ease of those reviewing, here's what changed (important things start with +)

Code

+ New function: msubs in functions.py
+ New function: find_dynamicsymbols in functions.py
+ Refactor of KanesMethod and LagrangesMethod to improve speed and readability. No user facing changes here, but this is important code, so you may want to glance through. Honestly, it was mostly removing super nested loops/removing dead code/improving consistency between the two classes.
Improvements to Linearizer class. Only a couple lines changed here. Again, just some small speed improvements.

Docs

+ Added: linearize.rst file. This contains the walkthrough of the linearization functionality.
+ Added: examples/lin_pend_nonmin.rst file. This contains a more in-depth linearization example.
Updated: KanesMethod docs to express the small changes in interface. This includes examples as well.
Updated: LagrangesMethod docs to express the small changes in interface. This includes examples as well.
Reorganized the doc folder. The docs were clearly written with kane in mind, not lagrange. Reorganized to make lagrange stuff feel less of a second-class method.
Small name updates: consistent file titles/naming structure.

srjoglekar246 · 2014-07-28T07:36:49Z

sympy/physics/mechanics/functions.py

-    """
-    Computes A^-1 * B symbolically w/ substitution, where B is not
-    necessarily a vector, but can be a matrix.
+def find_dynamicsymbols(expression, exclude=None):


I like this method! Do you think it could belong in sympy.physics.vector ? Mainly because that module also deals with dynamicsymbols anyways.

It could, as we're pulling from vector already. Would avoid chance of circular imports. How about we get this merged, then you can move it to where you'd want it/update your code to use it? I wouldn't know what to do with it inside vector.

- Fixed bug in _fraction_decomp, wasn't finding fractions with power denominators. - Changed literal set to set([*]) form used in Python2.6

moorepants · 2014-07-29T16:38:05Z

Something went crazy with the sphinx doc build on travis. See if it works locally.

moorepants · 2014-07-29T16:50:00Z

This looks good. Really nice documentation!

Question: after going through KanesMethod and LagrangesMethod, do you have any idea how much could be pushed to a Method super class for both? Would that make sense? @chrisdembia and I are working on a System class that we'd like both Methods's classes to play well with, but right now there are some differences.

chrisdembia · 2014-07-29T17:01:26Z

See pydy/pydy#81

jcrist · 2014-07-29T17:16:44Z

I'm not really sure what went on with that build. Html docs built fine, both on travis and on my computer. Haven't tried the latex stuff, but it doesn't look like it's erroring from something I did. I'll look into it.

As for a base class for all methods, they don't really share much that makes sense to have as a class-method. The only consistent method names are rhs mass_matrix, forcing, mass_matrix_full, forcing_full, linearize and to_linearizer. These all would have to be implemented on a class basis, so in the base class they'd just be NotImplementedError() in the body. This approach is find if you just want an easy way to determine if an object is a *Method class. Everything else is method specific (including the initializer).

I've been thinking about our type system in mechanics, and have some thoughts that would make more sense in hindsight but would be a pain to change due to deprecation. The main one is that KanesMethod and LagrangesMethod are basically factories for the EOM of a system. Further, they each seem to support rederivation using different forces/coordinates/etc..., which then mutates their respected attributes (mass and forcing matrices).

What would make more sense to me is to have a system class that holds the generalized system form (call it EOM or something). This would just store the results, not the method that derived them, and would implement the mass and forcing matrix stuff. With it in this form as a storage method, to_linearizer and linearize would be simpler too. And I imagine the visualization stuff as well. KanesMethod and LagrangesMethod would then act more as functions that return a derived system. Just some thoughts.

chrisdembia · 2014-07-29T17:20:39Z

@jcrist I'm going to paste your comments over at our PR. Feel free to help us with the System class, or at least with designing it.

tarzzz · 2014-07-29T18:06:45Z

doc/src/modules/physics/mechanics/advanced.rst

-3D animations; this would require a "reference orientation" for a camera as
-well as a "reference point" for distance to the camera. Development of this
-could also be tied into code output.
-
 Code Output
 -----------
 A function for generating code output for numerical integration is the highest


Good documentation man!! 👍
You have explained usage, as well as issues very well!

moorepants · 2014-08-06T16:02:47Z

@jcrist We need to make sure that latex build time isn't an issue before we merge this.

moorepants · 2014-08-06T16:03:05Z

You may need to merge in master or rebase here too.

moorepants · 2014-08-06T16:13:39Z

Make sure the linearization examples show up in the mechanics table of contents.

moorepants · 2014-08-06T16:15:30Z

The latex docs aren't building correctly for me. Here is some errors/warnings from the new stuff:

LaTeX Warning: Hyper reference `modules/physics/mechanics/reference:blajer1994'
 on page 1645 undefined on input line 131661.


LaTeX Warning: Reference `modules/physics/mechanics/reference:blajer1994' on pa
ge 1645 undefined on input line 131661.


LaTeX Warning: File `modules/physics/mechanics/examples/pendulum_nonmin.*' not 
found on input line 131712.

! Unable to load picture or PDF file 'modules/physics/mechanics/examples/pendul
um_nonmin.*'.
<to be read again> 
                   }
l.131712 .../mechanics/examples/pendulum_nonmin.*}
                                                  \hfill}
?

moorepants · 2014-08-06T16:17:23Z

Here is the full latex error output on my machine: https://gist.github.com/moorepants/69cde73989dc3a9adaf2

moorepants · 2014-08-06T16:21:07Z

doc/src/modules/physics/mechanics/examples/lin_pend_nonmin_example.rst

+describe the system using the `x` and `y` coordinates of the mass results in
+a need for constraints. The system is shown below:
+
+.. image:: pendulum_nonmin.*


I think these * are causing latex build errors.

It was actually something in the makefile for the latex build. There's a special thing with a list of *.svg files to be converted to pdf. These weren't in it. Fixed, everything runs fine now.

I just pulled the latest from this branch and the build failed. Maybe I missed some new commits?

Oh, I haven't pushed the changes yet. Was making sure there weren't more doc fixes to make so I didn't have lots of little commits. I'll push it in now.

moorepants · 2014-08-06T16:27:37Z

@jcrist This is excellent work! I'd really like to see the bicycle example run though, as that give me confidence that nothing critical has broken. Any status updates on that?

moorepants · 2014-08-06T16:38:17Z

@oliverlee @hazelnusse Jim needs a review here.

Needed to add new *.svg files to list in makefile so the pdf build won't fail.

moorepants · 2014-08-07T19:02:50Z

@jcrist I'm fine with merging this. The code functionality looks good. The docs look good and any improvements on them can be done in a new PR.

It looks like that the automerge won't work, so update with the master branch and did you stop those three builds on purpose? Travis didn't pass on slow or fast cache.

The only other point, is about the bicycle example. I'm guessing that is still up in the air?

I'd feel most comfortable if we have that example working for the next sympy release.

jcrist · 2014-08-07T20:29:04Z

@moorepants : The stuff that failed looks like it was a Travis issue (process died, or something). Had nothing to do with anything I touched, and the errors didn't look sympy related.

I haven't actually been working on fixing the bicycle example lately, been too preoccupied with codegen stuff (it's looking good so far). I'll take some time later to try some ideas, and let them run over night. As I've said before, any regression to the older methods results in a significant slowdown on my computer. Can I still merge this though?

moorepants · 2014-08-07T21:18:36Z

Let's merge, but the merge button isn't green, so you need to update with master I think.

So you have push rights now. All merges have to be through github's interface and it is a good idea to set you git config for this repo to the git:// so that you can't push to the main repo by accident. Each merge requires at least another +1 from a active dev and all tests passing on travis.

Mechanics improvements

jcrist added 11 commits July 3, 2014 22:58

Revert to using msubs in KM.to_linearizer

46094b0

It's slightly slower to partition with msubs than using KM._f_d and KM._k_d, but results in a significantly smaller expression size - yielding overall gains.

Bugfix in msubs

01a9c28

Added missing call to _recurser in _smart_subs.

Continuation of KanesMethod Refactor

7b68da3

- Cleaned up code to be more readable, no major changes in algorithms. - Added option to msubs to accept several dicts as *args.

Continuing work on KanesMethod refactoring

e39c3da

- Pulled out _find_dynamicsymbols into a separate function - Deleted unused _othersymbols function - Refactored out _partial_derivatives class method - Some code cleanup - Added tests for find_dynamicsymbols and msubs

Added docs for new linearization methods

cf78380

Added doc page for linearization methods in physics/mechanics.

Added api for linearize

5178c31

Merge branch 'master' into MechanicsImprovements

ae3014b

Conflicts: sympy/physics/mechanics/functions.py sympy/physics/mechanics/kane.py sympy/physics/mechanics/lagrange.py

Refactor of LagrangesMethod

245f733

Code cleanup of the LagrangesMethod class - Removed dead code - Refactored to be more efficient in places - Moved a common inner loop between KanesMethod and LagrangesMethod to the functions file. Function is _f_list_parser, and is internal only

jcrist added 3 commits July 26, 2014 13:10

Merge branch 'new_docs' into MechanicsDocumentation

c6b671f

Fixes to msubs to make it work in Python2

769c08b

Also fixed docstrings to pass doctests.

Documentation updates for new functionality

c574a7b

Added documentation for linearization methods, and all assorted changes that occurred along with them.

tarzzz reviewed Jul 27, 2014
View reviewed changes

Finishing touches on mechanics doc updates

632de1b

- Updates to docstrings to render well in sphinx docs - Added prose walkthrough of example sections - Spelling and grammar fixes

srjoglekar246 reviewed Jul 28, 2014
View reviewed changes

Bugfix in msubs & Python2.6 compat issue

0edab7c

- Fixed bug in _fraction_decomp, wasn't finding fractions with power denominators. - Changed literal set to set([*]) form used in Python2.6

jcrist mentioned this pull request Jul 28, 2014

Create Sphinx docs for mechanics Linearizer usage. #7755

Closed

chrisdembia mentioned this pull request Jul 29, 2014

Prototype System class. pydy/pydy#81

Merged

15 tasks

tarzzz reviewed Jul 29, 2014
View reviewed changes

moorepants reviewed Aug 6, 2014
View reviewed changes

Fix to doc makefile, add examples to index

a74fdf7

Needed to add new *.svg files to list in makefile so the pdf build won't fail.

jcrist added a commit that referenced this pull request Aug 7, 2014

Merge pull request #7790 from jcrist/MechanicsImprovements

8c08034

Mechanics improvements

jcrist merged commit 8c08034 into sympy:master Aug 7, 2014

chrisdembia mentioned this pull request Aug 7, 2014

Create Method abstract base class, with a UserDefinedMethod class. pydy/pydy#83

Open

angadsinghsandhu mentioned this pull request Apr 6, 2021

remove the new_method kwarg and deprecation warning. #21251

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Mechanics improvements #7790

Mechanics improvements #7790

jcrist commented Jul 25, 2014

jcrist commented Jul 25, 2014

moorepants commented Jul 25, 2014

jcrist commented Jul 25, 2014

moorepants commented Jul 25, 2014

jcrist commented Jul 25, 2014

tarzzz commented Jul 27, 2014

tarzzz Jul 27, 2014

jcrist commented Jul 28, 2014

jcrist commented Jul 28, 2014

srjoglekar246 Jul 28, 2014

jcrist Jul 28, 2014

moorepants commented Jul 29, 2014

moorepants commented Jul 29, 2014

chrisdembia commented Jul 29, 2014

jcrist commented Jul 29, 2014

chrisdembia commented Jul 29, 2014

tarzzz Jul 29, 2014

moorepants commented Aug 6, 2014

moorepants commented Aug 6, 2014

moorepants commented Aug 6, 2014

moorepants commented Aug 6, 2014

moorepants commented Aug 6, 2014

moorepants Aug 6, 2014

jcrist Aug 6, 2014

moorepants Aug 6, 2014

jcrist Aug 6, 2014

moorepants commented Aug 6, 2014

moorepants commented Aug 6, 2014

moorepants commented Aug 7, 2014

jcrist commented Aug 7, 2014

moorepants commented Aug 7, 2014

Mechanics improvements #7790

Mechanics improvements #7790

Conversation

jcrist commented Jul 25, 2014

jcrist commented Jul 25, 2014

moorepants commented Jul 25, 2014

jcrist commented Jul 25, 2014

moorepants commented Jul 25, 2014

jcrist commented Jul 25, 2014

tarzzz commented Jul 27, 2014

Choose a reason for hiding this comment

jcrist commented Jul 28, 2014

jcrist commented Jul 28, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

moorepants commented Jul 29, 2014

moorepants commented Jul 29, 2014

chrisdembia commented Jul 29, 2014

jcrist commented Jul 29, 2014

chrisdembia commented Jul 29, 2014

Choose a reason for hiding this comment

moorepants commented Aug 6, 2014

moorepants commented Aug 6, 2014

moorepants commented Aug 6, 2014

moorepants commented Aug 6, 2014

moorepants commented Aug 6, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

moorepants commented Aug 6, 2014

moorepants commented Aug 6, 2014

moorepants commented Aug 7, 2014

jcrist commented Aug 7, 2014

moorepants commented Aug 7, 2014