sympy icon indicating copy to clipboard operation
sympy copied to clipboard

change code so KanesMethod will find nonlinearities in speed constraints

Open Peter230655 opened this issue 1 year ago • 1 comments

References to other Issues or PRs

Brief description of what is fixed or changed

Other comments

Release Notes

  • physics.mechanics
    • KanesMethod allows for linear speed constraints, however it does not check whether they really are linear. It does test the kinematic differential equations for linearity. So, I simply copied the few lines of code there and added them around line 320

Peter230655 avatar Aug 02 '24 12:08 Peter230655

:white_check_mark:

Hi, I am the SymPy bot. I'm here to help you write a release notes entry. Please read the guide on how to write release notes.

Your release notes are in good order.

Here is what the release notes will look like:

  • physics.mechanics
    • KanesMethod requires linear speed constraints, but it does not check whether they indeed are linear. Using linear_eq_to_matrix instead of the Jacobian serves two purposes: 1) it makes solving the constraints faster and 2) it will raise an error in the speed constraints are not linear. (#26900 by @Peter230655)

This will be added to https://github.com/sympy/sympy/wiki/Release-Notes-for-1.14.

Click here to see the pull request description that was parsed.
<!-- Your title above should be a short description of what
was changed. Do not include the issue number in the title. -->

#### References to other Issues or PRs
<!-- If this pull request fixes an issue, write "Fixes #NNNN" in that exact
format, e.g. "Fixes #1234" (see
https://tinyurl.com/auto-closing for more information). Also, please
write a comment on that issue linking back to this pull request once it is
open. -->


#### Brief description of what is fixed or changed


#### Other comments


#### Release Notes

<!-- Write the release notes for this release below between the BEGIN and END
statements. The basic format is a bulleted list with the name of the subpackage
and the release note for this PR. For example:

* solvers
  * Added a new solver for logarithmic equations.

* functions
  * Fixed a bug with log of integers. Formerly, `log(-x)` incorrectly gave `-log(x)`.

* physics.units
  * Corrected a semantical error in the conversion between volt and statvolt which
    reported the volt as being larger than the statvolt.

or if no release note(s) should be included use:

NO ENTRY

See https://github.com/sympy/sympy/wiki/Writing-Release-Notes for more
information on how to write release notes. The bot will check your release
notes automatically to see if they are formatted correctly. -->

<!-- BEGIN RELEASE NOTES -->
* physics.mechanics
  * KanesMethod requires linear speed constraints, but it does not check whether they indeed are linear. Using _linear_eq_to_matrix_ instead of the Jacobian serves two purposes: 1) it makes solving the constraints faster and 2) it will raise an error in the speed constraints are not linear.

<!-- END RELEASE NOTES -->

Update

The release notes on the wiki have been updated.

sympy-bot avatar Aug 02 '24 12:08 sympy-bot

Benchmark results from GitHub Actions

Lower numbers are good, higher numbers are bad. A ratio less than 1 means a speed up and greater than 1 means a slowdown. Green lines beginning with + are slowdowns (the PR is slower then master or master is slower than the previous release). Red lines beginning with - are speedups.

Significantly changed benchmark results (PR vs master)


Significantly changed benchmark results (master vs previous release)


Full benchmark results can be found as artifacts in GitHub Actions (click on checks at the top of the PR).

github-actions[bot] avatar Sep 08 '24 18:09 github-actions[bot]

A test that catches this error should be added.

smichr avatar Sep 14 '24 16:09 smichr

I checked the speeds with my 40 link closed pendulum, as suggested: ( since the code for checking the linearity of the kinematic DEs is part of KanesMethod, and since I simply copied this code, I was hoping the time consumed would be similar - which it was in this example)

  • checking for linearity of the kinematic DEs: 0.0 sec
  • checking for linearity of the velocity constraints: 0.0sec to 0.01sec

It takes about 35 sec to set up KanesEquations in this example. I tested like this:

zeit = time.time()
code to be checked
......
......
duration = time.time() - zeit

Peter230655 avatar Sep 29 '24 13:09 Peter230655

A test that catches this error should be added.

You added the label PR: author's turn. I am unclear what changes I should submit. Thanks!

Peter230655 avatar Sep 29 '24 13:09 Peter230655

Doesn't the 40 link pendulum not have any speed constraints? What are you checking in that case?

moorepants avatar Sep 30 '24 08:09 moorepants

I used the version where the other end is also attached to a point. Sorry, should hsve mentioned it.

Peter230655 avatar Sep 30 '24 09:09 Peter230655

I am unclear what changes I should submit.

@smichr wrote "A test that catches this error should be added."

moorepants avatar Sep 30 '24 10:09 moorepants

I am unclear what changes I should submit.

@smichr wrote "A test that catches this error should be added."

I did make a test, it is PR # 26901, maybe this is why I did not understand what was meant. Is this o.k.?

Peter230655 avatar Sep 30 '24 11:09 Peter230655

The test needs to be in the same PR as the change, i.e. this PR.

moorepants avatar Sep 30 '24 12:09 moorepants

🟠

Hi, I am the SymPy bot. I've noticed that some of your commits add or delete files. Since this is sometimes done unintentionally, I wanted to alert you about it.

This is an experimental feature of SymPy Bot. If you have any feedback on it, please comment at https://github.com/sympy/sympy-bot/issues/75.

The following commits add new files:

  • e2f3773b7be23a97703713602ece473a253e8ca6:
    • sympy/physics/mechanics/tests/test_linearity_of_velocity_constraints.py

If these files were added/deleted on purpose, you can ignore this message.

sympy-bot avatar Sep 30 '24 13:09 sympy-bot

I added the test for nonlinearity of velocity constraints to this PR, I will close the other PR, which only held the test

Peter230655 avatar Sep 30 '24 14:09 Peter230655

I checked the time needed to check the velocity constrains for nonlinearity again, using an old simulation of mine. Basically the skateboard of your lecture, modified a bit. It has 6 velocity constraints. Checking for nonlinearity took between 0.0 sec and 0.002 sec, total time to get kanes eoms took 1.3 sec. I used the same method mentioned abvoe to check for the time:

zeit = time.time()
code to check for nonlinearity
.....
....
time_needed = time.tim() - zeit

I also made the velocity constraints nonlinear, and as I hoped, it caught it.

Peter230655 avatar Sep 30 '24 15:09 Peter230655

I managed the linear_eq_to_matrix part. But it seems, that the method must be change at other places. On the other had, it seems to cut the time needed by around 50% from 0.36 sec to 0.17sec. Worth pursuing? If yes, I guess this would be a new PR, as the idea has changed: speed up calculating the velocity constraints.

Peter230655 avatar Oct 01 '24 05:10 Peter230655

It doesn't need a new pr, but you can change the title to reflect what it has become.

moorepants avatar Oct 01 '24 05:10 moorepants

It doesn't need a new pr, but you can change the title to reflect what it has become.

I will do so, if I get it going. :-)

Peter230655 avatar Oct 01 '24 06:10 Peter230655

It seems to work, and definitely 50% to 80% faster, but I am running in this problem: linear_eq_to_matrix raises a PolyNonlinearError and a NonlinearError, but neither one seems 'defined': When I write

try:
    code
    ......
except NonlinearError as e:
    raise NonlineaeError('message')

I get the message NonlinearError is not defined or similar. Same with

with pytest.raises(NonlinearError):

So, I do not know how to write the test, which does not stop if the speed constraints are nonlinear. Thanks for any help!

Peter230655 avatar Oct 01 '24 08:10 Peter230655

I get the message NonlinearError is not defined or similar.

Did you forget to import NonlinearError?

import sympy as sm
from sympy.solvers.solveset import NonlinearError

x = sm.Matrix(sm.symbols("x1:3"))
A = sm.Matrix([[1, 2], [3, 4]])
b = sm.Matrix([5, 6])
sm.linear_eq_to_matrix(A * x - b, x[:])  # Works fine

try:
    sm.linear_eq_to_matrix(A * x * x[0] - b, x[:])
except NonlinearError as e:
    msg = "Some custom message."
    print("here")
    raise NonlinearError(msg) from e

This raises the following error:

Traceback (most recent call last):
  File "...\sympy\solvers\solveset.py", line 2874, in linear_eq_to_matrix
    eq, c = _linear_eq_to_dict(equations, symbols)
  File "...\sympy\polys\matrices\linsolve.py", line 171, in _linear_eq_to_dict
    c, d = _lin_eq2dict(e, symset)
  File "...\sympy\polys\matrices\linsolve.py", line 199, in _lin_eq2dict
    ci, ti = _lin_eq2dict(ai, symset)
  File "...\sympy\polys\matrices\linsolve.py", line 219, in _lin_eq2dict
    raise PolyNonlinearError(filldedent('''
sympy.polys.solvers.PolyNonlinearError:
nonlinear cross-term: x1*(x1 + 2*x2)

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "...\temp.py", line 10, in <module>
    sm.linear_eq_to_matrix(A * x * x[0] - b, x[:])
  File "...\sympy\solvers\solveset.py", line 2876, in linear_eq_to_matrix
    raise NonlinearError(str(err))
sympy.solvers.solveset.NonlinearError:
nonlinear cross-term: x1*(x1 + 2*x2)

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "...\temp.py", line 14, in <module>
    raise NonlinearError(msg) from e
sympy.solvers.solveset.NonlinearError: Some custom message.

tjstienstra avatar Oct 01 '24 09:10 tjstienstra

I get the message NonlinearError is not defined or similar.

Did you forget to import NonlinearError?

import sympy as sm
from sympy.solvers.solveset import NonlinearError

x = sm.Matrix(sm.symbols("x1:3"))
A = sm.Matrix([[1, 2], [3, 4]])
b = sm.Matrix([5, 6])
sm.linear_eq_to_matrix(A * x - b, x[:])  # Works fine

try:
    sm.linear_eq_to_matrix(A * x * x[0] - b, x[:])
except NonlinearError as e:
    msg = "Some custom message."
    print("here")
    raise NonlinearError(msg) from e

This raises the following error:

Traceback (most recent call last):
  File "...\sympy\solvers\solveset.py", line 2874, in linear_eq_to_matrix
    eq, c = _linear_eq_to_dict(equations, symbols)
  File "...\sympy\polys\matrices\linsolve.py", line 171, in _linear_eq_to_dict
    c, d = _lin_eq2dict(e, symset)
  File "...\sympy\polys\matrices\linsolve.py", line 199, in _lin_eq2dict
    ci, ti = _lin_eq2dict(ai, symset)
  File "...\sympy\polys\matrices\linsolve.py", line 219, in _lin_eq2dict
    raise PolyNonlinearError(filldedent('''
sympy.polys.solvers.PolyNonlinearError:
nonlinear cross-term: x1*(x1 + 2*x2)

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "...\temp.py", line 10, in <module>
    sm.linear_eq_to_matrix(A * x * x[0] - b, x[:])
  File "...\sympy\solvers\solveset.py", line 2876, in linear_eq_to_matrix
    raise NonlinearError(str(err))
sympy.solvers.solveset.NonlinearError:
nonlinear cross-term: x1*(x1 + 2*x2)

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "...\temp.py", line 14, in <module>
    raise NonlinearError(msg) from e
sympy.solvers.solveset.NonlinearError: Some custom message.

I did not forget - I did not know! :-) Same with PolyNonlinearError? Thanks!!

Peter230655 avatar Oct 01 '24 09:10 Peter230655

Same with PolyNonlinearError?

from sympy.polys.solvers import PolyNonlinearError

I wouldn't expect that you get a PolyNonlinearError as I guess that linear_eq_to_matrix actually catches those and changes them to NonlinearErrors.

tjstienstra avatar Oct 01 '24 09:10 tjstienstra

Same with PolyNonlinearError?

from sympy.polys.solvers import PolyNonlinearError

I wouldn't expect that you get a PolyNonlinearError as I guess that linear_eq_to_matrix actually catches those and changes them to NonlinearErrors.

I seem to get both:

PolyNonlinearError: nonlinear term: ux(t)**2

During handling of the above exception, another exception occurred:

NonlinearError                            Traceback (most recent call last)
File Untitled-1:306
    [305](untitled-1:305)     u_test = list(self.u)
--> [306](untitled-1:306)     self._k_nh, self._f_nh = linear_eq_to_matrix(vel_test, u_test)
    [308](untitled-1:308) # If no acceleration constraints given, calculate them.

Peter230655 avatar Oct 01 '24 09:10 Peter230655

I seem to get both:

No, what happens is that PolyNonlinearError is catched using try except and a NonlinearError is raised, which is what you get. However, to give users more insight Python also shows the original error, i.e. the PolyNonlinearError, in the traceback.

tjstienstra avatar Oct 01 '24 09:10 tjstienstra

I seem to get both:

No, what happens is that PolyNonlinearError is catched using try except and a NonlinearError is raised, which is what you get. However, to give users more insight Python also shows the original error, i.e. the PolyNonlinearError, in the traceback.

Thanks! So should I skip the try.....except(NonlinearError) part, as you suggested originally?

Peter230655 avatar Oct 01 '24 09:10 Peter230655

So should I skip the try.....except(NonlinearError) part, as you suggested originally?

No, I would just use self._k_nh, self._f_nh = linear_eq_to_matrix(...) directly without wrapping it in a try except statement. The error raised by linear_eq_to_matrix is already quite clear.

tjstienstra avatar Oct 01 '24 09:10 tjstienstra

Finally I got it to work, even with the try.....except! Thanks al lot - I did learn something today!!

The increase in speed is leally substantial as per my inaccurate measurements!

Peter230655 avatar Oct 01 '24 10:10 Peter230655

I used Timo's idea of linear_eq_to_matrix function to speed things up considerably. Raise error if nonlinear.

Peter230655 avatar Oct 01 '24 11:10 Peter230655

I just copied Timo's version, hopefully correctly so.

Peter230655 avatar Oct 01 '24 14:10 Peter230655

I copied correctly! :-) To be very clear: That this finally worked was ONLY Timo's skill!

Peter230655 avatar Oct 01 '24 15:10 Peter230655

KanesMethod allows for linear speed constraints, however it does not check whether they really are linear. It does test the kinematic differential equations for linearity. So, I simply copied the few lines of code there and added them around line 320

needs to be improved. State what is changes: "KanesMethod will now raise a NonLinearError if nonlinear velocity constraints are passed in."

moorepants avatar Oct 01 '24 15:10 moorepants

KanesMethod allows for linear speed constraints, however it does not check whether they really are linear. It does test the kinematic differential equations for linearity. So, I simply copied the few lines of code there and added them around line 320

needs to be improved. State what is changes: "KanesMethod will now raise a NonLinearError if nonlinear velocity constraints are passed in."

I changed the description. It was NOT my idea, but Timo's. Can I mention it there?

Peter230655 avatar Oct 01 '24 15:10 Peter230655