probml / JSL

Jax SSM Library
MIT License
50 stars 13 forks source link

Add back offset terms to LDS class to fix Kalman filtering and smoothing #46

Closed gileshd closed 2 years ago

gileshd commented 2 years ago

Replace LDS offset terms to fix kalman_filter.py

In the present form the kalman_step function from jls/lds/kalman_filter.py throws an error because it tries to access offset terms in the lds object which are not present. This causes jsl/demos/kalman_tracking.py to fail.

More specifically these get_*_offset_of methods are not present in the LDS class: https://github.com/probml/JSL/blob/b5f08e10f6cedea1d34123d383dcbb5468578710/jsl/lds/kalman_filter.py#L163 https://github.com/probml/JSL/blob/b5f08e10f6cedea1d34123d383dcbb5468578710/jsl/lds/kalman_filter.py#L172

I've tried to revert the appropriate recent changes so that these methods are once again present which seems to fix the jsl/demos/kalman_tracking.py demo.

It might be useful for someone to have a look to make sure that the changes I've made are alright @gerdm, @murphyk ?

gerdm commented 2 years ago

Hi @gileshd,

Is your PR branch considering the latest commit? I believe we already fix this. I ran the kf_tracking.py demo on the lates version of JSL to make sure (I suppose this is the demo you're referring to).

$ python jsl/demos/kf_tracking.py 
L2-filter: 3.2481
L2-smooth: 2.0450
murphyk commented 2 years ago

@gerdm The master branch is indeed missing these 2 functions. I think Giles's fix solves that. I will merge for now, but we really need some unit tests!

gileshd commented 2 years ago

Hi @gerdm,

I was working off b5f08e10f6cedea1d34123d383dcbb5468578710 it looks like everything was working in b50ac8103307ef7fa54dfe9f8c979744a67c131e but that perhaps the following commit, 72f210d77e58ce1d72fa4dbd36bde74c7ef63ddc, accidentally rolled some things back.

murphyk commented 2 years ago

Is the main brach working?

gileshd commented 2 years ago

Yes currently working for me (e494ddf879aad9fc4d3b7739b5d9ac12365fa440). The demo notebook runs fine.