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.

Sign up for GitHub

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

Jump to bottom

predict_f and predict_y as NamedTuple #1657

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from
Open

predict_f and predict_y as NamedTuple #1657

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
05873f7
MeanAndVariance as NamedTuple
antonykamp Mar 22, 2021
b318676
Added test about backwards compatibility
antonykamp Mar 22, 2021
35e3617
Added hint in docs, format
antonykamp Mar 22, 2021
c737f20
Implement review I
antonykamp Mar 30, 2021
9a0ad7e
add overload and Literal operator
antonykamp Apr 28, 2021
1bafd34
Add documentation
antonykamp May 4, 2021
415bad7
remove MeanAndCovariance from predict_y
antonykamp May 4, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions doc/source/notebooks/basics/regression.pct.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,9 @@
_ = plt.xlim(-0.1, 1.1)


# %% [markdown]
# Moreover, you can get the mean and variance data individually for example as `m.predict_f(xx).mean` instead of `m.predict_f(xx)[0]`.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# Moreover, you can get the mean and variance data individually for example as `m.predict_f(xx).mean` instead of `m.predict_f(xx)[0]`.
# Moreover, you can get the mean and variance predictions individually, using for `m.predict_f(xx).mean` instead of `m.predict_f(xx)[0]` and `m.predict_f(xx).variance` instead of `m.predict_f(xx)[1]`.

(note- if we rename the variance field we'll have to update this)


# %% [markdown]
# ## GP regression in higher dimensions
#
Expand Down
2 changes: 1 addition & 1 deletion gpflow/models/gplvm.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ def predict_f(
)
shape = tf.stack([1, tf.shape(Y_data)[1]])
var = tf.tile(tf.expand_dims(var, 1), shape)
return mean + self.mean_function(Xnew), var
return MeanAndVariance(mean + self.mean_function(Xnew), var)

def predict_log_density(self, data: OutputData) -> tf.Tensor:
raise NotImplementedError
2 changes: 1 addition & 1 deletion gpflow/models/gpmc.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,4 +106,4 @@ def predict_f(self, Xnew: InputData, full_cov=False, full_output_cov=False) -> M
mu, var = conditional(
Xnew, X_data, self.kernel, self.V, full_cov=full_cov, q_sqrt=None, white=True
)
return mu + self.mean_function(Xnew), var
return MeanAndVariance(mu + self.mean_function(Xnew), var)
2 changes: 1 addition & 1 deletion gpflow/models/gpr.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,4 +115,4 @@ def predict_f(
kmn, kmm_plus_s, knn, err, full_cov=full_cov, white=False
) # [N, P], [N, P] or [P, N, N]
f_mean = f_mean_zero + self.mean_function(Xnew)
return f_mean, f_var
return MeanAndVariance(f_mean, f_var)
11 changes: 8 additions & 3 deletions gpflow/models/model.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
# limitations under the License.

import abc
from typing import Optional, Tuple
from typing import NamedTuple, Optional

import tensorflow as tf

Expand All @@ -25,7 +25,12 @@
from ..utilities import to_default_float
from .training_mixins import InputData, RegressionData

MeanAndVariance = Tuple[tf.Tensor, tf.Tensor]

class MeanAndVariance(NamedTuple):
""" NamedTuple to access mean- and variance-function separately """
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
""" NamedTuple to access mean- and variance-function separately """
""" NamedTuple that holds mean and variance as named fields """


mean: tf.Tensor
variance: tf.Tensor
Copy link
Member

Choose a reason for hiding this comment

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

More of a general question at other maintainers (@vdutor @markvdw @awav) & anyone else ...: what should the name of this field be? In code we often abbreviate the variance to "var", but also it sometimes represents the covariance... or maybe that should be a different type? MeanAndVariance and MeanAndCov (and return a different one depending on full_cov etc)?

Copy link
Contributor

Choose a reason for hiding this comment

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

i like the idea of the two types. Haven't thought about it in detail

Copy link
Contributor

Choose a reason for hiding this comment

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

do you always know for sure if you've got the cov or var?

Copy link
Member

Choose a reason for hiding this comment

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

If you pass in full_cov=False and full_output_cov=False, you get the marginals back. If one of full_cov or full_output_cov is True, you get the covariance over inputs or outputs, respectively. If both are True, you should get the N P x N P covariance matrix (though this combination isn't actually implemented in several cases, I believe). So the output type is solely determined by the full_cov and full_output_cov arguments.

Copy link
Contributor

Choose a reason for hiding this comment

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

btw you can use typing.overload and typing.Literal to give more information than just MeanAndVariance | MeanAndCov, if you wanted to do that

Copy link
Author

Choose a reason for hiding this comment

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

I've created a small example, which should work as wanted:
on_hover
on_space

But I' not comfortable with the if-else statement :/

from typing import Literal, NamedTuple, overload

class MeanAndVariance(NamedTuple):
    mean: int
    variance: int

class MeanAndCovariance(NamedTuple):
    mean: int
    covariance: int

@overload
def predict_f(auto_cov: Literal[False]) -> MeanAndVariance:
    ...

@overload
def predict_f(auto_cov: Literal[True]) -> MeanAndCovariance:
    ...

def predict_f(auto_cov: bool = False) -> (MeanAndVariance | MeanAndCovariance):
    # calculations
    return  MeanAndCovariance(1, 2) if auto_cov else MeanAndVariance(1, 2)

Copy link
Member

Choose a reason for hiding this comment

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

I don't think there's a good way around the if-else, and I think it's better to be explicit than the ambiguity of having to remember whether it's a [N, Q] or [Q, N, N] tensor ...:) I'd be happy with this.

Copy link
Author

Choose a reason for hiding this comment

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

As it pointed out, typing.Literal is only available from Python 3.8 and up :/

Copy link
Member

Choose a reason for hiding this comment

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

@antonykamp the typing_extensions module provides backports for older versions of Python, it does seem to include Literal. :)

Copy link
Author

Choose a reason for hiding this comment

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

I added this pattern with overload and Tuple to the abstract method predict_f in gpflow/models/models.py. Should the overloads of predict_f be marked as abstract too? In any case I see no chance to test the Ellipsis operator of each overloaded funciton :/

Also, I wanted to ask if the parameters of the model constructor should be listed in the parametrization?



class BayesianModel(Module, metaclass=abc.ABCMeta):
Expand Down Expand Up @@ -218,7 +223,7 @@ def predict_y(
)

f_mean, f_var = self.predict_f(Xnew, full_cov=full_cov, full_output_cov=full_output_cov)
return self.likelihood.predict_mean_and_var(f_mean, f_var)
return MeanAndVariance(*self.likelihood.predict_mean_and_var(f_mean, f_var))

def predict_log_density(
self, data: RegressionData, full_cov: bool = False, full_output_cov: bool = False
Expand Down
2 changes: 1 addition & 1 deletion gpflow/models/sgpmc.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,4 +127,4 @@ def predict_f(self, Xnew: InputData, full_cov=False, full_output_cov=False) -> M
white=True,
full_output_cov=full_output_cov,
)
return mu + self.mean_function(Xnew), var
return MeanAndVariance(mu + self.mean_function(Xnew), var)
4 changes: 2 additions & 2 deletions gpflow/models/sgpr.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ def predict_f(self, Xnew: InputData, full_cov=False, full_output_cov=False) -> M
- tf.reduce_sum(tf.square(tmp1), 0)
)
var = tf.tile(var[:, None], [1, self.num_latent_gps])
return mean + self.mean_function(Xnew), var
return MeanAndVariance(mean + self.mean_function(Xnew), var)

def compute_qu(self) -> Tuple[tf.Tensor, tf.Tensor]:
"""
Expand Down Expand Up @@ -384,4 +384,4 @@ def predict_f(self, Xnew: InputData, full_cov=False, full_output_cov=False) -> M
) # [N, P]
var = tf.tile(var[:, None], [1, self.num_latent_gps])

return mean, var
return MeanAndVariance(mean, var)
2 changes: 1 addition & 1 deletion gpflow/models/svgp.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,4 +169,4 @@ def predict_f(self, Xnew: InputData, full_cov=False, full_output_cov=False) -> M
full_output_cov=full_output_cov,
)
# tf.debugging.assert_positive(var) # We really should make the tests pass with this here
return mu + self.mean_function(Xnew), var
return MeanAndVariance(mu + self.mean_function(Xnew), var)
4 changes: 2 additions & 2 deletions gpflow/models/vgp.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ def predict_f(
mu, var = conditional(
Xnew, X_data, self.kernel, self.q_mu, q_sqrt=self.q_sqrt, full_cov=full_cov, white=True,
)
return mu + self.mean_function(Xnew), var
return MeanAndVariance(mu + self.mean_function(Xnew), var)


class VGPOpperArchambeau(GPModel, InternalDataTrainingLossMixin):
Expand Down Expand Up @@ -252,4 +252,4 @@ def predict_f(
f_var = self.kernel(Xnew) - tf.linalg.matmul(LiKx, LiKx, transpose_a=True)
else:
f_var = self.kernel(Xnew, full_cov=False) - tf.reduce_sum(tf.square(LiKx), axis=1)
return f_mean, tf.transpose(f_var)
return MeanAndVariance(f_mean, tf.transpose(f_var))
4 changes: 4 additions & 0 deletions tests/gpflow/models/test_model_predict.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import gpflow
from gpflow.inducing_variables import InducingPoints
from gpflow.kernels import Matern32
from gpflow.models.model import MeanAndVariance

rng = np.random.RandomState(0)

Expand Down Expand Up @@ -97,6 +98,9 @@ def test_gaussian_mean_and_variance(Ntrain, Ntest, D):
assert np.allclose(mu_f, mu_y)
assert np.allclose(var_f, var_y - 1.0)

assert np.allclose(model_gp.predict_f(Xtest).mean, mu_f)
assert np.allclose(model_gp.predict_f(Xtest).variance, model_gp.predict_f(Xtest)[1])


@pytest.mark.parametrize("Ntrain, Ntest, D", [[100, 10, 2]])
def test_gaussian_log_density(Ntrain, Ntest, D):
Expand Down