JuliaGaussianProcesses / JuliaGaussianProcesses/AbstractGPs.jl
LatentGP: rename to jitter and restrict to scalar Real
Nobody has claimed this yet.
- Dominant language
- Julia
- Stars
- 287
- Forks
- 27
- PR merge metrics
- No merged PRs in 30d
Description
I have to say I find it quite confusing when it's good to add a type restriction because that'll prevent errors and when it's bad to add a type restriction because it'll make it less flexible. 😅 I mean, both of those are generally true. But why that results in wanting it sometimes and not wanting it other times.
In my view it's a judgement call each time -- I also don't have a standard set of rules for when I type stuff really strictly vs not.
In this case, my feeling is that knowing that you're going to get an AbstractMatrix{<:Real} (as a GP-implementer) is very helpful -- there's a good chance that logdet, Cholesky, and \ are going to work (which is not the case for Reals, AbstractVector{<:Real}s, UniformScalings etc), I can check the size if I like, I can specialise on particular matrix types etc if I know something particular about the structure of certain matrices. So it lets me make slightly stronger assumptions about the internals that are more likely to be true, and it doesn't restrict the user-facing interface.
We can't make the same restriction for LatentGP though, as at that point we don't actually know the length of the observation vector, so the jitter Σy actually needs to be a scalar, or a vector or matrix of the right shape (it simply gets passed to FiniteGP in the construction of LatentFiniteGP)...
Indeed. I think that's fine though, because it's a different type and I'm okay with it having different semantics. We should probably rename it to jitter and restrict it to be a Real at some point...
Originally posted by @willtebbutt in https://github.com/JuliaGaussianProcesses/AbstractGPs.jl/pull/236#discussion_r741274467
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Locate the LatentGP definition and the LatentFiniteGP construction, then inspect how the jitter/Σy value is passed to FiniteGP. Done means the relevant LatentGP argument is named jitter and restricted to scalar Real while preserving the shape-flexible behavior described for LatentFiniteGP.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- julia
- Domain
- machine-learning
- Issue type
- Refactor
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 42/100