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

Add ab04md #201

Merged
merged 8 commits into from
Aug 24, 2023
Merged

Add ab04md #201

merged 8 commits into from
Aug 24, 2023

Conversation

KybernetikJo
Copy link
Contributor

@KybernetikJo KybernetikJo commented Jul 20, 2023

Add ab04md

TODO:

  • add docstring
  • add unit tests
  • check behavior: intent(in, out, copy, ...), see Bugfix ab13bd #200
import slycot
import numpy as np

A = np.array([[1.0, 0.5],
              [0.5, 1.0]])
B = np.array([[0.0, -1.0],
              [1.0, 0.0]])
C = np.array([[-1.0, 0.0],
              [0.0, 1.0]])
D = np.array([[1.0, 0.0],
              [0.0, -1.0]])

Ad = np.array([[-1.0, -4.0],
               [-4.0, -1.0]])
Bd = np.array([[2.8284, 0.0],
               [0.0, -2.8284]])
Cd = np.array([[0.0, 2.8284],
               [-2.8284, 0.0]])
Dd = np.array([[-1.0, 0.0],
               [0.0, -3.0]])

n = 2
m = 2
p = 2

Ad_t, Bd_t, Cd_t, Dd_t = slycot.ab04md('C',n,m,p,A,B,C,D)

print("Ad_t = \n", Ad_t)
print("Bd_t = \n", Bd_t)
print("Cd_t = \n", Cd_t)
print("Dd_t = \n", Dd_t)

Ac_t, Bc_t, Cc_t, Dc_t = slycot.ab04md('D',n,m,p,Ad_t,Bd_t,Cd_t,Dd_t)

print("Ac_t = \n", Ac_t)
print("Bc_t = \n", Bc_t)
print("Cc_t = \n", Cc_t)
print("Dc_t = \n", Dc_t)

print(np.allclose(A,Ac_t))
print(np.allclose(B,Bc_t))
print(np.allclose(C,Cc_t))
print(np.allclose(D,Dc_t))

output

Ad_t = 
 [[-1. -4.]
 [-4. -1.]]
Bd_t = 
 [[-2.82842712 -0.        ]
 [-0.          2.82842712]]
Cd_t = 
 [[-0.          2.82842712]
 [-2.82842712  0.        ]]
Dd_t = 
 [[3. 0.]
 [0. 1.]]
Ac_t = 
 [[1.  0.5]
 [0.5 1. ]]
Bc_t = 
 [[ 0. -1.]
 [ 1. -0.]]
Cc_t = 
 [[-1.  0.]
 [-0.  1.]]
Dc_t = 
 [[ 1.  0.]
 [ 0. -1.]]
True
True
True
True

@KybernetikJo
Copy link
Contributor Author

I have added the docstring, 2 unittests and discussed f2py behavior in #200. I would also keep here f2py intent(in,out,copy) for the state space matrices A,B,C,D.

Remark:
There is quite a bit of variation in the style of docstrings and unittests in slycot.

@KybernetikJo KybernetikJo marked this pull request as ready for review July 29, 2023 17:44
slycot/analysis.py Outdated Show resolved Hide resolved
@@ -13,7 +13,7 @@
# import slycot.examples

# Analysis routines (15/40 wrapped)
from .analysis import ab01nd, ab05md, ab05nd, ab07nd, ab08nd, ab08nz
from .analysis import ab01nd, ab04md, ab05md, ab05nd, ab07nd, ab08nd, ab08nz
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please increase the count from above.

A reformat of the whle ab block seems in order, too

Copy link
Collaborator

Choose a reason for hiding this comment

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

@bnavigator
Copy link
Collaborator

Sorry for not being responsive as usual. I made a few remarks about formal things.

@KybernetikJo
Copy link
Contributor Author

I have incorporated the suggestions and tried to improve the docstrings.
However, the .pfy, .py files and the docstring are very different in slycot in general and specifically in the analysis module.

I think we would need templates to unify the docstrings of all routines to meet the numpydoc specification.
Currently, only a few routines like ab01nd meet the numpydoc specification.

@@ -139,6 +139,79 @@ def ab01nd(n, m, A, B, jobz='N', tol=0, ldwork=None):
Z = None
return Ac, Bc, ncont, indcon, nblk, Z, tau

def ab04md(type_t, n, m, p, A, B, C, D, alpha=1.0, beta=1.0, ldwork=None):
""" At,Bt,Ct,Dt = ab04md(type_bn, n, m, p, A, B, C, D, [alpha, beta,ldwork])
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
""" At,Bt,Ct,Dt = ab04md(type_bn, n, m, p, A, B, C, D, [alpha, beta,ldwork])
""" At,Bt,Ct,Dt = ab04md(type_t, n, m, p, A, B, C, D, [alpha, beta, ldwork])

D : (p,m) ndarray
The leading p-by-m part of this array must contain the system direct
transmission matrix D.
alpha : double
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
alpha : double
alpha : double, optional

from numpy.testing import assert_equal, assert_allclose


class test_ab04md(unittest.TestCase):
Copy link
Collaborator

Choose a reason for hiding this comment

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

unittest.TestCase not needed

@bnavigator bnavigator merged commit 4401437 into python-control:master Aug 24, 2023
51 checks passed
@bnavigator bnavigator added this to the 0.6.0 milestone Aug 26, 2023
@KybernetikJo KybernetikJo deleted the add_ab04md branch January 6, 2024 20:01
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.

2 participants