Skip to content

Add support for continuous delay systems #1148

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

Draft
wants to merge 18 commits into
base: main
Choose a base branch
from

Conversation

MythasNauveili
Copy link

This PR introduces initial support for linear time-invariant (LTI) systems with time delays in python-control, inspired by the capabilities of ControlSystems.jl package, and the InternalDelay.py script from Skogestad-Python.

Key features

  • Creation of delay sytems Systems with delays can now be intuitively constructed by multiplying a TransferFunction object with a delay(tau) object or an exp(-tau*s) expression:
    tf_delay = tf([1], [1,1]) * delay(2)
    # or
    s = tf("s")
    tf_delay = tf([1], [1,1]) * exp(-2*s)
  • Underlying Representation: The approach utilizes Linear Fractional Transformations (LFT) with a PartitionedStateSpace representation (implemented in partitionedssp.py). The DelayLTI class (in delaylti.py) serves as the core object for these systems.
  • Operations: Standard operations such as addition, multiplication, and feedback are supported for DelayLTI objects.
  • Time and Frequency Responses: Time responses for delayed systems are computed by solving the underlying Delay Differential Equations (DDEs). Frequency responses are also supported.
  • Retrocompatibility: Maintain retrocompatibility with existing functionalities.

Implementation Details:

  • DDE Solver: Time responses for delayed systems are computed by solving DDEs. After exploring alternatives, a custom DDE solver based on the Method of Steps, wrapping SciPy's solve_ivp, was implemented in dde.py.
    • Accuracy Note: While this solver performs well for many cases, for stiff problems, it may require smaller step sizes for convergence compared to specialized DDE solvers (like those in ControlSystems.jl). In tested scenarios, the error typically remains at least three orders of magnitude below the system response with reasonable step sizes. (An error estimation algorithm could be a valuable future addition).
  • Validation: A Julia script (control/julia/compute_tests.jl) using ControlSystems.jl has been added to generate reference results, which are exported to a JSON file. These results are used in the test suite (control/tests/delay_lti_test.py and control/tests/dde_test.py) to benchmark the accuracy of the DelayLTI implementation and DDE solver.

Current Scope and Limitations:

  • This implementation currently focuses on continuous-time delay systems. Discrete-time delay systems are not yet covered.
  • The DDE solver may benefit from further enhancements for stiff problems or adaptive step sizing.

Discussion Points:
This PR represents the initial groundwork for this feature. I'm submitting this draft to share these first steps and would appreciate feedback on:

  • The overall approach and class design (DelayLTI, PartitionedStateSpace).
  • The DDE solver implementation and its current performance.
  • Any potential integration issues or API considerations.

I'm aware there is still work to be done for this feature to be ready for a full release, but I believe this provides a solid foundation.

Copy link
Member

@murrayrm murrayrm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Initial round of comments. I haven't yet checked functionality, but found a few style issues that should be updated.

control/dde.py Outdated
U = _check_convert_array(
U, legal_shapes, "Parameter `U`: ", squeeze=False, transpose=transpose
)
print("U shape: ", U.shape)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like a debugging statement that was left in the code?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed in last commit

control/dde.py Outdated
discontinuity_times.remove(t_stop)
local_t_eval = [t for t in T if current_t < t <= t_stop]

print("Integrate bewtween ", current_t, " and ", t_stop)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Debugging statement?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed in last commit

@@ -89,6 +89,7 @@

# Allow access to phase_plane functions as ct.phaseplot.fcn or ct.pp.fcn
from . import phaseplot as phaseplot

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was there a reason to add an extraneous blank line here?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no, blank line is removed in the last commit

control/dde.py Outdated
)


def pchip_interp_u(T, U):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like it is probably an internal function. If so, add underscore to start of function name.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

underscore added

control/dde.py Outdated
return np.array([negative_wrapper(PchipInterpolator(T, ui)) for ui in U])


class DdeHistory:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might be better named DDEHistory (since DDE is a standard abbreviation, like LTI)?

Also, if this is an internal class, begin the class name with an underscore.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed the name and added the underscore

Notes
-----
The way to create a DelayLTI object is by multiplying a transfert
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo: transfert → transfer

Comment on lines 99 to 100
It's possible to MIMO delayed systems from arrays of SISO delayed systems,
see function mimo_delay
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's possible to MIMO → It is possible to create

Also missing a period at the end.


# Poles and zeros functions for DelayLTI are not supported by julia ControlSystems.jl
# might not be accurate for delayLTI, to be discussed
# Here the poles and zeros computed are the ones of the system without taking into accoutn the delay
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo + may be longer than 78 char (PEP8).

# Check dimensions of K
if K.shape != (self.nu, self.ny):
raise ValueError(
f"Feedback gain K has incompatible shape. Expected ({self.nu}, {self.ny}), got {K.shape}."
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update to avoid line with more than 78 characters and put closing parenthesis at end of string, for consistency with current code base.

Comment on lines 549 to 552
def issiso(self):
"""Check if the system is single-input, single-output."""
# Based on EXTERNAL dimensions
return self.nu == 1 and self.ny == 1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be handled by the parent class (InputOutputSystem).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is now moved to InputOutputSystem

@murrayrm
Copy link
Member

This would be a great contribution, @MythasNauveili. Thanks for working on it!

There are some issues with unit tests. In addition to the failed ruff and doctest checks, it looks like some of the standard tests might be hanging. Not sure if that is due to the new code or if something is messed up in the GitHub actions. I'll try to come back and take a look at that later.

In the meantime, some style things to adjust in your next pass.

@MythasNauveili
Copy link
Author

Thanks for the first feedback @murrayrm. I tried to fix the most in the two last commits. For the tests hanging, it might be because of debug plots that I did not disable in dde_test.py (this should be done now).

@coveralls
Copy link

coveralls commented May 12, 2025

Coverage Status

coverage: 94.16% (-0.6%) from 94.746%
when pulling 1150646 on MythasNauveili:dev-delay
into 632391c on python-control:main.

@murrayrm
Copy link
Member

@MythasNauveili There are still two unit tests failing. Click on the links above to see the issues. The ruff checks should be straightforward. For the doctest, it looks like something is printing out spurious information; not sure where that is happening.

@MythasNauveili
Copy link
Author

@murrayrm it might be ok now, I just removed a forgotten debug print in timeresp.py that was the source of the dxdt = [...]

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.

3 participants