-
Notifications
You must be signed in to change notification settings - Fork 17
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
Inflation #233
Inflation #233
Conversation
794c4b8
to
ba57c84
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your great work!
Is "stochastic_scaling_factor" N_batch/N ?
Could you also add your test in the PR?
Yes, ideally the user would set |
ba57c84
to
0296e16
Compare
61ed7b0
to
25dfef8
Compare
25dfef8
to
dfc36d6
Compare
dfc36d6
to
a282c89
Compare
516ad0e
to
40d53ee
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @costachris this looks great - I have left comments mainly on tightening up the docs and tests.
Sorry for some reason I added them as extra comments rather than a review!
PS could you also reference issue #230 in the PR
40d53ee
to
8b71248
Compare
b3a0c6f
to
c23f60c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great,
I have 2 more tiny comments, and could you add the quick get_localizer
in the test/Localizers/runtests.jl
that codecov is complaining about. Then happy for merge
bffdec8
to
eafbe11
Compare
@odunbar I included the option for using the prior covariance and updated the tests as discussed (including one for prior cov based noise). The docs now state more clearly how the covariance changes. |
2a2ea75
to
6cd2d01
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
6cd2d01
to
f9df438
Compare
bors r+ |
Build succeeded: |
Purpose
This PR adds the functionality of performing stochastic parameter updates in the form of multiplicative and additive inflation.
To-do
Make stochastic update the default.