Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Implement okid#1031

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Draft
KybernetikJo wants to merge2 commits intopython-control:main
base:main
Choose a base branch
Loading
fromKybernetikJo:implement_okid

Conversation

@KybernetikJo
Copy link
Contributor

@KybernetikJoKybernetikJo commentedJul 15, 2024•
edited
Loading

This PR implements okid=observer_kalman_identification.

The api should be the same or very similar to that ofct.markov.
An additional goal is forct.okid andct.era to work well together.

T=np.linspace(0,10,100)U=np.ones((1,100))T,Y=ct.forced_response(ct.tf([1], [1,0.5],True),T,U)H=ct.okid(Y,U,3)
T=np.linspace(0,10,100)U=np.ones((1,100))response=ct.forced_response(ct.tf([1], [1,0.5],True),T,U)H=ct.okid(response,3)

@coveralls
Copy link

coveralls commentedJul 15, 2024•
edited
Loading

Coverage Status

coverage: 94.127% (-0.6%) from 94.752%
when pulling5abbfe1 on KybernetikJo:implement_okid
intof73e893 on python-control:main.

@KybernetikJoKybernetikJoforce-pushed theimplement_okid branch 2 times, most recently from860acb7 to67d6ef0CompareJuly 16, 2024 09:47
@murrayrm
Copy link
Member

@KybernetikJo This PR needs to be rebased on main (and okid should be expanded toobserver_kalman_identification (orobserver_kalman_id to keep things a bit shorter?).

Copy link
Member

@murrayrmmurrayrm left a comment

Choose a reason for hiding this comment

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

Overall this looks good. Some stylistic issues that it would be useful to update for consistency with standard Python style. Perhaps also say something here (or in themarkov command) about the relationship betweenokid andmarkov?

-------
H : ndarray
First m Markov parameters, [D CB CAB ...].
Copy link
Member

Choose a reason for hiding this comment

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

Should we add a "See Also" section and referencemarkov? Perhaps also a note here (and inmarkov) about how this differs from themarkov command?

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

Would that be enough?

See Also

markov

Notes

The :func:~control.markov command estimates the Markov parameters directly, which can be hard for slightly damped systems.
The :func:~control.observer_kalman_identification command uses a Kalman filter, which is better suited for slightly damped systems.

@murrayrm
Copy link
Member

@KybernetikJo I will be doing a release of python-control in the coming days. If you have time to update this PR prior to that, we can include in v0.10.1. Otherwise, it can go in the next release.

@KybernetikJo
Copy link
ContributorAuthor

KybernetikJo commentedOct 19, 2024•
edited
Loading

@murrayrm

@KybernetikJo I will be doing a release of python-control in the coming days. If you have time to update this PR prior to that, we can include in v0.10.1. Otherwise, it can go in the next release.

Sorry for the late reply, I had no time at all.

State of okid:

  • I think code is OK, docstring is OK, example is OK!

But:

  • I have to think about the m parameter again. In the paper, the algorithm estimates (m+1) parameters, but it should be consistent with makrov's m parameters.
  • I am not satisfied with my unit tests, maybe someone has suggestions for improvement.

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@murrayrmmurrayrmmurrayrm left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@KybernetikJo@coveralls@murrayrm
[8]ページ先頭
©2009-2025 Movatter.jp