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

Control: function addition for the phase criterion #26341

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

Conversation

Spiros7bit
Copy link

@Spiros7bit Spiros7bit commented Mar 10, 2024

Brief description of what is fixed or changed

I added a function to the TransferFunction class called phase_condition and has an input.
This input can be a number that belongs to C,
if it belongs to the Root Locus of the transfer function it returns True otherwise False.

Other comments

  • I think it's a good start for creating extensions over the Root Locus.

Release Notes

  • physics.control
    • Added a new function for phase condition.

Reference

Root Locus Analysis

I added a function to the TransferFunction class
called phase_criterion(z) and has an input.
This input can be a number that belongs to C and
if it belongs to the Root Locus of the transfer
function it returns True otherwise False. I think
it's a good start for creating extensions over the
Root Locus.

Example:

>>> system2 = TransferFunction((1), ((s**2+100*s+2600)*(s+25)*s), s)
>>> z1 = 37.1+65.2*I
>>> system2.phase_criterion(z1)
True

modified:   sympy/physics/control/lti.py
modified:   sympy/physics/control/tests/test_lti.py
@sympy-bot
Copy link

sympy-bot commented Mar 10, 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.

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.
<!-- Control: function addition for the phase condition. -->


#### Brief description of what is fixed or changed
I added a function to the TransferFunction class called phase_condition and has an input. 
This input can be a number that belongs to C, 
if it belongs to the Root Locus of the transfer function it returns True otherwise False. 

#### Other comments
* I think it's a good start for creating extensions over the Root Locus.

#### Release Notes
<!-- BEGIN RELEASE NOTES -->
* physics.control
    * Added a new function for phase condition.
<!-- END RELEASE NOTES -->

#### Reference
[Root Locus Analysis](https://en.wikipedia.org/wiki/Root_locus_analysis)

@moorepants
Copy link
Member

Can you point to something that explains what the phase criteria is?

This may be a useful addition, but you'll need to make it work with fully symbolic transfer functions.

@Spiros7bit
Copy link
Author

Yes, it was a mistake not to analyze the phase criterion further. Τhe sum of the angles from the open-loop zeros to the point minus the angles from the open-loop poles to the point has to be equal to π. The point mentioned in the above wikipedia extension is the number we give to the function. If this criterion is true then this point is rootlocus' part.
I also understand that the name I gave is incorrect. The correct name is "phase condition" (not "phase criterion").

@Spiros7bit
Copy link
Author

I will correct the name and the errors that were thrown out. And I'll commit again.

I added a function to the TransferFunction class
called phase_condition(z) and has an input.
This input can be a number that belongs to C and
if it belongs to the Root Locus of the transfer function
phase_condition returns True otherwise False.
I think it's a good start for creating extensions over the Root Locus.

Example:

>>> system2 = TransferFunction((1), ((s**2+100*s+2600)*(s+25)*s), s)
>>> z1 = 37.1+65.2*I
>>> system2.phase_condition(z1)
True

modified:   sympy/physics/control/lti.py
modified:   sympy/physics/control/tests/test_lti.py

Note:
This is the same commit with previous but with correct name (condition, not criterion)

Reference:
https://en.wikipedia.org/wiki/Root_locus_analysis
This code quality error occur:
AssertionError: File contains trailing whitespace: /home/runner/work/sympy/sympy/sympy/physics/control/lti.py
Modified the code to pass the code-quality test
@Spiros7bit
Copy link
Author

Can you point to something that explains what the phase criteria is?

This may be a useful addition, but you'll need to make it work with fully symbolic transfer functions.

When I started writing this function, I wanted to make it as symbolic as possible. When I say symbolic, I mean using only the sympy tools. Could you refer me or give me more information about what you are thinking about?
Thank you for your time.

@moorepants
Copy link
Member

You can start by having no numbers in your unit tests, only symbols.

Notes: (I forgot to mention that the relation should be equal to the odd multiple of pi).
I modified the function to work with symbolic input numbers. Some constraints arose:
1. I was able to do the implementation for the first 8 odd multiples.
2. The processing of the symbolic characters needs a little more work.
Substrate the greek letter π with pi.
There was an error about the parentheses in the test_phase_condition.
@Spiros7bit
Copy link
Author

Spiros7bit commented Mar 16, 2024

You can start by having no numbers in your unit tests, only symbols.

I modified the function to be able to process symbolic inputs. But it still needs some corrections.

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

Successfully merging this pull request may close these issues.

None yet

3 participants