Repository navigation
Conversation
add pretty printing for discrete-time transfer functions remove unnecessary newline in at the end of the file
|
✅ 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.15. Click here to see the pull request description that was parsed. |
|
|
||
| def test_DTTransferFunction_construction(): | ||
| z = symbols("z") | ||
| tf = DTTransferFunction(z + 1, z**2 + z + 1, z, 10) |
There was a problem hiding this comment.
What do you plan for having fully symbolic transfer functions?
I've seen some recent activity about a pade approximation elsewhere in SymPy, are you going to use that?
There was a problem hiding this comment.
What do you mean?
The Padé approximation can be useful for approximating time delays. Can it be used for other purposes as well?
There was a problem hiding this comment.
I haven't taken digital control in a long long time, so I don't remember everything, but I thought that when transforming a continous transfer function into a discrete transfer function that you have a choice in the method of discretization. I thought using a pade approximation was one of them, but now I think I mis remembered.
So maybe my question should be: how are you going to manage different choices in discretization? Or is this assuming a zero order hold always?
There was a problem hiding this comment.
Honestly I didn't know much about Padé approximation for discretization, but I did some research and I found helpful resources.
Anyway, I'm going to group these functions
sympy/sympy/physics/control/lti.py
Lines 44 to 208 in c0fc066
into
DTTransferFunction as classmethods and I'm going to add as many discretization methods as I can.
…aceBase in test_args TODO: write test for DTStateSpace
… DTTransferFunction class
…alization in Series divisions and returned to previous eval_frequency calculation
…on to discrete-time. Add tests.
|
Please fill out all the top information: References to other Issues or PRs NO ENTRY |
| raises(NotImplementedError, | ||
| lambda: TransferFunctionBase.from_coeff_lists([1], [1], s)) | ||
| raises(NotImplementedError, | ||
| lambda: TransferFunctionBase.from_zpk([1], [1], 1, s)) |
There was a problem hiding this comment.
If we aren't implementing them, I would simply not have them as methods.
There was a problem hiding this comment.
I've done this because these functions must be inherited by a subclass, and we need to be aware that they exist.
| tf6_ = DTTransferFunction(1/2, 4, s) | ||
| assert tf6_.num == 0.5 | ||
| assert tf6_.den == 4 | ||
| assert tf6_.args == (0.500000000000000, 4, s, 1) |
There was a problem hiding this comment.
checking floating point with == is not good practice, you should check with a tolerance, i think there is a sympy function for this
There was a problem hiding this comment.
Are you referring to this?
import math
Float(0.1, 9).is_same(Float(0.1, 10), math.isclose)| expect4_ = DTTransferFunction(7*p**3 + s, p - s, p) | ||
| assert SP4.subs({a0: -1, a1: -7}) == expect4_ | ||
| assert SP4.subs({a0: -1, a1: -7}).evalf() == expect4 | ||
| assert expect4_.evalf() == expect4 |
There was a problem hiding this comment.
All of the above numerical substitution checks are ok to have, but you need to check symbolics as the primary test.
| 5*a2*b1*s**(S(8)/3)/3 + S(25)*b1*s/9) - \ | ||
| 5*3**(S(1)/3)*5**(S(2)/3)*\ | ||
| b0/(9*a1*a2*s**(S(8)/3) + 15*a1*s + \ | ||
| 15*a2*b1*s**(S(8)/3) + 25*b1*s) |
There was a problem hiding this comment.
Do these expected results come from text book answers? How do you know they are correct results?
There was a problem hiding this comment.
Best to avoid \ for line continuation. Just use parentheses around long lines.
There was a problem hiding this comment.
Do these expected results come from text book answers? How do you know they are correct results?
These tests are the same as the ones from the continuous-time case, so I assume they hold. I just added the one at line 722, which comes from here https://it.mathworks.com/help/control/ref/dynamicsystem.evalfr.html
| # Low-frequency (or DC) gain. | ||
| assert DTTransferFunction(z, z - 1, z).dc_gain() == oo | ||
| assert DTTransferFunction(z - 1, z**2 + 3*z + 1, z).dc_gain() == 0 | ||
| assert DTTransferFunction(z, z + 2, z, 0.2).dc_gain() == Rational(1, 3) |
There was a problem hiding this comment.
Would be nice to know if these are textbook results.
There was a problem hiding this comment.
I've applied the definition of dc_gain manually. I can find some references if needed.
| dtf_manual = DTTransferFunction.\ | ||
| from_coeff_lists(num_manual, den_manual, z, T).expand() | ||
|
|
||
| assert S.Zero == (tf_test_bilinear.simplify()-tf_test_manual.simplify()).simplify().num |
There was a problem hiding this comment.
We should not change tests unless this was somehow giving an incorrect result.
There was a problem hiding this comment.
I just thought it was the wrong way to verify correctness. My changes check the same thing.
| property is used for transfer functions in the ``Parallel`` class, and the | ||
| multiplicative property is used for transfer functions in the ``Series`` class. | ||
| represent Continuous-time Transfer functions in the Laplace domain, while | ||
| ``DTTransferFunction`` represent Discrete-time Transfer functions in the |
There was a problem hiding this comment.
I think we should name this "DiscreteTranferFunction", i.e. use full words instead of single letters. I'm open for other options, but it is customary in Python to be descriptive with names.
|
@moorepants I added the example in the tutorial and implemented the |
|
@moorepants I made the I noticed some pre-existing style issues, but I don't think it's worth fixing them here since this PR is already quite large. |
…tion tests for a change in its form
|
|
||
| """ | ||
| m = Mul(self.num, Pow(self.den, -1, evaluate=False), evaluate=False) | ||
| return limit(m, self.var, 0) | ||
| pass |
There was a problem hiding this comment.
Seems odd to have pass here.
There was a problem hiding this comment.
It's because it's an abstract method
- Renamed `new_tf` to `new_transfer_function` for clarity - Adjusted LaTeX and pretty printing outputs to use "sampling time" instead of "sampling_time". - Adjusted tutorial formatting and changed the image type to svg
|
I made the proposed changes, when we are ready I will open a new PR clone, so we won't have problems with the png images I uploaded. |
| Example 6 | ||
| --------- | ||
|
|
||
| .. image:: Control_Problems_Q6.svg |
|
Closing in favor of #28318 |

References to other Issues or PRs
Brief description of what is fixed or changed
This PR lays the groundwork for extending the lti module to discrete time. It implements the
TransferFunctionBaseclass, which is inherited by bothTransferFunctionandDiscreteTransferFunction. It also extends the ability to create interconnections between discrete-time systems.Other comments
Release Notes