QInfer / python-qinfer

Library for Bayesian inference via sequential Monte Carlo for quantum parameter estimation.
BSD 3-Clause "New" or "Revised" License
92 stars 31 forks source link

New derived model: GaussianRandomWalk #124

Closed ihincks closed 7 years ago

ihincks commented 7 years ago

This class is similar to RandomWalk except that it only supports zero-mean multivariate normal distributions, and it has the ability to automatically add model parameters which parameterize the covariance matrix, so that they can be learned in tandem with the parameters of interest.

See doc/guide/timedep.rst for a demo, which is worth more than what I can write here.

coveralls commented 7 years ago

Coverage Status

Coverage decreased (-13.08%) to 58.672% when pulling 4e732175c4355390e5bedee0c6519c4d839378a4 on ihincks:feature-gaussian-random-walk into 8118ab7adaf0bd82ea39577a08f2ad3b71bca1ae on QInfer:master.

coveralls commented 7 years ago

Coverage Status

Coverage increased (+1.4%) to 73.12% when pulling 81f147d042a4f402249c6307735d606801c36124 on ihincks:feature-gaussian-random-walk into 8118ab7adaf0bd82ea39577a08f2ad3b71bca1ae on QInfer:master.

coveralls commented 7 years ago

Coverage Status

Coverage increased (+1.4%) to 73.143% when pulling c20232c3aa03dd568759b03045624fb9e48f68cd on ihincks:feature-gaussian-random-walk into 8118ab7adaf0bd82ea39577a08f2ad3b71bca1ae on QInfer:master.

coveralls commented 7 years ago

Coverage Status

Coverage increased (+1.5%) to 73.236% when pulling 1c835acd7f88ecc9ced7e92f0ec24a70c9478d85 on ihincks:feature-gaussian-random-walk into 8118ab7adaf0bd82ea39577a08f2ad3b71bca1ae on QInfer:master.

ihincks commented 7 years ago

Thinking about it again, I think I have est_update_covariance wrong in the not-fixed-covariance case. I think I should be taking the mean over particles after computing the covariance matrix for each particle. Do you agree?

cgranade commented 7 years ago

For the docstring as written, I agree, est_update_covariance should take the mean after computing the covariance matrix. That said, it's also an operationally-relevant question to ask what the covariance for the estimated noise parameters are; if I understand correctly, that's the question answered by the current implementation of est_update_covariance?

coveralls commented 7 years ago

Coverage Status

Coverage increased (+1.5%) to 73.23% when pulling 31ef01b13804d5ff0b067c8d8982a302bcc3ce68 on ihincks:feature-gaussian-random-walk into 8118ab7adaf0bd82ea39577a08f2ad3b71bca1ae on QInfer:master.

ihincks commented 7 years ago

I think that should do it for est_update_covariance. I'm not exactly sure what the previous version corresponded to. It could correspond to what you are talking about, but it's not clear to me. In any case, all I wanted this function to do is to fill the gap of SMCUpdater.est_mean for the new model parameters, when one actually cares about the covariance matrix instead of the things that parameterize it.

ihincks commented 7 years ago

Any further concerns/requests on this PR?

cgranade commented 7 years ago

My apologies for falling behind. Anyway, I think this looks good so I'll go on and merge it in now. Thanks!