GPflow / GPflowOpt

Bayesian Optimization using GPflow
Apache License 2.0
270 stars 61 forks source link

MGP #60

Closed gpfins closed 7 years ago

gpfins commented 7 years ago

In this pull request I implemented the Approximatively Marginalised GP as described as indicated in issue #39 . It currently supports multi-output GPs. A notebook and some tests are included.

icouckuy commented 7 years ago

Awesome, thanks. I see that the Travis tests aren't triggered because of an external PR.

In init.py you removed the objective import by accident I think, as well as forgot to remove testmgp.py

But we will fix that once I moved the branch and created a new PR

javdrher commented 7 years ago

This is not to be included in the 0.1 release, preferable after tf 1.3 actual release

I suggest to fix things first, then create an in-house branch as @nknudde has no rights to push after that. Some things to consider:

codecov-io commented 7 years ago

Codecov Report

Merging #60 into master will increase coverage by 0.01%. The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #60      +/-   ##
==========================================
+ Coverage   99.78%   99.79%   +0.01%     
==========================================
  Files          16       17       +1     
  Lines         928      987      +59     
==========================================
+ Hits          926      985      +59     
  Misses          2        2
Impacted Files Coverage Δ
GPflowOpt/__init__.py 100% <100%> (ø) :arrow_up:
GPflowOpt/models.py 100% <100%> (ø)

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 a473070...b4c3615. Read the comment docs.

javdrher commented 7 years ago

Small general guideline: lets avoid rebases. For this PR I'm ok with it, and we won't start from scratch.

Rebases are actually very good but for normal circumstances I think they make the history confusing. its a very good tool for complex merge scenario with an enormous amount of conflicts expected as it allows you to resolve conflicts as they occur, commit by commit (think of a fork which has gone its own way for a while and then is merged). For normal scenarios a merge is preferred.

icouckuy commented 7 years ago

I'm also against rebases (at least I am now, some bad experiences). I thought it might be necessary to fix the commit history a bit as i thought it was corrupted due to rebranching, but it actually looks okay ( though there are several commits with the same name)