GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
GeezerVison is showing github.com — Add support for continuous delay systems by MythasNauveili · Pull Request #1148 · python-control/python-control ConversationThis PR introduces initial support for linear time-invariant (LTI) systems with time delays in Key features
Implementation Details:
Current Scope and Limitations:
Discussion Points:
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.
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactions…m to solve delay ODE in timeresp. Add litlle tests about timereps …n the DDE integration implemented only basic dde solving based on Skogestad python …improve doc and julia notebook. Clean the code …alize with json file. Fix scalar multiplication …ests. TODO: clean dde file, test functions in it, and reloacate dde response tests. More tests also like mimo force response are needed
murrayrm
requested changes
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactions
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This looks like a debugging statement that was left in the code?
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactions
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. removed in last commit
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactions
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Debugging statement?
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactions
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. removed in last commit
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactions
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Was there a reason to add an extraneous blank line here?
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactions
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. no, blank line is removed in the last commit
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactions
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactions
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. underscore added
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactions
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This might be better named Also, if this is an internal class, begin the class name with an underscore.
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactions
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Changed the name and added the underscore
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactions
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Typo: transfert → transfer
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactions
Comment on lines
+99
to
+100
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactions
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Typo + may be longer than 78 char (PEP8).
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactions
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactions
Comment on lines
+549
to
+552
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should be handled by the parent class (
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactions
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is now moved to
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactions
Member
All reactions
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page.
Author
All reactions
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page.
|
There was an error while loading. Please reload this page.
|
coverage: 94.153% (-0.6%) from 94.734% |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
@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. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
@murrayrm it might be ok now, I just removed a forgotten debug print in |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.