Checking model instances are not types

[ablaom]

A very common gotcha for users (not necessarily beginners) is calling a method with a model type instead of a model instance. Sometimes the consequence is a confusing error message.

Sometimes we check this (eg, here) but sometimes we don't (eg, base models in a Stack).

In some cases (eg, pipelines) we catch a type and automatically instantiate a default instance, if necessary. I have mixed feelings about this. For example, some model wrappers, eg, TunedModel, EnsembleModel, cannot be called with zero arguments (an atomic model must be specified). So I propose we eventually deprecate this behaviour

For now, I would like a review of MLJBase.jl which introduces the check everywhere a user is expected to call a method with an instance, but might supply a type by accident. An informative error is to be thrown. This could start with some infrastructure along the lines of:

const MSG_MODEL_EXPECTED  = "Type encountered where model instance expected"
function check_is_instance(model, message=MSG_MODEL_EXPECTED)
    model isa Model || throw(AssertionError(message))      # in future, use `ismodel(model)`
    return nothing
end 

cc @rikhuijzer

[ablaom]

I've made some progress here by adding a check_ismodel method to MLJBase #819 and implementing it for Stack.

Left to do is to implement the check in:

• [ ] TransformedTargetModel
• [ ] EnsembleModel
• [ ] learning network machines ?
• [ ] TunedModel
• [ ] IteratedModel

(For now pipelines allow types to be used in place of instances, which I think was a bad idea, but changing that now would be breaking.)