Skip to content

test_utils to make it easier for downstream implementations to check correctness#1825

Open
st-- wants to merge 19 commits intodevelopfrom
st/test_kernel
Open

test_utils to make it easier for downstream implementations to check correctness#1825
st-- wants to merge 19 commits intodevelopfrom
st/test_kernel

Conversation

@st--
Copy link
Copy Markdown
Member

@st-- st-- commented Mar 24, 2022 •
edited
Loading

PR type: new feature

Summary

A common use-case of GPflow is to implement your own down-stream kernels / models / likelihoods / etc.
We should make it easy for such users to check their implementation for basic correctness (e.g. kernel is pos-def on test inputs) and adherence to interfaces (accepts tensors of appropriate shapes & returns the right things).

Proposed changes

  • Provide a test_utils submodule with basic checks for interfaces.

Minimal working example

import gpflow
from gpflow.test_utils import test_kernel

class MyKernel(gpflow.kernels.Kernel):
    # ...

test_kernel(MyKernel())

Release notes

Fully backwards compatible: yes

PR checklist

  • New features: code is well-documented
    • detailed docstrings (API documentation)
    • notebook examples (usage demonstration)
  • The bug case / new feature is covered by unit tests
  • Code has type annotations
  • Build checks
    • I ran the black+isort formatter (make format)
    • I locally tested that the tests pass (make check-all)
  • Release management
    • RELEASE.md updated with entry for this change
    • New contributors: I've added myself to CONTRIBUTORS.md

st--
st-- commented Mar 24, 2022
View reviewed changes
Comment thread gpflow/test_utils.py Outdated
assert np.linalg.eigvals(A).min() > -tol, "test for positive semi definite matrix"


def test_kernel(kernel: Kernel, X: np.ndarray, X2: np.ndarray):
Copy link
Copy Markdown
Member Author

@st-- st-- Mar 24, 2022

Choose a reason for hiding this comment

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

Should we provide default arguments for X and X2?

@st-- st-- marked this pull request as ready for review March 25, 2022 12:13
@st-- st-- requested a review from jesnie March 25, 2022 12:13
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 25, 2022 •
edited
Loading

Codecov Report

Merging #1825 (a0274a1) into develop (1a77914) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff            @@
##           develop    #1825   +/-   ##
========================================
  Coverage    98.00%   98.01%           
========================================
  Files          107      108    +1     
  Lines         5761     5785   +24     
========================================
+ Hits          5646     5670   +24     
  Misses         115      115
Impacted Files Coverage Δ
gpflow/__init__.py 100.00% <ø> (ø)
gpflow/test_utils.py 100.00% <100.00%> (ø)

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 1a77914...a0274a1. Read the comment docs.

jesnie
jesnie approved these changes Mar 28, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@jesnie jesnie left a comment

Choose a reason for hiding this comment

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

LGTM

Comment thread gpflow/test_utils.py Outdated
st--
st-- commented Mar 30, 2022
View reviewed changes
Comment thread tests/gpflow/kernels/test_positive_semidefinite.py Outdated
st--
st-- commented Mar 30, 2022
View reviewed changes
Comment thread tests/gpflow/kernels/test_positive_semidefinite.py Outdated
@st-- @jesnie
Apply suggestions from code review
0f8ea7c 
Co-authored-by: Jesper Nielsen <44195043+jesnie@users.noreply.github.com>
st--
st-- commented Mar 30, 2022
View reviewed changes
Comment thread RELEASE.md Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jesnie jesnie jesnie approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@st-- @jesnie