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

Incorrect equations in SGPR_notes #1753

Closed
joacorapela opened this issue Jan 14, 2022 · 7 comments
Closed

Incorrect equations in SGPR_notes #1753

joacorapela opened this issue Jan 14, 2022 · 7 comments
Labels

Comments

@joacorapela
Copy link
Contributor

Bug / performance issue / build issue

I believe there is an error in the next-to-last equation in the documentation page at https://gpflow.readthedocs.io/en/master/notebooks/theory/SGPR_notes.html, which makes the last prediction equation invalid. I provide details at http://www.gatsby.ucl.ac.uk/~rapela/gpflow/wrongPredictionEq.pdf. Can this be right?

@joacorapela
Copy link
Contributor Author

The next-to-the-last equation in the documentation page is correct, as shown here.

@st--
Copy link
Member

st-- commented Feb 7, 2022

Hi @joacorapela , it's great that you're carefully going through the docs, and thanks for raising what you saw as a mistake!
I've just gone through both your writeup and the docs, I think you had a minor mistake in your note 5. - you write you used Kuu⁻¹ Λ Kuu⁻¹ = L⁻ᵀ B⁻¹ L⁻¹, but in the documentation page it actually reads Kuu⁻¹ Λ⁻¹ Kuu⁻¹ = L⁻ᵀ B⁻¹ L⁻¹, note the Λ vs Λ⁻¹. With this, both sides of the second-to-last equation should match up.

@joacorapela
Copy link
Contributor Author

Thanks @st-- . The third to the last equation in the GPflow documentation page correctly states Kuu⁻¹ Λ Kuu⁻¹ = L⁻ᵀ and as proved here the GPflow documentation is correct. What do you think?

@st--
Copy link
Member

st-- commented Feb 8, 2022

There's clearly a bug somewhere. I'm not quite sure where it is. One way that I find helpful is to think of covariance matrices as having "units". So e.g. Kfu Kuu⁻¹ Kuf has units of (cov) (cov)⁻¹ (cov) which cancels out to (cov) which is appropriate for Qff.

We can see that the equations for m and Λ are inconsistent with the definition q(u) = N(m, Λ) which would imply Λ is a covariance matrix, but based on its definition below, Λ clearly has "units" (cov)⁻¹, which matches up with the mean, m = Λ⁻¹ Kuu⁻¹ Kuf y σ⁻², the units are (mean) = [Λ⁻¹] (cov)⁻¹ (cov) (mean) (cov)⁻¹, so the unit of Λ⁻¹ has to be (cov) for it to be consistent.

@st--
Copy link
Member

st-- commented Feb 8, 2022

In other words, Kuu⁻¹ Λ Kuu⁻¹ isn't consistent, it should be Kuu⁻¹ Λ⁻¹ Kuu⁻¹ based on the "units".

@st-- st-- reopened this Feb 8, 2022
@st-- st-- changed the title Incorrect prediction formula? Incorrect prediction formula Feb 8, 2022
@st-- st-- changed the title Incorrect prediction formula Incorrect prediction formula in SGPR_notes Feb 8, 2022
@st-- st-- changed the title Incorrect prediction formula in SGPR_notes Incorrect equations in SGPR_notes Feb 8, 2022
@joacorapela
Copy link
Contributor Author

joacorapela commented Feb 22, 2022

Thanks for your comment @st--. I believe there is a typo in the third to the last equation of this GPflow note. The right hand side of this equation should contain instead of , as proven here. With this correction, the proof of the predictive mean (second to the last equation of this GPflow note) becomes simple, as shown here. I created a pull request about this. Do you agree @st-- ?

@st--
Copy link
Member

st-- commented Mar 23, 2022

I think we'd resolved this now ? If anyone disagrees please shout and we can reopen it:)

@st-- st-- closed this as completed Mar 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants