search

invenia / Models.jl

An interface package that defines the methods and types for working with models.
MIT License
3 stars 0 forks source link

Strong assumptions about form of distributions in TestUtils #18

Closed willtebbutt closed 4 years ago

willtebbutt commented 4 years ago

The Models.TestUtils tests currently assume that single-output regressors return vectors of Normal distributions, and that multi-output regressors return Vectors of MvNormals. See here and here resp.

@glennmoy @nickrobinson251 why are we doing this? It seems overly-restrictive.

nickrobinson251 commented 4 years ago

hm. seem wrong I don't think we need to test for Real eltype either

should probably just test for Univariate and Multivariate distributions?

but @glennmoy may know better

glennmoy commented 4 years ago

It seems overly-restrictive

Yeah I think is just a legacy of when it lived in our internal code base. I don't think there was a conscious decision to restrict them so much - at least going by the initial review there was no comment made about it.

I think it's reasonable to replace with <:Univariate and <:Multivariate (I assume that's what you mean?) and not worry about the eltype as @nickrobinson251 says.