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

Made a kalman_filter method similar to kalman_smooth. #235

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Made a kalman_filter method similar to kalman_smooth. #235

wants to merge 5 commits into from

Conversation

murphyk
Copy link

@murphyk murphyk commented Feb 9, 2019

Added kalman_sample to generate from model.
Added example of 2d Kalman filtering and smoothing.

Added kalman_sample to generate from model.
Added example of 2d Kalman filtering and smoothing.
@sglyon
Copy link
Member

sglyon commented Feb 9, 2019

Thank you for your contribution!

Before we merge pull requests that add new library code, we ask that contributors write simple tests to ensure that their code functions as expected.

Would you be willing to do that?

I see that you provided an example (thank you!). Perhaps the example code could be re-used for the tests.

@murphyk
Copy link
Author

murphyk commented Feb 9, 2019

I have added some extra tests to test_kalman.jl. They pass locally on my laptop but Travis complains, I don't know why. (I am new to Julia and open source development...) I have temporarily commented out the new tests. Note that the call to smooth actually calls my new filter function, so is an indirect test of correctness :) But a direct test would be better. Would you mind telling me what I'm doing wrong?

@sglyon
Copy link
Member

sglyon commented Feb 22, 2019

Hey @murphyk from what I can tell the error on Travis has to do with a no method error when trying to call a Kalman instance like a function: https://travis-ci.org/QuantEcon/QuantEcon.jl/builds/491097872#L816

Does that sound familiar to you?

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