Hello @jonathanng! Thanks for updating the PR.

Cheers ! There are no PEP8 issues in this Pull Request. 🍻

Comment last updated on February 01, 2018 at 17:38 Hours UTC

Codecov Report

Merging #627 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #627      +/-   ##
==========================================
+ Coverage    86.6%   86.61%   +<.01%     
==========================================
  Files          23       23              
  Lines        1777     1778       +1     
==========================================
+ Hits         1539     1540       +1     
  Misses        238      238

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 20e46a7...1c72154. Read the comment docs.

I find that for large # of datapoints, GP is very slow. This pull request does not fix that. Anyone else seeing this issue?

I think the fact that GPs slow down as the number of data points increases is one of the drawbacks of GPs. I tried to find the relevant bit of the scikit-learn docs that explains the scaling behaviour but I couldn't find it.

Yup a normal thing for GP:

https://arxiv.org/pdf/1502.05700.pdf see Figure 1 there

@jonathanng maybe add this somewhere to the comments if you have time? Otherwise we could add it later

I still think having a control over n_jobs would be very beneficial, esp. if you run multiple optimizations at the same time in order to test for example the algorithm

Beyond the n_jobs docs thingy LGTM! Please add [MRG] to the beginning of the name of the PR if you are ready to merge, or [WIP] if you wish to work a bit more on this.