From d4119a916ad4db703221add01454f5c08b286ba2 Mon Sep 17 00:00:00 2001 From: sidhu1012 Date: Tue, 22 Sep 2020 18:00:14 +0530 Subject: [PATCH 1/4] warning for cyclic paths and other possible paths along with test cases --- sympy/physics/vector/point.py | 22 +++++++++++-- sympy/physics/vector/tests/test_point.py | 41 ++++++++++++++++++++++-- 2 files changed, 58 insertions(+), 5 deletions(-) diff --git a/sympy/physics/vector/point.py b/sympy/physics/vector/point.py index e49a978839ac..7097d9690bed 100644 --- a/sympy/physics/vector/point.py +++ b/sympy/physics/vector/point.py @@ -1,6 +1,7 @@ from __future__ import print_function, division from .vector import Vector, _check_vector from .frame import _check_frame +from warnings import warn __all__ = ['Point'] @@ -529,24 +530,41 @@ def vel(self, frame): _check_frame(frame) if not (frame in self._vel_dict): + valid_neighbor_found = False + is_cyclic = False visited = [] queue = [self] + candidate_neighbor = [] while queue: #BFS to find nearest point node = queue.pop(0) if node not in visited: visited.append(node) for neighbor, neighbor_pos in node._pos_dict.items(): + if neighbor in visited: + continue try: neighbor_pos.express(frame) #Checks if pos vector is valid except ValueError: continue + if neighbor in queue: + is_cyclic = True try : neighbor_velocity = neighbor._vel_dict[frame] #Checks if point has its vel defined in req frame except KeyError: queue.append(neighbor) continue - self.set_vel(frame, self.pos_from(neighbor).dt(frame) + neighbor_velocity) - return self._vel_dict[frame] + candidate_neighbor.append(neighbor) + if not valid_neighbor_found: + self.set_vel(frame, self.pos_from(neighbor).dt(frame) + neighbor_velocity) + valid_neighbor_found = True + if is_cyclic: + warn('Cyclic branches used. Branches should be acyclic.') + if len(candidate_neighbor) > 1: + warn('Velocity automatically calculated based on point ' + + candidate_neighbor[0].name + ' but it is also possible from ' + + str(candidate_neighbor[1:]) + '. Velocities from these paths are not the same.') + if valid_neighbor_found: + return self._vel_dict[frame] else: raise ValueError('Velocity of point ' + self.name + ' has not been' ' defined in ReferenceFrame ' + frame.name) diff --git a/sympy/physics/vector/tests/test_point.py b/sympy/physics/vector/tests/test_point.py index 617d0bddd8c1..e5095421dd52 100644 --- a/sympy/physics/vector/tests/test_point.py +++ b/sympy/physics/vector/tests/test_point.py @@ -1,6 +1,6 @@ from sympy.physics.vector import dynamicsymbols, Point, ReferenceFrame -from sympy.testing.pytest import raises - +from sympy.testing.pytest import raises, ignore_warnings +import warnings def test_point_v1pt_theorys(): q, q2 = dynamicsymbols('q q2') @@ -216,7 +216,10 @@ def test_auto_point_vel_shortest_path(): O1 = Point('O1') O1.set_pos(O, q2 * B.z) P4.set_pos(O1, q1 * B.x + q2 * B.z) - assert P4.vel(B) == q1.diff(t) * B.x + u2 * B.y + 2 * q2.diff(t) * B.z + with warnings.catch_warnings(): + warnings.simplefilter('error') + with ignore_warnings(UserWarning): + assert P4.vel(B) == q1.diff(t) * B.x + u2 * B.y + 2 * q2.diff(t) * B.z def test_auto_point_vel_connected_frames(): t = dynamicsymbols._t @@ -230,3 +233,35 @@ def test_auto_point_vel_connected_frames(): raises(ValueError, lambda: P.vel(N)) N.orient(B, 'Axis', (q, B.x)) assert P.vel(N) == (u + q1.diff(t)) * N.x + q2.diff(t) * B.y - q2 * q.diff(t) * B.z + +def test_auto_point_vel_multiple_short_paths(): + q, u = dynamicsymbols('q u') + N = ReferenceFrame('N') + O = Point('O') + P = Point('P') + Q = Point('Q') + R = Point('R') + P.set_vel(N, u * N.x) + Q.set_vel(N, u *N.y) + R.set_vel(N, u * N.z) + O.set_pos(P, q * N.z) + O.set_pos(Q, q * N.y) + O.set_pos(R, q * N.x) + with warnings.catch_warnings(): + warnings.simplefilter("error") + raises(UserWarning ,lambda: O.vel(N)) + +def test_auto_vel_cyclic_warning(): + P = Point('P') + P1 = Point('P1') + P2 = Point('P2') + P3 = Point('P3') + N = ReferenceFrame('N') + P.set_vel(N, N.x) + P1.set_pos(P, N.x) + P2.set_pos(P1, N.y) + P3.set_pos(P2, N.z) + P1.set_pos(P3, N.x + N.y) + with warnings.catch_warnings(): + warnings.simplefilter("error") + raises(UserWarning ,lambda: P2.vel(N)) From 5d3597b1d0c56a6e2cbe9f113c8a6785859215da Mon Sep 17 00:00:00 2001 From: sidhu1012 Date: Thu, 24 Sep 2020 12:50:45 +0530 Subject: [PATCH 2/4] Updated warning message, added new test cases for checking msg --- sympy/physics/vector/point.py | 4 +-- sympy/physics/vector/tests/test_point.py | 37 ++++++++++++++++++++++-- 2 files changed, 37 insertions(+), 4 deletions(-) diff --git a/sympy/physics/vector/point.py b/sympy/physics/vector/point.py index 7097d9690bed..a93ee7e2f57b 100644 --- a/sympy/physics/vector/point.py +++ b/sympy/physics/vector/point.py @@ -558,11 +558,11 @@ def vel(self, frame): self.set_vel(frame, self.pos_from(neighbor).dt(frame) + neighbor_velocity) valid_neighbor_found = True if is_cyclic: - warn('Cyclic branches used. Branches should be acyclic.') + warn('Multiple points have their position defined with respect to one point.') if len(candidate_neighbor) > 1: warn('Velocity automatically calculated based on point ' + candidate_neighbor[0].name + ' but it is also possible from ' + - str(candidate_neighbor[1:]) + '. Velocities from these paths are not the same.') + str(candidate_neighbor[1:]) + '. Velocities from these points are not the same.') if valid_neighbor_found: return self._vel_dict[frame] else: diff --git a/sympy/physics/vector/tests/test_point.py b/sympy/physics/vector/tests/test_point.py index e5095421dd52..b2cf49248df7 100644 --- a/sympy/physics/vector/tests/test_point.py +++ b/sympy/physics/vector/tests/test_point.py @@ -234,7 +234,7 @@ def test_auto_point_vel_connected_frames(): N.orient(B, 'Axis', (q, B.x)) assert P.vel(N) == (u + q1.diff(t)) * N.x + q2.diff(t) * B.y - q2 * q.diff(t) * B.z -def test_auto_point_vel_multiple_short_paths(): +def test_auto_point_vel_multiple_paths_warning_arises(): q, u = dynamicsymbols('q u') N = ReferenceFrame('N') O = Point('O') @@ -251,7 +251,7 @@ def test_auto_point_vel_multiple_short_paths(): warnings.simplefilter("error") raises(UserWarning ,lambda: O.vel(N)) -def test_auto_vel_cyclic_warning(): +def test_auto_vel_cyclic_warning_arises(): P = Point('P') P1 = Point('P1') P2 = Point('P2') @@ -265,3 +265,36 @@ def test_auto_vel_cyclic_warning(): with warnings.catch_warnings(): warnings.simplefilter("error") raises(UserWarning ,lambda: P2.vel(N)) + +def test_auto_vel_cyclic_warning_msg(): + P = Point('P') + P1 = Point('P1') + P2 = Point('P2') + P3 = Point('P3') + N = ReferenceFrame('N') + P.set_vel(N, N.x) + P1.set_pos(P, N.x) + P2.set_pos(P1, N.y) + P3.set_pos(P2, N.z) + P1.set_pos(P3, N.x + N.y) + with warnings.catch_warnings(record = True) as w: + warnings.simplefilter("always") + P2.vel(N) + assert issubclass(w[-1].category, UserWarning) + assert 'Multiple points have their position defined with respect to one point.' in str(w[-1].message) + +def test_auto_vel_multiple_path_warning_msg(): + N = ReferenceFrame('N') + O = Point('O') + P = Point('P') + Q = Point('Q') + P.set_vel(N, N.x) + Q.set_vel(N, N.y) + O.set_pos(P, N.z) + O.set_pos(Q, N.y) + with warnings.catch_warnings(record = True) as w: + warnings.simplefilter("always") + O.vel(N) + assert issubclass(w[-1].category, UserWarning) + assert 'Velocity automatically calculated based on point' in str(w[-1].message) + assert 'Velocities from these points are not the same.' in str(w[-1].message) From 01b91dc341cc75e1c8a65eb607f81345aa2b8f49 Mon Sep 17 00:00:00 2001 From: sidhu1012 Date: Wed, 30 Sep 2020 16:04:58 +0530 Subject: [PATCH 3/4] Updated warning msg as per suggestion --- sympy/physics/vector/point.py | 6 +++--- sympy/physics/vector/tests/test_point.py | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/sympy/physics/vector/point.py b/sympy/physics/vector/point.py index a93ee7e2f57b..9113ee1b0817 100644 --- a/sympy/physics/vector/point.py +++ b/sympy/physics/vector/point.py @@ -558,11 +558,11 @@ def vel(self, frame): self.set_vel(frame, self.pos_from(neighbor).dt(frame) + neighbor_velocity) valid_neighbor_found = True if is_cyclic: - warn('Multiple points have their position defined with respect to one point.') + warn('Kinematic loops are defined among the positions of points. This is likely not desired and may cause errors in your calculations.') if len(candidate_neighbor) > 1: warn('Velocity automatically calculated based on point ' + - candidate_neighbor[0].name + ' but it is also possible from ' + - str(candidate_neighbor[1:]) + '. Velocities from these points are not the same.') + candidate_neighbor[0].name + ' but it is also possible from points(s):' + + str(candidate_neighbor[1:]) + '. Velocities from these points are not necessarily the same. This may cause errors in your calculations.') if valid_neighbor_found: return self._vel_dict[frame] else: diff --git a/sympy/physics/vector/tests/test_point.py b/sympy/physics/vector/tests/test_point.py index b2cf49248df7..88b683122ec7 100644 --- a/sympy/physics/vector/tests/test_point.py +++ b/sympy/physics/vector/tests/test_point.py @@ -216,7 +216,7 @@ def test_auto_point_vel_shortest_path(): O1 = Point('O1') O1.set_pos(O, q2 * B.z) P4.set_pos(O1, q1 * B.x + q2 * B.z) - with warnings.catch_warnings(): + with warnings.catch_warnings(): #Multipath warning warnings.simplefilter('error') with ignore_warnings(UserWarning): assert P4.vel(B) == q1.diff(t) * B.x + u2 * B.y + 2 * q2.diff(t) * B.z @@ -281,7 +281,7 @@ def test_auto_vel_cyclic_warning_msg(): warnings.simplefilter("always") P2.vel(N) assert issubclass(w[-1].category, UserWarning) - assert 'Multiple points have their position defined with respect to one point.' in str(w[-1].message) + assert 'Kinematic loops are defined among the positions of points. This is likely not desired and may cause errors in your calculations.' in str(w[-1].message) def test_auto_vel_multiple_path_warning_msg(): N = ReferenceFrame('N') @@ -297,4 +297,4 @@ def test_auto_vel_multiple_path_warning_msg(): O.vel(N) assert issubclass(w[-1].category, UserWarning) assert 'Velocity automatically calculated based on point' in str(w[-1].message) - assert 'Velocities from these points are not the same.' in str(w[-1].message) + assert 'Velocities from these points are not necessarily the same. This may cause errors in your calculations.' in str(w[-1].message) From 8a61a15a21fdd946551b694e5e04b9438f99040a Mon Sep 17 00:00:00 2001 From: sidhu1012 Date: Wed, 30 Sep 2020 17:30:08 +0530 Subject: [PATCH 4/4] Added comments for warnings raised --- sympy/physics/vector/tests/test_point.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/sympy/physics/vector/tests/test_point.py b/sympy/physics/vector/tests/test_point.py index 88b683122ec7..ce53bec4bd36 100644 --- a/sympy/physics/vector/tests/test_point.py +++ b/sympy/physics/vector/tests/test_point.py @@ -216,7 +216,7 @@ def test_auto_point_vel_shortest_path(): O1 = Point('O1') O1.set_pos(O, q2 * B.z) P4.set_pos(O1, q1 * B.x + q2 * B.z) - with warnings.catch_warnings(): #Multipath warning + with warnings.catch_warnings(): #There are two possible paths in this point tree, thus a warning is raised warnings.simplefilter('error') with ignore_warnings(UserWarning): assert P4.vel(B) == q1.diff(t) * B.x + u2 * B.y + 2 * q2.diff(t) * B.z @@ -247,7 +247,7 @@ def test_auto_point_vel_multiple_paths_warning_arises(): O.set_pos(P, q * N.z) O.set_pos(Q, q * N.y) O.set_pos(R, q * N.x) - with warnings.catch_warnings(): + with warnings.catch_warnings(): #There are two possible paths in this point tree, thus a warning is raised warnings.simplefilter("error") raises(UserWarning ,lambda: O.vel(N)) @@ -262,7 +262,7 @@ def test_auto_vel_cyclic_warning_arises(): P2.set_pos(P1, N.y) P3.set_pos(P2, N.z) P1.set_pos(P3, N.x + N.y) - with warnings.catch_warnings(): + with warnings.catch_warnings(): #The path is cyclic at P1, thus a warning is raised warnings.simplefilter("error") raises(UserWarning ,lambda: P2.vel(N)) @@ -277,7 +277,7 @@ def test_auto_vel_cyclic_warning_msg(): P2.set_pos(P1, N.y) P3.set_pos(P2, N.z) P1.set_pos(P3, N.x + N.y) - with warnings.catch_warnings(record = True) as w: + with warnings.catch_warnings(record = True) as w: #The path is cyclic at P1, thus a warning is raised warnings.simplefilter("always") P2.vel(N) assert issubclass(w[-1].category, UserWarning) @@ -292,7 +292,7 @@ def test_auto_vel_multiple_path_warning_msg(): Q.set_vel(N, N.y) O.set_pos(P, N.z) O.set_pos(Q, N.y) - with warnings.catch_warnings(record = True) as w: + with warnings.catch_warnings(record = True) as w: #There are two possible paths in this point tree, thus a warning is raised warnings.simplefilter("always") O.vel(N) assert issubclass(w[-1].category, UserWarning)