GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Uniform processing of time response and optimization parameters by murrayrm · Pull Request #1125 · python-control/python-control

Conversation

@murrayrm

@murrayrm murrayrm commented Feb 9, 2025

Copy link
Copy Markdown
Member

This PR regularizes the use of various keywords for time responses and optimization routines, while maintaining backward compatibility with existing code. The motivation for the PR is that over time we have introduced different terms to refer to parameters that have the same purpose. This can be confusing for people trying to use the package.

For example:

  • The named parameters x0 and X0 are used in different places for the initial state in simulation and optimization functions.
  • The parameter return_x is used when a function should return the state as part of a tuple, though sometime return_states is used instead.
  • Responses use the states property (not x) and setting the the state names is done via states.
  • Similar issues arise for inputs (u0, U, inputs) and outputs (y0, outputs).
  • Time points for simulation and optimization are sometimes referred to as timepts but other times as T.
  • In the optimization and flatness modules, cost functions are referred to as cost, trajectory_cost or integral cost, depending on the function, and constraints are referred to as trajectory_constraints in some places as constraints in others.
  • Other parameters have names that don't seem to fit existing python-control naming patterns: t_eval, yfinal, etc.

To address these issues, this PR introduces a common set of terms across various time response and optimal control functions, with the following more consistent usage patterns and functionality:

  • Parameters specifying the inputs, outputs, and states are referred to as inputs, outputs, and states consistently throughout the functions.
  • Variables associated with inputs, outputs, states and time use those words plus an appropriate modifier: initial_state, final_output, input_indices etc.
  • Aliases are used both to maintain backward compatibility and to allow shorthand descriptions: e.g., U, Y, X0. Short form aliases are documented in docstrings by listing the parameter as long_form (or sf) : type.
  • Existing legacy keywords are allowed and generate a PendingDeprecationWarning.
  • Specifying a parameter value in two different ways (eg, via long form and an alias) generates a TypeError.

The implementation is done in a manner that can later be utilized for other functions where we want to have aliases and legacy keys, as documented in the Developer Notes section of the Reference Manual:

[P]arameter names are generally longer strings that describe the purpose fo the paramater. Similar to matplotlib (e.g., the use of lw as an alias for linewidth), some commonly used parameter names can be specified using an “alias” that allows the use of a shorter key.

Named parameter and keyword variable aliases are processed using the config._process_kwargs() and config._process_param() functions. These functions allow the specification of a list of aliases and a list of legacy keys for a given named parameter or keyword. To make use of these functions, the _process_kwargs() is first called to update the kwargs variable by replacing aliases with the full key:

_process_kwargs(kwargs, aliases)

The values for named parameters can then be assigned to a local variable using a call to process_param() of the form:

var = _process_kwargs('param', param, kwargs, aliases)

where param is the named parameter used in the function signature and var is the local variable in the function (may also be param, but doesn’t have to be).

...

The alias mapping is a dictionary that returns a tuple consisting of valid aliases and legacy aliases:

alias_mapping = {
    'argument_name_1', (['alias', ...], ['legacy', ...]),
     ...}

If an alias is present in the dictionary of keywords, it will be used to set the value of the argument. If a legacy keyword is used, a warning is issued.

Two primary tables have been created in this PR. The first is a
dictionary of aliases and legacy names for time response commands (from timeresp.py):

_timeresp_aliases = {
    # param:            ([alias, ...], [legacy, ...])
    'timepts':          (['T'],        []),
    'inputs':           (['U'],        ['u']),
    'outputs':          (['Y'],        ['y']),
    'initial_state':    (['X0'],       ['x0']),
    'final_output':     (['yfinal'],   []),
    'return_states':    (['return_x'], []),
    'evaluation_times': (['t_eval'],   []),
    'timepts_num':      (['T_num'],    []),
    'input_indices':    (['input'],    []),
    'output_indices':   (['output'],   []),
}

The second is a dictionary of aliases for optimal control functions (from optimal.py):

_optimal_aliases = {
    # param:                  ([alias, ...],                [legacy, ...])
    'integral_cost':          (['trajectory_cost', 'cost'], []),
    'initial_state':          (['x0', 'X0'],                []),
    'initial_input':          (['u0', 'U0'],                []),
    'final_state':            (['xf'],                      []),
    'final_input':            (['uf'],                      []),
    'initial_time':           (['T0'],                      []),
    'trajectory_constraints': (['constraints'],             []),
    'return_states':          (['return_x'],                []),
}

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
murrayrm added 4 commits February 8, 2025 16:25
@murrayrm
first pass at uniform argument names for time responses (+ aliases)
8e16fd6
@murrayrm
fix up ruff + errors from example
a6f970e
@murrayrm
update optimal and flatsys to use consistent parameter names
17b6978
@murrayrm
add developer notes
fc9fadb
@coveralls

coveralls commented Feb 9, 2025

Copy link
Copy Markdown

Coverage Status

coverage: 94.715% (-0.04%) from 94.751%
when pulling fc9fadb on murrayrm:keyword_aliases-07Feb2025
into cf77f99 on python-control:main.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@slivingston
slivingston self-requested a review February 10, 2025 03:10
slivingston
slivingston approved these changes Feb 10, 2025
View reviewed changes

@slivingston slivingston left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Good work! There are still several places where _process_legacy_keyword() is called (statefbk.py, pzmap.py, ...), but we can change those later.

I marked several small items to correct.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread doc/develop.rst Outdated
-----------------

As described above, parameter names are generally longer strings that
describe the purpose fo the paramater. Similar to `matplotlib` (e.g.,

@slivingston slivingston Feb 10, 2025

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Suggested change
describe the purpose fo the paramater. Similar to `matplotlib` (e.g.,
describe the purpose for the parameter. Similar to `matplotlib` (e.g.,

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread doc/develop.rst Outdated

var = _process_kwargs('param', param, kwargs, aliases)

where 'param` is the named parameter used in the function signature

@slivingston slivingston Feb 10, 2025

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Suggested change
where 'param` is the named parameter used in the function signature
where `param` is the named parameter used in the function signature

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread control/config.py Outdated
param = _process_param('param', defval, kwargs, function_aliases)

If `param` is a variable keyword argument (in `kwargs`), `defval` can
be pssed as either None or the default value to use if `param` is not

@slivingston slivingston Feb 10, 2025

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Suggested change
be pssed as either None or the default value to use if `param` is not
be passed as either None or the default value to use if `param` is not

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread control/config.py Outdated
defval : object or dict
Default value for the parameter.
kwargs : dict
Dictionary of varaible keyword arguments.

@slivingston slivingston Feb 10, 2025

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Suggested change
Dictionary of varaible keyword arguments.
Dictionary of variable keyword arguments.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread control/config.py
Raises
------
TypeError
If multiple keyword aliased are used for the same parameter.

@slivingston slivingston Feb 10, 2025

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Suggested change
If multiple keyword aliased are used for the same parameter.
If multiple keyword aliases are used for the same parameter.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread control/optimal.py Outdated
self.states = response.states


# Parameter and keyword aliases

@slivingston slivingston Feb 10, 2025

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

This dictionary is better placed near the top of the file, next to other module-level variables like _optimal_defaults. The motivation for this practice is clarity: the variable has module-level scope, so it is in-scope for functions defined before it, too. In my experience, it is easier to have all of these variables in the same part of file, just after imports.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread control/timeresp.py Outdated
return ControlPlot(lines, cplt.axes, cplt.figure)

# Dictionary of aliases for time response commands
_timeresp_aliases = {

@slivingston slivingston Feb 10, 2025

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Same comment here as for _optimal_aliases: I recommend to move the definition of _timeresp_aliases to the top of the file, just after __all__.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread control/tests/timeresp_test.py

# Aliases
resp_short = ct.input_output_response(sys, timepts, 1, X0=[1, 1])
np.testing.assert_allclose(resp_long.states, resp_posn.states)

@slivingston slivingston Feb 10, 2025

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Suggested change
np.testing.assert_allclose(resp_long.states, resp_posn.states)
np.testing.assert_allclose(resp_short.states, resp_posn.states)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread control/tests/docstrings_test.py

elif not (match := re.search(
"\n" + r"((\w+|\.{3}), )*" + argname + r"(, (\w+|\.{3}))* :",
"\n" + r"((\w+|\.{3}), )*" + argname_ + r"(, (\w+|\.{3}))* :",

@slivingston slivingston Feb 10, 2025

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

There is a search for duplicates below on line 658. Should argname there also be changed to argname_ ?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread control/tests/timeresp_test.py
# Legacy
with pytest.warns(PendingDeprecationWarning, match="legacy"):
resp_legacy = ct.input_output_response(sys, timepts, 1, x0=[1, 1])
np.testing.assert_allclose(resp_long.states, resp_posn.states)

@slivingston slivingston Feb 10, 2025

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Suggested change
np.testing.assert_allclose(resp_long.states, resp_posn.states)
np.testing.assert_allclose(resp_legacy.states, resp_posn.states)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@murrayrm
address @slivingston comments + pycodestyle updates
3b4e3a4
@murrayrm
murrayrm merged commit a042895 into python-control:main Feb 12, 2025
@murrayrm murrayrm added this to the 0.10.2 milestone Feb 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@slivingston slivingston slivingston approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

0.10.2

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@murrayrm @coveralls @slivingston
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.