-
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
Extend tests for GPEmulator.jl and Observations.jl #62
Conversation
@@ -117,12 +117,12 @@ function GPObj(inputs, data, package::GPJL; GPkernel::Union{K, KPy, Nothing}=not | |||
GPkernel_i = deepcopy(GPkernel) | |||
# inputs: N_samples x N_parameters | |||
# data: N_samples x N_data | |||
logstd_obs_noise = log(sqrt(0.5)) # log standard dev of obs noise | |||
logstd_obs_noise = log(sqrt(0.01)) # log standard dev of obs noise |
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.
Should we make this dependent on the ensemble covariance? I also think that Ollie wanted to base this on the SVD stuff.
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.
Yes, we should! This will come in another (probably the next) PR though - the purpose of this one is merely to extend the test coverage. The change of the hard-coded regularization noise from log(sqrt(0.5))
to log(sqrt(0.01))
is just there because I experimented with it after our discussion and thought I could as well leave it at a smaller value until we have implemented the SVD (since the idea is that the regularization noise should be smaller than the learned white noise, a criterion that is more likely to be met the smaller the regularization noise is - speaking of which, one of the next PRs should also address the issue of naming all these noises (white noise, observational noise, regularization noise etc.) in a consistent way that minimizes confusion as much as possible).
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.
Sounds good. And yes, I think with all the covariances involved, being precise in the notation would be helpful.
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.
Aside from the algorithmic changes, this looks fine to me.
Update src/Observations.jl Co-authored-by: Charles Kawczynski <[email protected]>
4237680
to
a163957
Compare
@@ -117,12 +117,12 @@ function GPObj(inputs, data, package::GPJL; GPkernel::Union{K, KPy, Nothing}=not | |||
GPkernel_i = deepcopy(GPkernel) | |||
# inputs: N_samples x N_parameters | |||
# data: N_samples x N_data | |||
logstd_obs_noise = log(sqrt(0.5)) # log standard dev of obs noise | |||
logstd_obs_noise = log(sqrt(0.01)) # log standard dev of obs noise |
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.
Sounds good. And yes, I think with all the covariances involved, being precise in the notation would be helpful.
# Zero mean function | ||
kmean = MeanZero() | ||
m = GPE(inputs', dropdims(data[:, i]', dims=1), kmean, GPkernel_i, | ||
logstd_obs_noise) | ||
optimize!(m, noise=false) | ||
optimize!(m, noise=true) |
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.
FYI this is the default of optimize!
. I will try to read the documentation of GaussianProcesses.jl to see what would be the best setting for our purposes.
bors r+ |
Build succeeded: |
This PR extends the coverage of the tests for the modules
GPEmulator.jl
andObservations.jl
.