Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid model introspection by requiring users to provide a function that defines a model instance #199

Open
CameronBieganek opened this issue Oct 20, 2023 · 3 comments

Comments

@CameronBieganek
Copy link

CameronBieganek commented Oct 20, 2023

I think I mentioned this idea a few years ago on a different thread, but I never gave the proposal its own issue.

This is a breaking change, but I think it would provide a cleaner interface for hyperparameter tuning, and it would also address #174.

The idea is that instead of specifying the hyperparameters that need to be tuned with a quoted expression like
:(estimator.leafsize), the user can simply provide a function that creates a new model. Here is one way that it could work:

function make_model(; _K, _leafsize)
    Pipeline(
        encoder = ContinuousEncoder(),
        estimator = KNNRegressor(K=_K, leafsize=_leafsize)
    )
end

domain = Domain(
    _K = (1, 3, 5, 7, 11),
    _leafsize = (5, 10, 15)
)

tunable_model = TunedModel(make_model, domain; strategy=Grid())

I've prefixed the feature names with an underscore to emphasize that what matters is that the keyword argument names in make_model match the keyword argument names in Domain. The actual names are arbitrary.

The downside to requiring the user defined function to use keyword arguments is that you can't really use do-notation in TunedModel, because as far as I can tell there is no way to define an anonymous function with keyword arguments by using do-notation. So, an alternative interface would be to require the user defined function to take a single positional argument with property destructuring, like this:

function make_model((; _K, _leafsize))
    Pipeline(
        encoder = ContinuousEncoder(),
        estimator = KNNRegressor(K=_K, leafsize=_leafsize)
    )
end

domain = Domain(
    _K = (1, 3, 5, 7, 11),
    _leafsize = (5, 10, 15)
)

tunable_model = TunedModel(make_model, domain; strategy=Grid())

This can then be expressed with do-notation as follows:

tunable_model = (
    TunedModel(domain; strategy=Grid()) do (; _K, _leafsize)
        Pipeline(
            encoder = ContinuousEncoder(),
            estimator = KNNRegressor(K=_K, leafsize=_leafsize)
        )
    end
)

I believe this interface is generic enough to work for any hyperparameter tuning strategy.

@CameronBieganek
Copy link
Author

CameronBieganek commented Oct 20, 2023

The docs could probably specify that the model factory function is provided with an object whose properties are the same as the keyword arguments to Domain. So, the user can use argument destructuring as described in the previous post, or they can access hyperparameters via getproperty, like this:

domain = Domain(
    K = (1, 3, 5, 7, 11),
    leafsize = (5, 10, 15)
)

function make_model(params)
    Pipeline(
        encoder = ContinuousEncoder(),
        estimator = KNNRegressor(K=params.K, leafsize=params.leafsize)
    )
end

tunable_model = TunedModel(make_model, domain; strategy=Grid())

or like this:

tunable_model = (
    TunedModel(domain; strategy=Grid()) do params
        Pipeline(
            encoder = ContinuousEncoder(),
            estimator = KNNRegressor(K=params.K, leafsize=params.leafsize)
        )
    end
)

or this:

tunable_model = (
    TunedModel(domain; strategy=Grid()) do params
        (; K, leafsize) = params
        Pipeline(
            encoder = ContinuousEncoder(),
            estimator = KNNRegressor(K=K, leafsize=leafsize)
        )
    end
)

or this:

tunable_model = (
    TunedModel(domain; strategy=Grid()) do params
        Pipeline(
            encoder = ContinuousEncoder(),
            estimator = KNNRegressor(; params...)
        )
    end
)

@ablaom
Copy link
Member

ablaom commented Oct 25, 2023

@CameronBieganek Thanks for pitching in with this idea. Here are a few drawbacks that come to mind. I wonder what you think about them:

  • Essentially you are replacing an inspectable, mutable hyper-parameter, of a TunedModel with a function, which is neither inspectable, nor mutable, except by total replacement. This seems to limit composition. For an admittedly contrived example, suppose I want to tune hyper-parameters one at a time, with a sequence of wrappings, as in TunedModel(model=TunedModel(model=SomeAtomicModel(), range=r1), range=r2) where r2 refers to (nested) hypeparameters of SomeAtomicModel(). This would not be possible. Do you see that as an issue?

  • With the atomic model types hidden, I have a problem with forwarding traits of the AtomicModel to the wrapper TunedModel(model=AtomicModel()). For example, if I'm wrapping a probabilistic predictor model (prediction_type(model) == :probabilistic) as a TunedModel, then the wrapped model must also be seen as probabilistic. How should that work here? (Currently, we also need to get an abstract model type as a type parameter for the wrapper, which would similarly be opaque in your suggested scheme.)

@CameronBieganek
Copy link
Author

CameronBieganek commented Oct 29, 2023

@ablaom I will try to respond to your questions soon, but for now I just want to point out a couple advantages of this approach.

Fields of model types are no longer public:

The current hyperparameter tuning interface requires that the fields of model types be public API, otherwise the user wouldn't know the correct :foo symbol to write. With my proposal above, those fields would no longer need to be public API. Of course the part of the API that is still public is the keyword argument constructor for a model. (But of course there is no requirement that the keyword argument names match the field names of the model type.)

Pipeline no longer needs a keyword argument constructor:

The only reason that Pipeline needs a keyword argument constructor right now is to make it easier for users to specify the right expression for nested parameters in hyperparameter tuning, e.g.
:(random_forest.min_samples_leaf). With my proposal above this would no longer be necessary, so the positional argument Pipeline constructor would be sufficient.

That might seem like a small win, but interface simplifications are always a win in my book, and this would bring MLJ closer to alignment with TableTransforms.jl.

@github-project-automation github-project-automation bot moved this to tracking/discussion/metaissues/misc in General Aug 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: tracking/discussion/metaissues/misc
Development

No branches or pull requests

2 participants