SossMLJ.jl icon indicating copy to clipboard operation
SossMLJ.jl copied to clipboard

Reorganization of the predictor types

Open DilumAluthge opened this issue 4 years ago • 6 comments

Currently, we only have a single predictor type:

struct SossMLJPredictor{M} <: Distributions.Distribution{MixedVariate, MixedSupport}
    model::M
    post
    pred
    args
end

I think we'll need a slightly more complicated set of types.

For predict_joint, we will continue to use SossMLJPredictor. But for predict, I think we'll need different types.

For predict with a classification model, we'll need to use MLJBase.UnivariateFinite for compliance with the MLJ model interface.

For predict with a regression model, it's not immediately obvious to me what we should return.

DilumAluthge avatar Aug 19 '20 02:08 DilumAluthge

Yeah, I think you're right. How about this:

  • We keep SossMLJPredictor as the "general" case, which is probably also good for regression
  • Add SossMLJMarginalPredictor for marginals. I think users shouldn't have to type this, we can have a marginalize function to build it
  • MLJSossClassifier for classification in the MLJ sense, since that needs this special treatment

Oh, and we'll need type parameters on these, I just left it open to get a quick start

cscherrer avatar Aug 19 '20 14:08 cscherrer

For consistency, can we name the last one SossMLJClassifier?

DilumAluthge avatar Aug 20 '20 21:08 DilumAluthge

Oops, yes that's what I meant :)

cscherrer avatar Aug 20 '20 21:08 cscherrer

The parts of this that are blocking 0.1.0 are implemented in #91.

There is still additional work to be done, but it doesn't block 0.1.0.

DilumAluthge avatar Aug 28 '20 06:08 DilumAluthge

I'm now second-guessing whether we really need a SossMLJClassifier type. Let's see how it goes first

cscherrer avatar Aug 30 '20 15:08 cscherrer

I'm going to mark this as potentially breaking, since it will probably result in some changes to the return types of public functions.

DilumAluthge avatar Sep 15 '20 23:09 DilumAluthge