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

Kernel.output_shape semantics and further RandomProcess refactoring #652

Merged

Conversation

marvinpfoertner
Copy link
Collaborator

@marvinpfoertner marvinpfoertner commented Feb 25, 2022

In a Nutshell

A few months ago, I redefined the shape of the outputs of Kernel.__call__ to cov.output_shape + batch_shape.
However, this is very counterintuitive when in the context of random processes, since we would expect the outputs of the mean, var, and std functions to be batch_shape + randproc.output_shape, i.e. "the other way around".
My original change was made with multi-output GPs in mind, but even here it turns out that the batch_shape + cov.output_shape-order is easier to handle.

Detailed

  • rename Kernel.{shape => output_shape}
  • change output shapes of Kernel.{__call__, matrix} to batch_shape + kernel.output_shape
  • adapt random processes to these changes
  • fixed a bug in Matern 3/2 kernel
  • refactor RandomProcess
    • add input_ndim and output_ndim as public arguments (also add these to Function and Kernel)
    • add default implementation of _sample_at_input
    • remove unnecessary overrides of push_forward in derived classes
    • add mean and cov arguments to the RandomProcess constructor and provide default implementations
    • improve the docstrings

Related Issues

None

@marvinpfoertner marvinpfoertner added refactoring Refactoring of existing functionality improvement Improvements of existing functionality randvars Issues related to random variables labels Feb 25, 2022
@marvinpfoertner marvinpfoertner self-assigned this Feb 25, 2022
@marvinpfoertner marvinpfoertner added the bug Something isn't working label Feb 25, 2022
@marvinpfoertner marvinpfoertner changed the title Kernel.output_shape and further RandomProcess refactoring Kernel.output_shape semantics and further RandomProcess refactoring Feb 25, 2022
@codecov
Copy link

codecov bot commented Feb 25, 2022

Codecov Report

Merging #652 (6162957) into main (db6e7d3) will increase coverage by 0.02%.
The diff coverage is 89.06%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #652      +/-   ##
==========================================
+ Coverage   89.99%   90.02%   +0.02%     
==========================================
  Files         186      186              
  Lines        7025     7025              
  Branches     1102     1105       +3     
==========================================
+ Hits         6322     6324       +2     
+ Misses        463      460       -3     
- Partials      240      241       +1     
Impacted Files Coverage Δ
src/probnum/_function.py 95.12% <66.66%> (-2.25%) ⬇️
src/probnum/randprocs/markov/_markov_process.py 85.29% <85.71%> (+5.80%) ⬆️
src/probnum/randprocs/kernels/_kernel.py 93.67% <88.88%> (-1.00%) ⬇️
src/probnum/randprocs/_random_process.py 91.89% <90.24%> (+5.61%) ⬆️
src/probnum/randprocs/_gaussian_process.py 100.00% <100.00%> (ø)
src/probnum/randprocs/kernels/_matern.py 93.54% <100.00%> (ø)

@marvinpfoertner marvinpfoertner marked this pull request as ready for review February 25, 2022 16:31
Copy link
Contributor

@JonathanWenger JonathanWenger left a comment

Choose a reason for hiding this comment

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

Nice work! I didn't find anything that needs to be changed.

@marvinpfoertner marvinpfoertner merged commit 7af755a into probabilistic-numerics:main Feb 26, 2022
@marvinpfoertner marvinpfoertner deleted the kernel-output-shape branch February 26, 2022 10:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working improvement Improvements of existing functionality randvars Issues related to random variables refactoring Refactoring of existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants