search

JuliaStats / TimeModels.jl

Modeling time series in Julia
Other
57 stars 28 forks source link

Rework tests and fix smoothing code #38

Closed rob-luke closed 9 years ago

rob-luke commented 9 years ago

There appears to be a bug that running simulate twice avoided. It appears to be related to #34 .

Also removed loglik test as the true value was taken from the julia code itself.

rob-luke commented 9 years ago

@mschauer does this address issue #35 ?

mschauer commented 9 years ago

hi @codles, you can directly compare with https://gist.github.com/mschauer/3ffebdb8581f56341a9d, which is tested against a matlab kalman filter package, http://www.cs.ubc.ca/~murphyk/Software/Kalman/kalman.html

milktrader commented 9 years ago

Let me know if you guys want this merged with failing tests. We'll need passing tests to bump a new version though.

GordStephen commented 9 years ago

Came across a few more bugs in the smoother (see above) while writing some new tests - rather than have duelling PRs I thought we could just make the changes here?

It might just be because it's late, but that double-simulate bug really has me stumped... If @codles could fix the remaining smoother issues I'd be fine with going ahead and merging this PR - I'd rather have a fixed smoother and a failing test (for now) than vice versa?

rob-luke commented 9 years ago

Thanks for taking a look @GordStephen. I made the changes you suggested and added a test to ensure the filter and smoother return the same size results (third bug you spotted).

I also think merging the failing test is a good idea. I commented out that one line and everything else passes.

GordStephen commented 9 years ago

Awesome, thanks! I'll give @milktrader the last word but pulling this gets a :+1: from me.

rob-luke commented 9 years ago

Sorry that's my last addition @GordStephen. Ready to merge if you think its ok @milktrader.

milktrader commented 9 years ago

Feel free to do so yourself in the future @GordStephen since you're clearly taking a careful look at the code. :smile:

Once we get passing tests, it would be good to tag a new version and bump it to METADATA.