GPJax icon indicating copy to clipboard operation
GPJax copied to clipboard

Initial kronecker work

Open thomaspinder opened this issue 3 years ago • 2 comments

Pull request type

Please check the type of change your PR introduces:

  • [ ] Bugfix
  • [x] Feature
  • [ ] Code style update (formatting, renaming)
  • [ ] Refactoring (no functional changes, no api changes)
  • [ ] Build related changes
  • [ ] Documentation content changes
  • [ ] Other (please describe):

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Other information

thomaspinder avatar Jul 20 '22 12:07 thomaspinder

Codecov Report

Merging #87 (429f594) into master (246addf) will decrease coverage by 2.66%. The diff coverage is 76.47%.

:exclamation: Current head 429f594 differs from pull request most recent head d330769. Consider uploading reports for the commit d330769 to get more accurate results

@@            Coverage Diff             @@
##           master      #87      +/-   ##
==========================================
- Coverage   98.95%   96.29%   -2.67%     
==========================================
  Files          13       13              
  Lines         961     1080     +119     
==========================================
+ Hits          951     1040      +89     
- Misses         10       40      +30     
Flag Coverage Δ
unittests 96.29% <76.47%> (-2.67%) :arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
gpjax/kernels.py 86.79% <74.40%> (-11.90%) :arrow_down:
gpjax/gps.py 100.00% <100.00%> (ø)
gpjax/variational_families.py 100.00% <100.00%> (ø)
gpjax/variational_inference.py 97.67% <100.00%> (+0.02%) :arrow_up:
gpjax/types.py 100.00% <0.00%> (ø)
gpjax/utils.py 100.00% <0.00%> (ø)
gpjax/config.py 100.00% <0.00%> (ø)
gpjax/likelihoods.py 100.00% <0.00%> (ø)
... and 5 more

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

codecov[bot] avatar Jul 20 '22 13:07 codecov[bot]

Comment from Dan's push:

  • Minimal Kronecker kernel. The inverse is highly inefficient - this can be improved!
  • Added white noise kernel.
  • Added diagonal computation abstraction.
  • Added minimal computation abstraction for kernel combinations (i.e., for * and +).

Main issues:

  • Though we can replace the computations in the gps.py, we might miss out with caching stuff e.g., the Cholesky decomposition in the dense computation case. Is there a sensible way around this?

  • As a further point to the preceding point, we should not be computing the gram matrix in general (yes I known we need to in the dense setting).

  • Think about computation savings for KL divergences - how can we resolve these?

Minor issues:

  • Need to discuss the best way to add some computation methods - it might be tidier than just adding a load of static methods altogether.
  • Need tests.

thomaspinder avatar Aug 02 '22 08:08 thomaspinder

Closed PR - Kronecker kernels will be added from a different branch, and not kernel_compute.

daniel-dodd avatar Oct 10 '22 15:10 daniel-dodd