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

MGP #65

Open
wants to merge 60 commits into
base: master
Choose a base branch
from
Open

MGP #65

wants to merge 60 commits into from

Conversation

javdrher
Copy link
Member

In-house version of #60 . Full credits to @nknudde for implementing this.

I already took care of merging #64 and derived the MGP from it. Also introduced a tf_wraps file for the rowwise_gradients. To be merged after releasing.

@javdrher javdrher added this to the 0.2.0 release milestone Aug 10, 2017
@codecov-io
Copy link

codecov-io commented Aug 10, 2017

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff            @@
##           master     #65      +/-   ##
=========================================
+ Coverage   99.78%   99.8%   +0.01%     
=========================================
  Files          16      18       +2     
  Lines         928    1004      +76     
=========================================
+ Hits          926    1002      +76     
  Misses          2       2
Impacted Files Coverage Δ
GPflowOpt/acquisition/ei.py 100% <ø> (ø) ⬆️
GPflowOpt/__init__.py 100% <100%> (ø) ⬆️
GPflowOpt/acquisition/acquisition.py 100% <100%> (ø) ⬆️
GPflowOpt/models.py 100% <100%> (ø)
GPflowOpt/scaling.py 100% <100%> (ø) ⬆️
GPflowOpt/tf_wraps.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...c2764eb. Read the comment docs.

icouckuy
icouckuy previously approved these changes Aug 14, 2017
from . import pareto
from . import models
Copy link
Contributor

Choose a reason for hiding this comment

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

import models twice?

N = tf.shape(fmean)[0]
D = tf.shape(fmean)[1]

fmeanf = tf.reshape(fmean, [N * D, 1]) # N*D x 1
Copy link
Contributor

Choose a reason for hiding this comment

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

N x D x 1

:)

Copy link
Contributor

Choose a reason for hiding this comment

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

No, has to be N*D x 1 so I can use rowwise_gradients, then I reshape later

@AutoFlow((float_type, [None, None]))
def predict_f(self, Xnew):
"""
Compute the mean and variance of the latent function(s) at the points
Copy link
Contributor

Choose a reason for hiding this comment

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

update doc string? marginalised around ...

Compute the mean and variance of the latent function(s) at the points
Xnew.
"""
theta = self._predict_f_AF_storage['free_vars']
Copy link
Contributor

Choose a reason for hiding this comment

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

ugh

Copy link
Contributor

Choose a reason for hiding this comment

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

No other way, have to wait for GPflow issue

@icouckuy
Copy link
Contributor

I skipped the ModelWrapper class in my review as it is a separate PR #64

@javdrher
Copy link
Member Author

I'd like to comment that this isnt to be merged until GPflow/GPflow#480 is addressed, or we hack some way around it.

@javdrher javdrher removed this from the 0.2.0 release milestone Aug 27, 2017
Copy link
Contributor

@icouckuy icouckuy left a comment

Choose a reason for hiding this comment

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

@icouckuy icouckuy dismissed their stale review September 14, 2017 19:01

mgp is not compatible with GPflow

@icouckuy
Copy link
Contributor

@nknudde what's the state on this? Is it more compatible with GPflow 1.0?

Or is using MCMC to marginalize hyperparameters fast and accurate enough?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants