JuliaGaussianProcesses / AbstractGPs.jl

Abstract types and methods for Gaussian Processes.
https://juliagaussianprocesses.github.io/AbstractGPs.jl/dev
Other
218 stars 20 forks source link

Clarify API for GP approximations #361

Closed st-- closed 1 year ago

st-- commented 1 year ago

Summary AbstractGPs.jl should define the API we expect across the ecosystem. Currently, this is not the case for GP approximations, even though they're already introduced in this package (VFE for sparse GPR).

Proposed changes Expose posterior and approx_log_evidence as part of the API with nice generic docstrings explaining their purpose.

Related issues Resolves #221

Also see #223, #241, #318, maybe #319

What alternatives have you considered? Status quo: some definitions in ApproximateGPs.jl, but people not realizing that you could just use a three-parameter form for posterior, leading to more reimplementation of base FiniteGPs than would be necessary (e.g. https://github.com/SebastianCallh/IterGP.jl/blob/AbstractGPs-interface/src/gp.jl)

codecov[bot] commented 1 year ago

Codecov Report

Patch coverage: 87.50% and project coverage change: -0.74 :warning:

Comparison is base (3e5f0a5) 97.63% compared to head (501b702) 96.89%.

:exclamation: Current head 501b702 differs from pull request most recent head f260171. Consider uploading reports for the commit f260171 to get more accurate results

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #361 +/- ## ========================================== - Coverage 97.63% 96.89% -0.74% ========================================== Files 10 10 Lines 380 387 +7 ========================================== + Hits 371 375 +4 - Misses 9 12 +3 ``` | [Impacted Files](https://app.codecov.io/gh/JuliaGaussianProcesses/AbstractGPs.jl/pull/361?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaGaussianProcesses) | Coverage Δ | | |---|---|---| | [src/abstract\_gp.jl](https://app.codecov.io/gh/JuliaGaussianProcesses/AbstractGPs.jl/pull/361?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaGaussianProcesses#diff-c3JjL2Fic3RyYWN0X2dwLmps) | `100.00% <ø> (ø)` | | | [src/exact\_gpr\_posterior.jl](https://app.codecov.io/gh/JuliaGaussianProcesses/AbstractGPs.jl/pull/361?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaGaussianProcesses#diff-c3JjL2V4YWN0X2dwcl9wb3N0ZXJpb3Iuamw=) | `92.50% <0.00%> (-7.50%)` | :arrow_down: | | [src/sparse\_approximations.jl](https://app.codecov.io/gh/JuliaGaussianProcesses/AbstractGPs.jl/pull/361?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaGaussianProcesses#diff-c3JjL3NwYXJzZV9hcHByb3hpbWF0aW9ucy5qbA==) | `100.00% <100.00%> (ø)` | |

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

Crown421 commented 1 year ago

Should there be an AbstractGPApproximation type or so? That other packages can/ should use?

st-- commented 1 year ago

Should there be an AbstractGPApproximation type or so? That other packages can/ should use?

Hm... where would that be useful? Is there anything that you would be able to do with any approximation, without knowing what approximation it is (i.e. it'd be necessary as a common base type in the hierarchy)? Or just as a "type hint" (i.e. it might be better to just improve the docs)?

st-- commented 1 year ago

Open question from https://github.com/JuliaGaussianProcesses/AbstractGPs.jl/issues/221#issuecomment-1521960655:

should we add a struct ExactGP end "approximation" type to support a consistent API regardless of whether one is using exact GPR or a sparse approximation such as VFE/DTC?

Crown421 commented 1 year ago

Open question from #221 (comment):

should we add a struct ExactGP end "approximation" type to support a consistent API regardless of whether one is using exact GPR or a sparse approximation such as VFE/DTC?

I think this makes sense to me, with appropriate aliases for the current two argument form.

Re the above, I was thinking about a type hint, need to think about whether such a type might also be useful elsewhere.

Crown421 commented 1 year ago

One way I can think of where a AbstractApproximation type might be useful in starting a more general hierarchy. Next level down would be the methods mentioned here: https://github.com/JuliaGaussianProcesses/ApproximateGPs.jl/issues/84, might be some supertype of VFE, DTC (possibly others in the future like FITC)

st-- commented 1 year ago

Hello! What do you think needs doing/addressing/... before merging this ?