search

sympy / sympy

A computer algebra system written in pure Python
https://sympy.org/
Other
12.49k stars 4.32k forks source link

Continue: Obtaining reaction loads from apply_support #26569

Closed mvg2002 closed 3 weeks ago

mvg2002 commented 3 weeks ago

References to other Issues or PRs

Uses the code proposed in PR #26296 Different solution for the same problem as PR #26567 #### Brief description of what is fixed or changed The ``apply_support()`` now returns reaction load symbols that can be used in ``solve_for_reaction_loads()``. This prevents users from having to create the symbols for unknown reactions. #### Other comments This pull request uses the solution proposed in PR #26296 after this possible solution was pointed out to me in PR #26567. The documentation is updated and the test is modified. PR #26296 used the following code: ``` def apply_support(self, loc, type="fixed", return_ = False): if not return_: return if type in ("pin", "roller"): return reaction_load else: return reaction_load, reaction_moment ``` This solution has the following code: ``` def apply_support(self, loc, type="fixed"): if type in ("pin", "roller"): return reaction_load else: return reaction_load, reaction_moment ``` The ` return_` parameter was left out in this proposed solution, because I didn't see the real added value. I thought leaving it out would be better for clarity and simplisity of the code. As I'm quite new to SymPy I might have overlooked something and leaving it in would be better. Please let me know if this is the case (and also why for my own learning process). #### Release Notes
sympy-bot commented 3 weeks ago

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

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. #### References to other Issues or PRs Uses the code proposed in PR #26296 Different solution for the same problem as PR #26567 #### Brief description of what is fixed or changed The ``apply_support()`` now returns reaction load symbols that can be used in ``solve_for_reaction_loads()``. This prevents users from having to create the symbols for unknown reactions. #### Other comments This pull request uses the solution proposed in PR #26296 after this possible solution was pointed out to me in PR #26567. The documentation is updated and the test is modified. PR #26296 used the following code: ``` def apply_support(self, loc, type="fixed", return_ = False): if not return_: return if type in ("pin", "roller"): return reaction_load else: return reaction_load, reaction_moment ``` This solution has the following code: ``` def apply_support(self, loc, type="fixed"): if type in ("pin", "roller"): return reaction_load else: return reaction_load, reaction_moment ``` The ` return_` parameter was left out in this proposed solution, because I didn't see the real added value. I thought leaving it out would be better for clarity and simplisity of the code. As I'm quite new to SymPy I might have overlooked something and leaving it in would be better. Please let me know if this is the case (and also why for my own learning process). #### Release Notes * physics.continuum_mechanics * Changed `apply_support()` to prevent having to create unknown reaction symbols.

**Update** The release notes on the [wiki](https://github.com/sympy/sympy/wiki/Release-Notes-for-1.13) have been updated.

moorepants commented 3 weeks ago

Thanks. This looks good other than the two comments.

moorepants commented 3 weeks ago

You'll need to add yourself to the AUTHORS files to have that test pass.

moorepants commented 3 weeks ago

I get this failure when running doctests locally:

sympy(master)$ bin/doctest sympy/physics/continuum_mechanics/
==================================================================================== test process starts =====================================================================================
executable:         /home/moorepants/miniconda/bin/python  (3.10.14-final-0) [CPython]
architecture:       64-bit
cache:              yes
ground types:       gmpy 2.1.5
numpy:              1.26.4
hash randomization: on (PYTHONHASHSEED=495277368)

sympy/physics/continuum_mechanics/beam.py[43] ..............F............................                                                                                               [FAIL]
sympy/physics/continuum_mechanics/cable.py[5] .....                                                                                                                                       [OK]
sympy/physics/continuum_mechanics/truss.py[13] .............                                                                                                                              [OK]

______________________________________________________________________________________________________________________________________________________________________________________________
______________________________________________________________________ sympy.physics.continuum_mechanics.beam.Beam.draw ______________________________________________________________________
File "/home/moorepants/src/sympy/sympy/physics/continuum_mechanics/beam.py", line 2184, in sympy.physics.continuum_mechanics.beam.Beam.draw
Failed example:
    p  # doctest: +ELLIPSIS
Exception raised:
    Traceback (most recent call last):
      File "/home/moorepants/miniconda/lib/python3.10/doctest.py", line 1350, in __run
        exec(compile(example.source, filename, "single",
      File "<doctest sympy.physics.continuum_mechanics.beam.Beam.draw[17]>", line 1, in <module>
        p  # doctest: +ELLIPSIS
    NameError: name 'p' is not defined
**********************************************************************
File "/home/moorepants/src/sympy/sympy/physics/continuum_mechanics/beam.py", line 2191, in sympy.physics.continuum_mechanics.beam.Beam.draw
Failed example:
    p.show()
Exception raised:
    Traceback (most recent call last):
      File "/home/moorepants/miniconda/lib/python3.10/doctest.py", line 1350, in __run
        exec(compile(example.source, filename, "single",
      File "<doctest sympy.physics.continuum_mechanics.beam.Beam.draw[18]>", line 1, in <module>
        p.show()
    NameError: name 'p' is not defined

=================================================================== tests finished: 60 passed, 1 failed, in 17.12 seconds ====================================================================
DO *NOT* COMMIT!

The question would be as to why that doesn't cause a failure in the CI.

moorepants commented 3 weeks ago

It looks like the CI is skipping that doctest:

sympy/physics/continuum_mechanics/beam.py[43] ..............s...................
.........
mvg2002 commented 3 weeks ago

Okay that explains it. I already doubted that this error was the result of the new code. Thank you for the explanation. I will implement your remark on the docstring and push again.

github-actions[bot] commented 3 weeks ago

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)

| Change   | Before [2487dbb5]    | After [08595175]    |   Ratio | Benchmark (Parameter)                                                |
|----------|----------------------|---------------------|---------|----------------------------------------------------------------------|
| -        | 69.1±1ms             | 44.3±0.2ms          |    0.64 | integrate.TimeIntegrationRisch02.time_doit(10)                       |
| -        | 67.7±0.7ms           | 43.1±0.2ms          |    0.64 | integrate.TimeIntegrationRisch02.time_doit_risch(10)                 |
| +        | 18.3±0.4μs           | 30.4±1μs            |    1.66 | integrate.TimeIntegrationRisch03.time_doit(1)                        |
| -        | 5.29±0.03ms          | 2.85±0.01ms         |    0.54 | logic.LogicSuite.time_load_file                                      |
| -        | 72.6±0.4ms           | 28.9±0.4ms          |    0.4  | polys.TimeGCD_GaussInt.time_op(1, 'dense')                           |
| -        | 25.5±0.2ms           | 17.0±0.05ms         |    0.66 | polys.TimeGCD_GaussInt.time_op(1, 'expr')                            |
| -        | 73.1±0.6ms           | 29.0±0.1ms          |    0.4  | polys.TimeGCD_GaussInt.time_op(1, 'sparse')                          |
| -        | 250±2ms              | 126±1ms             |    0.5  | polys.TimeGCD_GaussInt.time_op(2, 'dense')                           |
| -        | 255±2ms              | 125±0.8ms           |    0.49 | polys.TimeGCD_GaussInt.time_op(2, 'sparse')                          |
| -        | 644±3ms              | 369±2ms             |    0.57 | polys.TimeGCD_GaussInt.time_op(3, 'dense')                           |
| -        | 651±3ms              | 371±0.8ms           |    0.57 | polys.TimeGCD_GaussInt.time_op(3, 'sparse')                          |
| -        | 490±2μs              | 291±1μs             |    0.59 | polys.TimeGCD_LinearDenseQuadraticGCD.time_op(1, 'dense')            |
| -        | 1.78±0ms             | 1.07±0.01ms         |    0.6  | polys.TimeGCD_LinearDenseQuadraticGCD.time_op(2, 'dense')            |
| -        | 5.74±0.08ms          | 3.12±0.05ms         |    0.54 | polys.TimeGCD_LinearDenseQuadraticGCD.time_op(3, 'dense')            |
| -        | 448±4μs              | 230±2μs             |    0.51 | polys.TimeGCD_QuadraticNonMonicGCD.time_op(1, 'dense')               |
| -        | 1.47±0.01ms          | 686±4μs             |    0.47 | polys.TimeGCD_QuadraticNonMonicGCD.time_op(2, 'dense')               |
| -        | 4.92±0.07ms          | 1.67±0.01ms         |    0.34 | polys.TimeGCD_QuadraticNonMonicGCD.time_op(3, 'dense')               |
| -        | 372±2μs              | 210±0.8μs           |    0.57 | polys.TimeGCD_SparseGCDHighDegree.time_op(1, 'dense')                |
| -        | 2.42±0.02ms          | 1.24±0.01ms         |    0.51 | polys.TimeGCD_SparseGCDHighDegree.time_op(3, 'dense')                |
| -        | 9.99±0.04ms          | 4.34±0.04ms         |    0.43 | polys.TimeGCD_SparseGCDHighDegree.time_op(5, 'dense')                |
| -        | 354±3μs              | 172±1μs             |    0.48 | polys.TimeGCD_SparseNonMonicQuadratic.time_op(1, 'dense')            |
| -        | 2.49±0.02ms          | 923±7μs             |    0.37 | polys.TimeGCD_SparseNonMonicQuadratic.time_op(3, 'dense')            |
| -        | 9.43±0.2ms           | 2.70±0.04ms         |    0.29 | polys.TimeGCD_SparseNonMonicQuadratic.time_op(5, 'dense')            |
| -        | 1.02±0.01ms          | 427±3μs             |    0.42 | polys.TimePREM_LinearDenseQuadraticGCD.time_op(3, 'dense')           |
| -        | 1.71±0.01ms          | 517±1μs             |    0.3  | polys.TimePREM_LinearDenseQuadraticGCD.time_op(3, 'sparse')          |
| -        | 5.88±0.01ms          | 1.80±0.01ms         |    0.31 | polys.TimePREM_LinearDenseQuadraticGCD.time_op(5, 'dense')           |
| -        | 8.40±0.05ms          | 1.54±0.01ms         |    0.18 | polys.TimePREM_LinearDenseQuadraticGCD.time_op(5, 'sparse')          |
| -        | 285±4μs              | 66.4±0.9μs          |    0.23 | polys.TimePREM_QuadraticNonMonicGCD.time_op(1, 'sparse')             |
| -        | 3.38±0.04ms          | 396±8μs             |    0.12 | polys.TimePREM_QuadraticNonMonicGCD.time_op(3, 'dense')              |
| -        | 3.98±0.05ms          | 285±0.6μs           |    0.07 | polys.TimePREM_QuadraticNonMonicGCD.time_op(3, 'sparse')             |
| -        | 7.00±0.04ms          | 1.26±0.01ms         |    0.18 | polys.TimePREM_QuadraticNonMonicGCD.time_op(5, 'dense')              |
| -        | 8.72±0.02ms          | 858±7μs             |    0.1  | polys.TimePREM_QuadraticNonMonicGCD.time_op(5, 'sparse')             |
| -        | 5.04±0.02ms          | 3.01±0.01ms         |    0.6  | polys.TimeSUBRESULTANTS_LinearDenseQuadraticGCD.time_op(2, 'sparse') |
| -        | 12.2±0.09ms          | 6.68±0.02ms         |    0.55 | polys.TimeSUBRESULTANTS_LinearDenseQuadraticGCD.time_op(3, 'dense')  |
| -        | 22.4±0.3ms           | 9.20±0.08ms         |    0.41 | polys.TimeSUBRESULTANTS_LinearDenseQuadraticGCD.time_op(3, 'sparse') |
| -        | 5.23±0.04ms          | 874±3μs             |    0.17 | polys.TimeSUBRESULTANTS_QuadraticNonMonicGCD.time_op(1, 'sparse')    |
| -        | 12.6±0.05ms          | 7.08±0.01ms         |    0.56 | polys.TimeSUBRESULTANTS_QuadraticNonMonicGCD.time_op(2, 'sparse')    |
| -        | 100±0.5ms            | 25.7±0.1ms          |    0.26 | polys.TimeSUBRESULTANTS_QuadraticNonMonicGCD.time_op(3, 'dense')     |
| -        | 168±2ms              | 54.5±0.2ms          |    0.32 | polys.TimeSUBRESULTANTS_QuadraticNonMonicGCD.time_op(3, 'sparse')    |
| -        | 172±1μs              | 113±0.7μs           |    0.66 | polys.TimeSUBRESULTANTS_SparseGCDHighDegree.time_op(1, 'dense')      |
| -        | 364±6μs              | 217±2μs             |    0.6  | polys.TimeSUBRESULTANTS_SparseGCDHighDegree.time_op(1, 'sparse')     |
| -        | 4.20±0.04ms          | 836±6μs             |    0.2  | polys.TimeSUBRESULTANTS_SparseGCDHighDegree.time_op(3, 'dense')      |
| -        | 5.21±0.03ms          | 388±4μs             |    0.07 | polys.TimeSUBRESULTANTS_SparseGCDHighDegree.time_op(3, 'sparse')     |
| -        | 20.2±0.2ms           | 2.83±0.01ms         |    0.14 | polys.TimeSUBRESULTANTS_SparseGCDHighDegree.time_op(5, 'dense')      |
| -        | 22.6±0.05ms          | 639±4μs             |    0.03 | polys.TimeSUBRESULTANTS_SparseGCDHighDegree.time_op(5, 'sparse')     |
| -        | 483±1μs              | 136±0.8μs           |    0.28 | polys.TimeSUBRESULTANTS_SparseNonMonicQuadratic.time_op(1, 'sparse') |
| -        | 4.63±0.03ms          | 612±2μs             |    0.13 | polys.TimeSUBRESULTANTS_SparseNonMonicQuadratic.time_op(3, 'dense')  |
| -        | 5.22±0.03ms          | 138±0.6μs           |    0.03 | polys.TimeSUBRESULTANTS_SparseNonMonicQuadratic.time_op(3, 'sparse') |
| -        | 12.8±0.07ms          | 1.29±0.01ms         |    0.1  | polys.TimeSUBRESULTANTS_SparseNonMonicQuadratic.time_op(5, 'dense')  |
| -        | 13.7±0.2ms           | 142±0.9μs           |    0.01 | polys.TimeSUBRESULTANTS_SparseNonMonicQuadratic.time_op(5, 'sparse') |
| -        | 135±0.7μs            | 74.3±0.7μs          |    0.55 | solve.TimeMatrixOperations.time_rref(3, 0)                           |
| -        | 250±2μs              | 88.0±0.7μs          |    0.35 | solve.TimeMatrixOperations.time_rref(4, 0)                           |
| -        | 24.2±0.1ms           | 10.2±0.03ms         |    0.42 | solve.TimeSolveLinSys189x49.time_solve_lin_sys                       |
| -        | 28.4±0.1ms           | 15.5±0.09ms         |    0.55 | solve.TimeSparseSystem.time_linsolve_Aaug(20)                        |
| -        | 54.4±0.2ms           | 25.0±0.05ms         |    0.46 | solve.TimeSparseSystem.time_linsolve_Aaug(30)                        |
| -        | 28.5±0.2ms           | 15.2±0.08ms         |    0.53 | solve.TimeSparseSystem.time_linsolve_Ab(20)                          |
| -        | 54.8±0.4ms           | 24.8±0.1ms          |    0.45 | solve.TimeSparseSystem.time_linsolve_Ab(30)                          |

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

moorepants commented 3 weeks ago

Congrats on your first PR. All looks good. Thanks!

mvg2002 commented 2 weeks ago

@Tom-van-Woudenberg