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

Fix IndependentPosteriorMultiOutput for kernels with active_dims #1724

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

BLarvaron
Copy link

@BLarvaron BLarvaron commented Oct 13, 2021

I propose to replace use of K method by call as explained in #1723

PR type: enhancement

Related issue(s)/PRs: #1723

I propose to replace use of K method by __call__ as explained in GPflow#1723
@st--
Copy link
Member

st-- commented Oct 26, 2021

Hi @BL7X4 ! thanks for catching this, I suspect you are right. You need to run make format (or manually call the black formatter) to get the format-checker to pass (and merge in develop).

It'd be great if you could also add a test case that confirms that having kernels with active_dims was broken before, and is fixed by this change!

@st--
Copy link
Member

st-- commented Oct 26, 2021

(BTW- it's generally helpful to give PRs more descriptive names, for example "Fix IndependentPosteriorMultiOutput for kernels with active_dims" in this case - to make it more immediately obvious what it is about and here that it's fixing a bug:))

@st-- st-- added the bugfix label Oct 26, 2021
@st-- st-- changed the title Update posteriors.py Fixes IndependentPosteriorMultiOutput for kernels with active_dims Oct 26, 2021
@st-- st-- changed the title Fixes IndependentPosteriorMultiOutput for kernels with active_dims Fix IndependentPosteriorMultiOutput for kernels with active_dims Oct 26, 2021
@st--
Copy link
Member

st-- commented Feb 16, 2022

@BL7X4 to merge this in, you would need to:

  1. merge the current state of the master branch back into your fork/branch
  2. make sure you run the black formatter before committing
  3. add a small test that demonstrates the bug (without your change) and that this change fixes it (with your change).
    Thanks for your contribution!

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 this pull request may close these issues.

None yet

2 participants