Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Nyquist plot functionality #26436

Open
wants to merge 35 commits into
base: master
Choose a base branch
from

Conversation

Soham-xT
Copy link

@Soham-xT Soham-xT commented Apr 1, 2024

Description:

This pull request adds the Nyquist plot functionality to the project. The nyquist_numerical_data function calculates the numerical data for the Nyquist plot of a given system, while the nyquist_plot function generates and displays the Nyquist plot using the obtained numerical data. It reduces dependencies on external libraries. This approach allows for more efficient computation and plotting of Nyquist plots while providing greater flexibility and control.

Changes:

  • Implemented nyquist_numerical_data function using SymPy's symbolic computation capabilities to generate numerical data for Nyquist plots.

  • Added nyquist_plot function that utilizes SymPy's plot_parametric for generating Nyquist plots using the obtained numerical data.

  • Reduced dependencies on Matplotlib and NumPy, enhancing the project's portability and reducing potential conflicts with other dependencies.

  • Inspired by the approach taken in [GSOC 1.2.1] WIP : Implementation of the Nyquist Plot #25575 for improved functionality and code clarity.

Testing:

Tested both functions with various systems to ensure correct computation and plotting of Nyquist plots.
Instructions for testing:
Define a system using SymPy's TransferFunction.
Call nyquist_numerical_data with the defined system to obtain numerical data.
Call nyquist_plot with the system to generate and display the Nyquist plot.

  1. system = TransferFunction(8, (s**2 + 9*s + 18), s) nyquist_plot(system)

image

  1. system = TransferFunction(2*s**2+5*s+1,(s**2+2*s+3),s) nyquist_plot(system)

image

Additional Notes:

This approach enhances the Nyquist plot functionality while improving code maintainability and readability.
Feedback on the provided functions' documentation and code implementation is appreciated.
Suggestions for further enhancements or optimizations are welcome.

References to other Issues or PRs

#25575

Brief description of what is fixed or changed

The changes in this pull request involve optimizing the Nyquist plot functionality by relying less on external libraries such as Matplotlib and NumPy. Instead, SymPy's built-in symbolic computation capabilities are utilized to generate numerical data for Nyquist plots. The nyquist_numerical_data function now computes symbolic expressions for Nyquist plot data, and the nyquist_plot function uses SymPy's plot_parametric to generate Nyquist plots using these expressions

Other comments

Release Notes

NO ENTRY

@sympy-bot
Copy link

sympy-bot commented Apr 1, 2024

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.

  • No release notes entry will be added for this pull request.
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. -->
## Description:
This pull request adds the Nyquist plot functionality to the project. The nyquist_numerical_data function calculates the numerical data for the Nyquist plot of a given system, while the nyquist_plot function generates and displays the Nyquist plot using the obtained numerical data. It reduces dependencies on external libraries. This approach allows for more efficient computation and plotting of Nyquist plots while providing greater flexibility and control.

## Changes:

- Implemented `nyquist_numerical_data` function using SymPy's symbolic computation capabilities to generate numerical data for Nyquist plots.

- Added `nyquist_plot` function that utilizes SymPy's `plot_parametric` for generating Nyquist plots using the obtained numerical data.

- Reduced dependencies on Matplotlib and NumPy, enhancing the project's portability and reducing potential conflicts with other dependencies.

- Inspired by the approach taken in https://github.com/sympy/sympy/pull/25575 for improved functionality and code clarity.

## Testing:

Tested both functions with various systems to ensure correct computation and plotting of Nyquist plots.
Instructions for testing:
Define a system using SymPy's TransferFunction.
Call nyquist_numerical_data with the defined system to obtain numerical data.
Call nyquist_plot with the system to generate and display the Nyquist plot.

1. `system = TransferFunction(8, (s**2 + 9*s + 18), s)
nyquist_plot(system)`

![image](https://github.com/sympy/sympy/assets/136966898/c4f4f13a-7e31-4219-922a-d32cd0f6a7c0)

2.  `system = TransferFunction(2*s**2+5*s+1,(s**2+2*s+3),s)
nyquist_plot(system)`

![image](https://github.com/sympy/sympy/assets/136966898/3de61e2a-a16e-4bca-a4c3-502bd77116ba)

Additional Notes:

This approach enhances the Nyquist plot functionality while improving code maintainability and readability.
Feedback on the provided functions' documentation and code implementation is appreciated.
Suggestions for further enhancements or optimizations are welcome.

#### 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. -->
https://github.com/sympy/sympy/pull/25575 

#### Brief description of what is fixed or changed
     
The changes in this pull request involve optimizing the Nyquist plot functionality by relying less on external libraries such as Matplotlib and NumPy. Instead, SymPy's built-in symbolic computation capabilities are utilized to generate numerical data for Nyquist plots. The nyquist_numerical_data function now computes symbolic expressions for Nyquist plot data, and the nyquist_plot function uses SymPy's plot_parametric to generate Nyquist plots using these expressions

#### Other comments


#### Release Notes

<!-- BEGIN RELEASE NOTES -->
NO ENTRY
<!-- END RELEASE NOTES -->

Copy link

github-actions bot commented Apr 5, 2024

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 [e7fb2714]    |   Ratio | Benchmark (Parameter)                                                |
|----------|----------------------|---------------------|---------|----------------------------------------------------------------------|
| -        | 70.4±0.8ms           | 44.4±0.3ms          |    0.63 | integrate.TimeIntegrationRisch02.time_doit(10)                       |
| -        | 67.7±1ms             | 43.4±0.5ms          |    0.64 | integrate.TimeIntegrationRisch02.time_doit_risch(10)                 |
| +        | 19.1±0.6μs           | 30.3±0.3μs          |    1.59 | integrate.TimeIntegrationRisch03.time_doit(1)                        |
| -        | 5.48±0.04ms          | 2.85±0.02ms         |    0.52 | logic.LogicSuite.time_load_file                                      |
| -        | 73.0±0.6ms           | 28.5±0.4ms          |    0.39 | polys.TimeGCD_GaussInt.time_op(1, 'dense')                           |
| -        | 25.7±0.05ms          | 16.7±0.1ms          |    0.65 | polys.TimeGCD_GaussInt.time_op(1, 'expr')                            |
| -        | 73.4±0.8ms           | 28.8±0.2ms          |    0.39 | polys.TimeGCD_GaussInt.time_op(1, 'sparse')                          |
| -        | 253±2ms              | 125±0.3ms           |    0.49 | polys.TimeGCD_GaussInt.time_op(2, 'dense')                           |
| -        | 255±1ms              | 123±0.4ms           |    0.48 | polys.TimeGCD_GaussInt.time_op(2, 'sparse')                          |
| -        | 658±5ms              | 366±2ms             |    0.56 | polys.TimeGCD_GaussInt.time_op(3, 'dense')                           |
| -        | 650±2ms              | 368±2ms             |    0.57 | polys.TimeGCD_GaussInt.time_op(3, 'sparse')                          |
| -        | 495±1μs              | 283±2μs             |    0.57 | polys.TimeGCD_LinearDenseQuadraticGCD.time_op(1, 'dense')            |
| -        | 1.78±0.01ms          | 1.07±0.1ms          |    0.6  | polys.TimeGCD_LinearDenseQuadraticGCD.time_op(2, 'dense')            |
| -        | 5.76±0.03ms          | 3.06±0.01ms         |    0.53 | polys.TimeGCD_LinearDenseQuadraticGCD.time_op(3, 'dense')            |
| -        | 453±4μs              | 227±1μs             |    0.5  | polys.TimeGCD_QuadraticNonMonicGCD.time_op(1, 'dense')               |
| -        | 1.48±0.01ms          | 675±6μs             |    0.46 | polys.TimeGCD_QuadraticNonMonicGCD.time_op(2, 'dense')               |
| -        | 4.89±0.01ms          | 1.62±0.01ms         |    0.33 | polys.TimeGCD_QuadraticNonMonicGCD.time_op(3, 'dense')               |
| -        | 378±3μs              | 204±3μs             |    0.54 | polys.TimeGCD_SparseGCDHighDegree.time_op(1, 'dense')                |
| -        | 2.42±0.02ms          | 1.21±0.02ms         |    0.5  | polys.TimeGCD_SparseGCDHighDegree.time_op(3, 'dense')                |
| -        | 9.95±0.05ms          | 4.33±0.04ms         |    0.43 | polys.TimeGCD_SparseGCDHighDegree.time_op(5, 'dense')                |
| -        | 360±2μs              | 169±0.9μs           |    0.47 | polys.TimeGCD_SparseNonMonicQuadratic.time_op(1, 'dense')            |
| -        | 2.46±0.01ms          | 899±10μs            |    0.37 | polys.TimeGCD_SparseNonMonicQuadratic.time_op(3, 'dense')            |
| -        | 9.47±0.1ms           | 2.63±0.01ms         |    0.28 | polys.TimeGCD_SparseNonMonicQuadratic.time_op(5, 'dense')            |
| -        | 1.02±0.02ms          | 424±2μs             |    0.41 | polys.TimePREM_LinearDenseQuadraticGCD.time_op(3, 'dense')           |
| -        | 1.74±0.03ms          | 504±1μs             |    0.29 | polys.TimePREM_LinearDenseQuadraticGCD.time_op(3, 'sparse')          |
| -        | 5.93±0.1ms           | 1.78±0.01ms         |    0.3  | polys.TimePREM_LinearDenseQuadraticGCD.time_op(5, 'dense')           |
| -        | 8.40±0.04ms          | 1.48±0.01ms         |    0.18 | polys.TimePREM_LinearDenseQuadraticGCD.time_op(5, 'sparse')          |
| -        | 283±1μs              | 64.2±0.4μs          |    0.23 | polys.TimePREM_QuadraticNonMonicGCD.time_op(1, 'sparse')             |
| -        | 3.38±0.02ms          | 394±2μs             |    0.12 | polys.TimePREM_QuadraticNonMonicGCD.time_op(3, 'dense')              |
| -        | 3.95±0.03ms          | 275±3μs             |    0.07 | polys.TimePREM_QuadraticNonMonicGCD.time_op(3, 'sparse')             |
| -        | 7.05±0.05ms          | 1.27±0.01ms         |    0.18 | polys.TimePREM_QuadraticNonMonicGCD.time_op(5, 'dense')              |
| -        | 8.71±0.07ms          | 848±3μs             |    0.1  | polys.TimePREM_QuadraticNonMonicGCD.time_op(5, 'sparse')             |
| -        | 4.99±0.02ms          | 2.98±0.01ms         |    0.6  | polys.TimeSUBRESULTANTS_LinearDenseQuadraticGCD.time_op(2, 'sparse') |
| -        | 12.1±0.1ms           | 6.54±0.06ms         |    0.54 | polys.TimeSUBRESULTANTS_LinearDenseQuadraticGCD.time_op(3, 'dense')  |
| -        | 22.3±0.1ms           | 8.96±0.03ms         |    0.4  | polys.TimeSUBRESULTANTS_LinearDenseQuadraticGCD.time_op(3, 'sparse') |
| -        | 5.21±0.01ms          | 870±2μs             |    0.17 | polys.TimeSUBRESULTANTS_QuadraticNonMonicGCD.time_op(1, 'sparse')    |
| -        | 12.5±0.05ms          | 7.03±0.05ms         |    0.56 | polys.TimeSUBRESULTANTS_QuadraticNonMonicGCD.time_op(2, 'sparse')    |
| -        | 102±0.4ms            | 25.9±0.08ms         |    0.25 | polys.TimeSUBRESULTANTS_QuadraticNonMonicGCD.time_op(3, 'dense')     |
| -        | 165±0.8ms            | 53.7±0.2ms          |    0.33 | polys.TimeSUBRESULTANTS_QuadraticNonMonicGCD.time_op(3, 'sparse')    |
| -        | 175±1μs              | 113±0.6μs           |    0.64 | polys.TimeSUBRESULTANTS_SparseGCDHighDegree.time_op(1, 'dense')      |
| -        | 360±2μs              | 215±1μs             |    0.6  | polys.TimeSUBRESULTANTS_SparseGCDHighDegree.time_op(1, 'sparse')     |
| -        | 4.23±0.04ms          | 856±10μs            |    0.2  | polys.TimeSUBRESULTANTS_SparseGCDHighDegree.time_op(3, 'dense')      |
| -        | 5.21±0.02ms          | 384±3μs             |    0.07 | polys.TimeSUBRESULTANTS_SparseGCDHighDegree.time_op(3, 'sparse')     |
| -        | 20.0±0.2ms           | 2.80±0.01ms         |    0.14 | polys.TimeSUBRESULTANTS_SparseGCDHighDegree.time_op(5, 'dense')      |
| -        | 23.1±0.06ms          | 623±3μs             |    0.03 | polys.TimeSUBRESULTANTS_SparseGCDHighDegree.time_op(5, 'sparse')     |
| -        | 479±3μs              | 134±0.8μs           |    0.28 | polys.TimeSUBRESULTANTS_SparseNonMonicQuadratic.time_op(1, 'sparse') |
| -        | 4.72±0.05ms          | 619±4μs             |    0.13 | polys.TimeSUBRESULTANTS_SparseNonMonicQuadratic.time_op(3, 'dense')  |
| -        | 5.32±0.09ms          | 140±1μs             |    0.03 | polys.TimeSUBRESULTANTS_SparseNonMonicQuadratic.time_op(3, 'sparse') |
| -        | 13.0±0.2ms           | 1.28±0ms            |    0.1  | polys.TimeSUBRESULTANTS_SparseNonMonicQuadratic.time_op(5, 'dense')  |
| -        | 14.1±0.06ms          | 144±2μs             |    0.01 | polys.TimeSUBRESULTANTS_SparseNonMonicQuadratic.time_op(5, 'sparse') |
| -        | 134±1μs              | 75.8±0.4μs          |    0.57 | solve.TimeMatrixOperations.time_rref(3, 0)                           |
| -        | 255±4μs              | 87.8±0.3μs          |    0.34 | solve.TimeMatrixOperations.time_rref(4, 0)                           |
| -        | 24.2±0.1ms           | 10.3±0.02ms         |    0.42 | solve.TimeSolveLinSys189x49.time_solve_lin_sys                       |
| -        | 28.4±0.4ms           | 15.5±0.1ms          |    0.55 | solve.TimeSparseSystem.time_linsolve_Aaug(20)                        |
| -        | 54.7±0.2ms           | 24.9±0.09ms         |    0.45 | solve.TimeSparseSystem.time_linsolve_Aaug(30)                        |
| -        | 28.1±0.2ms           | 15.3±0.07ms         |    0.54 | solve.TimeSparseSystem.time_linsolve_Ab(20)                          |
| -        | 54.9±0.2ms           | 24.6±0.09ms         |    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).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants