Make physics.mechanics.pathway public.

[moorepants]

References to other Issues or PRs

Original PRs:

• Implemented PathwayBase and LinearPathway in sympy.physics.mechanics #24914
• Implemented WrappingPathway in sympy.physics.mechanics #25400

Brief description of what is fixed or changed

Make mechanic's pathway.py public and complete documentation.

Other comments

Merge #25510 first.

Release Notes

• physics.mechanics
  • Adds module for constructing force pathways to physics.mechanics. Includes a linear pathway and wrapping pathway.

[sympy-bot]

✅

Hi, I am the SymPy bot (v170). 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
  • Adds module for constructing force pathways to physics.mechanics. Includes a linear pathway and wrapping pathway. (#25511 by @brocksam and @moorepants)

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

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

Original PRs:

- https://github.com/sympy/sympy/pull/24914
- https://github.com/sympy/sympy/pull/25400

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

Make mechanic's `pathway.py` public and complete documentation.


#### Other comments

Merge https://github.com/sympy/sympy/pull/25510 first.


#### 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
  * Adds module for constructing force pathways to physics.mechanics. Includes a linear pathway and wrapping pathway.

<!-- END RELEASE NOTES -->

Update

The release notes on the wiki have been updated.

[sympy-bot]

🟠

Hi, I am the SymPy bot (v170). 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 sympy/sympy-bot#75.

The following commits add new files:

• 987d6a0:
  • doc/src/modules/physics/mechanics/api/pathway.rst

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

[moorepants]

"expansile" does not explain the sign conventions as well as it could be. We face this in the Beam package too and it has caused lots of confusion. A diagram that shows two points with the sign conventions for the velocity vectors of each point and the forces acting on each point would make this most clear, other wise there are 4 possibilities for velocity signs and 4 for forces.

[moorepants]

From LinearPathway there is:

    @property
    def extension_velocity(self) -> ExprType:
        """Exact analytical expression for the pathway's extension velocity."""
        relative_position = self.attachments[-1].pos_from(self.attachments[0])
        if not relative_position:
            return S.Zero
        t = dynamicsymbols._t  # type: ignore
        # A reference frame is needed to differentiate ``relative_position`` to
        # ``relative_velocity`` so choose the first ``ReferenceFrame`` that
        # ``relative_position`` is defined using.
        frame = relative_position.args[0][1]
        relative_velocity = relative_position.diff(t, frame)
        extension_velocity = relative_velocity.dot(relative_position.normalize())
        return extension_velocity

1. I'm not sure I see why length and extension_velocity are properties. We can't set them and the calculations that each run can be non-trivial, especially for more complex pathway classes. This makes me think they should be methods instead.

2. Selecting a frame in which one point has a velocity defined in to take the derivative strikes me as error prone. It isn't clear to me that this will guarantee to give you a correct frame needed for this calculation. Why not just differentiate the length expression with respect to time? (as you've done for the WrappingPathway). That seems less error prone and is precisely what you are after.

[moorepants]

Here is a first attempt at trying to say something about the sign convention.

As you can probably see, "extension" and "expansion" don't necessarily have any meaning unless you make the assumption that a2 is always positioned to the right of a1. But the equations present in these classes are valid even if a1 is the right of a2.

[moorepants]

I'm seeing now that your "extension_vel" means "when the two points are moving away from each other". Here's a little sign table:

    def test_2D_pathway_extension_velocity(self) -> None:
        """

        q1  q1d  extension_vel
        +   +    +
        +   -    -
        -   +    -
        -   -    +

        Distance between points is growing: + extension_vel
        Distance between points is shrinking: - extension_vel

        """
        self.pB.set_pos(self.pA, 2*self.q1*self.N.x)
        expected = 2*self.q1*self.q1d/sqrt(self.q1**2)
        assert self.pathway.extension_velocity == expected

Your sign convention is a function of position, which strikes me as odd at this moment. I'll think about it more in the morning.

[brocksam]

I think we've got this explained clearly now.

[moorepants]

This has the same issue with the type hints and sphinx for the classes that do not have unique names in sympy.

[oscarbenjamin]

This has the same issue with the type hints and sphinx for the classes that do not have unique names in sympy.

Let me take a look.

[moorepants]

Would "ends" or "endpoints" be a better word and more generic?

[moorepants]

endpoint
ĕnd′point″
noun

Either of two points marking the end of a line segment.

[oscarbenjamin]

This has the same issue with the type hints and sphinx for the classes that do not have unique names in sympy.

Let me take a look.

Here are the first few warnings when building the docs with this PR:

WARNING: autodoc: failed to import class '_system.System' from module 'sympy.physics.mechanics'; the following exception was raised:
No module named 'sympy.physics.mechanics._pathway'
/home/oscar/current/active/sympy/sympy/physics/mechanics/pathway.py:docstring of sympy.physics.mechanics.pathway.LinearPathway.compute_loads:1: WARNING: py:class reference target not found: ExprType
/home/oscar/current/active/sympy/sympy/physics/mechanics/pathway.py:docstring of sympy.physics.mechanics.pathway.LinearPathway.compute_loads:1: WARNING: py:class reference target not found: LoadBase
/home/oscar/current/active/sympy/sympy/physics/mechanics/pathway.py:docstring of sympy.physics.mechanics.pathway.LinearPathway.extension_velocity:1: WARNING: py:class reference target not found: ExprType
/home/oscar/current/active/sympy/sympy/physics/mechanics/pathway.py:docstring of sympy.physics.mechanics.pathway.LinearPathway.length:1: WARNING: py:class reference target not found: ExprType

The first warning is I guess because you have renamed the _pathway module or something.

The next is complaining about ExprType. You can see where ExprType is defined here:

sympy/sympy/physics/mechanics/pathway.py

Lines 13 to 20 in b222234

	if TYPE_CHECKING:
	from sympy.core.backend import USE_SYMENGINE
	from sympy.physics.mechanics.loads import LoadBase
	if USE_SYMENGINE:
	from sympy.core.backend import Basic as ExprType
	else:
	from sympy.core.expr import Expr as ExprType

Sphinx can't see ExprType because of the if TYPE_CHECKING. If you remove that then it will see that ExprType is just Expr (if you are not using SymEngine).

Most of the warnings go away with this change:

diff --git a/sympy/physics/mechanics/pathway.py b/sympy/physics/mechanics/pathway.py
index 833c40fba4..9617713207 100644
--- a/sympy/physics/mechanics/pathway.py
+++ b/sympy/physics/mechanics/pathway.py
@@ -10,14 +10,13 @@
 from sympy.physics.mechanics.wrapping_geometry import GeometryBase
 from sympy.physics.vector import dynamicsymbols
 
-if TYPE_CHECKING:
-    from sympy.core.backend import USE_SYMENGINE
-    from sympy.physics.mechanics.loads import LoadBase
-
-    if USE_SYMENGINE:
-        from sympy.core.backend import Basic as ExprType
-    else:
-        from sympy.core.expr import Expr as ExprType
+from sympy.core.backend import USE_SYMENGINE
+from sympy.physics.mechanics.loads import LoadBase
+
+if USE_SYMENGINE:
+    from sympy.core.backend import Basic as ExprType
+else:
+    from sympy.core.expr import Expr as ExprType
 
 
 __all__ = ['PathwayBase', 'LinearPathway', 'WrappingPathway']

Then Sphinx can see that ExprType is just Expr. Probably the best fix would be to add ExprType itself to the docs. I had to do that with e.g.:

sympy/sympy/polys/matrices/_typing.py

Lines 7 to 16 in 3878065

	class RingElement(Protocol):
	"""A ring element.
	Must support ``+``, ``-``, ``*``, ``**`` and ``-``.
	"""
	def __add__(self: T, other: T, /) -> T: ...
	def __sub__(self: T, other: T, /) -> T: ...
	def __mul__(self: T, other: T, /) -> T: ...
	def __pow__(self: T, other: int, /) -> T: ...
	def __neg__(self: T, /) -> T: ...

https://docs.sympy.org/dev/modules/polys/domainmatrix.html#sympy.polys.matrices._typing.RingElement

[github-actions]

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)

       before           after         ratio
     [8059df73]       [3236f743]
     <sympy-1.12^0>                 
-         107±2ms         70.5±1ms     0.66  integrate.TimeIntegrationRisch02.time_doit(10)
-         108±2ms       70.7±0.8ms     0.65  integrate.TimeIntegrationRisch02.time_doit_risch(10)
-     2.65±0.05ms          712±8μs     0.27  polys.TimePREM_LinearDenseQuadraticGCD.time_op(3, 'sparse')
-      12.7±0.2ms      2.07±0.05ms     0.16  polys.TimePREM_LinearDenseQuadraticGCD.time_op(5, 'sparse')
-        395±10μs         90.1±2μs     0.23  polys.TimePREM_QuadraticNonMonicGCD.time_op(1, 'sparse')
-      6.13±0.1ms         397±10μs     0.06  polys.TimePREM_QuadraticNonMonicGCD.time_op(3, 'sparse')
-      13.3±0.3ms      1.21±0.03ms     0.09  polys.TimePREM_QuadraticNonMonicGCD.time_op(5, 'sparse')
-      7.23±0.2ms       4.29±0.1ms     0.59  polys.TimeSUBRESULTANTS_LinearDenseQuadraticGCD.time_op(2, 'sparse')
-      32.4±0.8ms       13.1±0.2ms     0.40  polys.TimeSUBRESULTANTS_LinearDenseQuadraticGCD.time_op(3, 'sparse')
-      7.70±0.1ms      1.31±0.04ms     0.17  polys.TimeSUBRESULTANTS_QuadraticNonMonicGCD.time_op(1, 'sparse')
-      18.0±0.4ms       9.93±0.4ms     0.55  polys.TimeSUBRESULTANTS_QuadraticNonMonicGCD.time_op(2, 'sparse')
-         250±2ms         76.6±1ms     0.31  polys.TimeSUBRESULTANTS_QuadraticNonMonicGCD.time_op(3, 'sparse')
-     7.79±0.09ms         637±10μs     0.08  polys.TimeSUBRESULTANTS_SparseGCDHighDegree.time_op(3, 'sparse')
-      35.1±0.7ms         982±40μs     0.03  polys.TimeSUBRESULTANTS_SparseGCDHighDegree.time_op(5, 'sparse')
-        713±20μs          253±7μs     0.35  polys.TimeSUBRESULTANTS_SparseNonMonicQuadratic.time_op(1, 'sparse')
-      8.74±0.2ms          262±7μs     0.03  polys.TimeSUBRESULTANTS_SparseNonMonicQuadratic.time_op(3, 'sparse')
-      21.4±0.7ms          257±9μs     0.01  polys.TimeSUBRESULTANTS_SparseNonMonicQuadratic.time_op(5, 'sparse')
-         210±7μs         119±20μs     0.57  solve.TimeMatrixOperations.time_rref(3, 0)
-         368±4μs         145±10μs     0.39  solve.TimeMatrixOperations.time_rref(4, 0)
-        37.2±1ms       16.0±0.3ms     0.43  solve.TimeSolveLinSys189x49.time_solve_lin_sys

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

[brocksam]

1. I'm not sure I see why length and extension_velocity are properties. We can't set them and the calculations that each run can be non-trivial, especially for more complex pathway classes. This makes me think they should be methods instead.

I chose to use properties for length and extension_velocity as a pathway "has" both of these things so I could see a user wanting to treat these as straight attributes. compute_loads needs to be a method because it requires a scalar force to be passed to it.

2. Selecting a frame in which one point has a velocity defined in to take the derivative strikes me as error prone. It isn't clear to me that this will guarantee to give you a correct frame needed for this calculation. Why not just differentiate the length expression with respect to time? (as you've done for the WrappingPathway). That seems less error prone and is precisely what you are after.

I agree with this. Addressed in dc1bfce.