You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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...
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 thatlogdet
,Cholesky
, and\
are going to work (which is not the case forReal
s,AbstractVector{<:Real}
s,UniformScaling
s etc), I can check thesize
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.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 #236 (comment)
The text was updated successfully, but these errors were encountered: