Skip to content
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

GP bug for noise_learn = false underprediction of uncertainty #143

Merged
merged 3 commits into from
Jun 1, 2022

Conversation

odunbar
Copy link
Collaborator

@odunbar odunbar commented May 28, 2022

Purpose

Fix the noise_learn = false bug greatly underpredicting uncertainty. In Lorenz example, the following two should be similar. with noise_learn=false and true:

co-author with @lm2612

In this PR

  • Initial bugfix to correct the regularization noise vs white kernel
  • Tightened runtests thanks to improved prediction,
  • new alg_reg_noise optional argument to GP to set the regularization when noise_learn=true (removing magic_number)
  • Lorenz example - with SKLJL() option

@codecov
Copy link

codecov bot commented May 28, 2022

Codecov Report

Merging #143 (48598e3) into master (ad38cb5) will increase coverage by 0.55%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #143      +/-   ##
==========================================
+ Coverage   88.15%   88.71%   +0.55%     
==========================================
  Files           4        4              
  Lines         380      381       +1     
==========================================
+ Hits          335      338       +3     
+ Misses         45       43       -2     
Impacted Files Coverage Δ
src/GaussianProcess.jl 93.00% <100.00%> (+2.09%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ad38cb5...48598e3. Read the comment docs.

@odunbar odunbar requested a review from lm2612 May 28, 2022 00:32
@odunbar
Copy link
Collaborator Author

odunbar commented May 28, 2022

bors try

bors bot added a commit that referenced this pull request May 28, 2022
@bors
Copy link
Contributor

bors bot commented May 28, 2022

@lm2612
Copy link
Collaborator

lm2612 commented Jun 1, 2022

Looks good with SKLJL.
When I test with GPJL and noise_learn=false, I'm finding the predicted covariance matrix y_var to now be around 2x Γy. Is it possible that this was already included for GPJL?

@odunbar
Copy link
Collaborator Author

odunbar commented Jun 1, 2022

The predicted covariance (when returned with transform_to_real=true) will be the sum of 2 contributions

  1. one from the GP kernel approximation (e.g. the fact that we use an RBF to approximate something)
  2. one from the observational noise.

So in theory the y_var must be >= observational noise. The amount that it is bigger will be due to the error in the GP approximation.

does it return something similar with noise_learn = true? This will fix contribution 2. = observational noise, and only learns contribution 1. If they are similar, this would indicate that it's just not a great approximation if they are not similar then maybe there is an issue (note they are unlikely to be exactly the same).

I did try to do something in the unit tests like this - (see the added file)

@odunbar
Copy link
Collaborator Author

odunbar commented Jun 1, 2022

Ignore what i said: from the GaussianProcess.jl source code I found this:
thus they add the "lognoise" (i.e. the regularization noise) back in during prediction

function predict_y(gp::GPE, x::AbstractMatrix; full_cov::Bool=false)
    μ, σ2 = predict_f(gp, x; full_cov=full_cov)
    if full_cov
        npred = size(x, 2)
        return μ, σ2 + ScalMat(npred, exp(2*gp.logNoise))
    else
        return μ, σ2 .+ exp(2*gp.logNoise)
    end
end

Thanks! I will update accordingly

@lm2612
Copy link
Collaborator

lm2612 commented Jun 1, 2022

Ok great! I forgot to say in my previous comment I was referring to the Lorenz example

@odunbar
Copy link
Collaborator Author

odunbar commented Jun 1, 2022

OK

  • I've moved the magic_number to be an input argument for GP.
  • I've tightened up the new unit tests, so now predicted means, and variances, with noise_learn and without have to be within 5% of each other (was 50% before - hence why it missed the bug!)

Copy link
Collaborator

@lm2612 lm2612 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, I get virtually the same results for both GPJL and SKLJL now, thanks!

@odunbar
Copy link
Collaborator Author

odunbar commented Jun 1, 2022

bors r+

@bors
Copy link
Contributor

bors bot commented Jun 1, 2022

@bors bors bot merged commit a073451 into master Jun 1, 2022
@bors bors bot deleted the orad/bugfix-gp-reg branch June 1, 2022 19:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants