QuantEcon.py icon indicating copy to clipboard operation
QuantEcon.py copied to clipboard

Getting rid of the LinearStateSpace argument in Kalman

Open natashawatkins opened this issue 6 years ago • 8 comments

Wouldn't it be easier to just have the arguments that a LinearStateSpace object takes available when creating a Kalman object? It's somewhat cumbersome to first create the LSS object and then put that into Kalman (you also need to import two classes).

We could just create a LSS object using the arguments in Kalman.

@jstac

natashawatkins avatar Oct 20 '17 16:10 natashawatkins

Yeah, fair point. I had a look at the code and the Kalman class never actually uses any of the functionality of the LinearStateSpace class internally. So from a coding perspective there's no advantage linking them.

Plus, as you say, if we do need such functionality down the road we can always build an instance of LSS inside the class.

Could you please put together a PR that closes this issue? We'll also need to coordinate on changing the lecture. Although that shouldn't happen until the next release of the library. (CC @mmcky )

jstac avatar Oct 22 '17 22:10 jstac

Thanks @jstac and @natashawatkins. I was thinking of putting a release together but I will do so after this is resolved.

mmcky avatar Oct 22 '17 22:10 mmcky

I also think this is a reasonable thing to do.

If you wanted to make sure you going from LSS to Kalman was low cost then one natural thing to do would be to add a method to LSS that created a Kalman object.

cc7768 avatar Oct 23 '17 14:10 cc7768

Lets talk with John before changing this please. It makes sense not to change it. Tom

On Oct 23, 2017 9:40 AM, "Chase Coleman" [email protected] wrote:

I think this is a reasonable thing to do.

If you wanted to make sure you going from LSS to Kalman was low cost then one natural thing to do would be to add a method to LSS that created a Kalman object.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/QuantEcon/QuantEcon.py/issues/354#issuecomment-338681143, or mute the thread https://github.com/notifications/unsubscribe-auth/AD22Tja44_AD_jKMYgQKnfS8vwsk3Fotks5svKVSgaJpZM4QA4-8 .

thomassargent30 avatar Oct 23 '17 15:10 thomassargent30

Hi @thomassargent30, I think @natashawatkins has a good point here: To build an instance of Kalman you need to know about LinearStateSpace. This makes sense on a mathematical level but forms a barrier to entry when coding, since users need to look up the documentation for two classes.

If in the future we need to use the functionality of LinearStateSpace inside Kalman, we can build an instance of LinearStateSpace inside Kalman, rather than outside.

So on balance I support the change proposed by @natashawatkins.

jstac avatar Oct 23 '17 18:10 jstac

Go with what John and Natasha think is best. There will be some general equilibrium effects down the line because we have built some notebooks (in the pipeline) about the Kalman filter that use the current protocol.

On Mon, Oct 23, 2017 at 11:07 AM, John Stachurski [email protected] wrote:

Hi @thomassargent30 https://github.com/thomassargent30, I think @natashawatkins https://github.com/natashawatkins has a good point here: To build an instance of Kalman you need to know about LinearStateSpace. This makes sense on a mathematical level but forms a barrier to entry when coding, since users need to look up the documentation for two classes.

If in the future we need to use the functionality of LinearStateSpace inside Kalman, we can build an instance of LinearStateSpace inside Kalman, rather than outside.

So on balance I support the change proposed by @natashawatkins https://github.com/natashawatkins.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/QuantEcon/QuantEcon.py/issues/354#issuecomment-338747015, or mute the thread https://github.com/notifications/unsubscribe-auth/AD22Tv6sPH7FYqWxRaD0t_Cs7x6HWwKYks5svNXvgaJpZM4QA4-8 .

thomassargent30 avatar Oct 24 '17 01:10 thomassargent30

We might be able to use a decorator to keep backward compatibility. If an LSS object is passed we could then unwrap the object and pass in the data (from LSS). Working out the interface will need some thought. @natashawatkins would you intend for A, C, G, H=None, mu_0=None, Sigma_0=None to be passed into Kalman instead?

mmcky avatar Oct 24 '17 02:10 mmcky

@mmcky Thanks, great suggestion.

@thomassargent30 This is a neat suggestion that will allow us to initialize the class with either the A, B, C matrices (for ease of use) or a corresponding LinearStateSpace instance (to exploit the fact that Kalman filters build on linear state space models). With such an approach there will be no negative equilibrium effects for notebooks in the pipeline.

jstac avatar Oct 24 '17 02:10 jstac