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

WIP: Update Kalman Interface to accept setup matrices directly (in addition to LSS) #368

Closed
wants to merge 1 commit into from

Conversation

mmcky
Copy link
Contributor

@mmcky mmcky commented Oct 26, 2017

a proof of concept trial for accepting multiple interfaces for Kalman object,

Note: no updated docstrings yet. Setting up in Branch for collaboration.

This would allow kalman to accept the lecture example:

# == parameters == #
theta = 10  # Constant value of state x_t
A, C, G, H = 1, 0, 1, 1
ss = LinearStateSpace(A, C, G, H, mu_0=theta)

# == set prior, initialize kalman filter == #
x_hat_0, Sigma_0 = 8, 1
kalman = Kalman(ss, x_hat_0, Sigma_0)

in addition to

# == parameters == #
theta = 10  # Constant value of state x_t
A, C, G, H = 1, 0, 1, 1
x_hat_0, Sigma_0 = 8, 1
kalman = Kalman(A, C, G, H, mu_0=theta, x_hat_0, Sigma_0)

the key assumption is to check if the first element of the arguments list is an LSS object then it init's the object class (as per previously implemented) otherwise it constructs an LSS object internally with the matrices and then set's up using an LSS object.

@natashawatkins is this what you had in mind for #354? I think you were suggesting to replace the acceptance of LSS altogether but the benefit here is that old code won't break and Kalman will continue to work with those packaging up A, C, and G in an LSS object.

The downside to this approach is it makes the introspected interface to Kalman harder to read due to accepting arbitrary arguments and keyword arguments to parse for various init methods.

The key assumption here is that the argument list is checked and if the first element of the passed argument list is type LinearStateSpace then _init_lss is used to populate the object otherise it is assumed that a user has passed A, C, and G matrices.

@mmcky mmcky requested a review from natashawatkins October 26, 2017 00:35
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.1%) to 94.688% when pulling eb2f677 on kalman-spec into ad6be3e on master.

@mmcky mmcky added the in-work label Oct 26, 2017
@natashawatkins
Copy link
Member

Yeah I was thinking of just getting rid lss argument

I'm not sure - it kind of overcomplicates the code, and I think our library generally is useful as template

But it is handy in case someone wants to use both a linear state space and the kalman object.

@mmcky
Copy link
Contributor Author

mmcky commented Jun 6, 2018

I am cleaning up some old PR's and it doesn't seem like this is that useful. It is probably best to choose either matrix or LSS entry point to reduce complexity of the code for maintenance. I propose to close this without merge.

@mmcky mmcky closed this Jan 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants