Add zero velocity constraint in v1pt and v2pt

[tjstienstra]

References to other Issues or PRs

Fixes the problem presented in pull request 12322.

Brief description of what is fixed or changed

Add a check if the velocity of the otherpoint in the velocity theorems is indeed set to 0

Other comments

Possibly also add a check for the acceleration theorems (i.e. a1pt_theory and a2pt_theory).

Release Notes

• physics.vector
  • Add a zero velocity requirement to v1pt_theory and v2pt_theory

[sympy-bot]

✅

Hi, I am the SymPy bot (v167). 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.vector
  • Add a zero velocity requirement to v1pt_theory and v2pt_theory (#23359 by @TJStienstra)

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

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. -->
Fixes the problem presented in pull request [12322](https://github.com/sympy/sympy/pull/12322).

#### Brief description of what is fixed or changed
Add a check if the velocity of the `otherpoint` in the velocity theorems is indeed set to `0`

#### Other comments
Possibly also add a check for the acceleration theorems (i.e. `a1pt_theory` and `a2pt_theory`).

#### 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.

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.vector
  * Add a zero velocity requirement to `v1pt_theory` and `v2pt_theory`
<!-- END RELEASE NOTES -->

[moorepants]

Seeing that we have to add this now, I'm not sure this is the solution we want. I thought that we only want to through an error if the velocity of the fixed point is such that it has a value and that value indicates it isn't fixed. So we should allow the velocity to 1) be undefined or 2) fixed in the shared frame. Having to set this velocity to zero is just going to be annoying for users.

[moorepants]

Given my comment above, this should be something like "if otherpoint velocity is defined and it isn't zero in the shared frame, then ...."

[tjstienstra]

Yes, I agree. We could at most display a warning. The best option I see is to use try except to catch the ValueError, when the velocity is not defined. This will result in something like:

other_point_is_valid = True
try:
    if not otherpoint.vel(interframe)==0:
        otherpoint_is_valid = False
except ValueError:
    pass
if not other_point_is_valid:
    raise ValueError(...)

Advantage is that this is a extensive test, which really checks whether the velocity is defined. Disadvantages: it uses try except and will possibly add {interframe: 0} to the otherpoint._vel_dict' by calling otherpoint.vel` (not sure if this is a disadvantage).

Shall I implement this solution? Or is there a better one? Also should I revert my previous commit, so the test files and model.py will not be changed?

[moorepants]

It is fine to do explicit checks over the try/except clause. Maybe preferred in this case.

We try to avoid warnings in SymPy, but we already have some for this particular piece of code. I think it is fine to throw an error if the velocity is defined and it is not constant. If it is not defined, the behavior should be as it is, don't do anything.

[tjstienstra]

Should I also undo my previous changes to the other files?

[moorepants]

Yes, unfortunately. I don't think we want to add this requirement that you have to set the velocity of the base point in the shared frame. It is really unnecessary. The alternative is, as @angadhn suggested, to set that velocity automatically for the user. It's probably ok to do that. So, if the velocity is undefined, set it to 0 and if it is defined and not zero, then throw an error. The only thing I'm hung up on that is that we don't set more than one velocity with any other methods. But I may be warmed up to the idea by now. If it throws an error if it is not constant and sets it to zero otherwise, I can't see a major issue, but then we wouldn't return that second velocity. But that method really shouldn't return a velocity, it should just set them. If you set the velocity to zero and make that very clear in the documentation, then that's probably fine.

[tjstienstra]

I will just revert my changes and implement the suggestion. For this one the velocity in that frame will only be set to 0 if it is already known that it is 0.

[tjstienstra]

Just cleaned up a bit and changed the implementation as well. However I found out that there is one big problem, namely that it will fail on cases where the velocity term contains a kde. This happens in the last example here, which can be replicated with the following code:

from sympy.physics.vector import dynamicsymbols, Point, ReferenceFrame
from sympy import Symbol, cos, sin

N = ReferenceFrame('N')
u1, u2 = dynamicsymbols('u1 u2')
q1, q2 = dynamicsymbols('q1 q2')
l = Symbol('l')
R = Symbol('R')
C = N.orientnew('C', 'Axis', [q1, N.x])
C.set_ang_vel(N, u1 * N.x)
O = Point('O')
O.set_vel(N, 0)
Q = O.locatenew('Q', -l * C.z)
P = Q.locatenew('P', R * (cos(q2) * C.x + sin(q2) * C.y))
P.set_vel(C, R * u2 * (-sin(q2) * C.x + cos(q2) * C.y))
print(O.vel(C))
Q.v2pt_theory(O, N, C)

An option would be to check that there are no derivative terms in the velocity, but personally I do not really like that solution.

[tjstienstra]

An option would be to check that there are no derivative terms in the velocity

Another problem could be given if someone manages to define something double with different variable.
Another options would be replacing it with a warning.

[tjstienstra]

@moorepants or @sidhu1012 what shall we do with this PR:

• Just let it be, not adding a zero velocity constraint, only edit the small bugs in the tests that were found
• Check that there are no derivative terms in the checks: self.vel(frame) == 0 and self.acc(frame) == 0
• Replace raise ValueError with a warning

[moorepants]

Hi @tjstienstra is there a reason to close this? It seems to have been forgotten, but looks like there are some aspects that may be desired. Or maybe its been addressed in other work?

[moorepants]

Was this an error in the example?

[tjstienstra]

Yes, the next line sets the velocity as P.set_vel(B, qd * B.x + q2d * B.y)