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

Enable entry of model wrappers into the MLJ Model Registry #1125

Closed
10 tasks done
ablaom opened this issue May 31, 2024 · 0 comments · Fixed by #1127
Closed
10 tasks done

Enable entry of model wrappers into the MLJ Model Registry #1125

ablaom opened this issue May 31, 2024 · 0 comments · Fixed by #1127
Assignees

Comments

@ablaom
Copy link
Member

ablaom commented May 31, 2024

This has been requested for documentation purposes. Model wrappers include things like TunedModel and BalancedModel and the idea is that such wrappers should be more discoverable by adding them to the model registry, with a tag of "meta-algorithm"; they will then automatically appear in the "Model Browser" (and a planned modernisation of that).

Currently, wrappers are excluded from the registry for these technical reasons:

  1. Model wrappers are typically instantiated by the user with a constructor with a different name from the type and the type returned depends on constructor arguments (usually the atomic model being wrapped). That is, there are multiple types associated with a single "wrapper". For example: we have TunedModel (public user constructor) returning DeterministicTunedModel or ProbabilisticTunedModel ("private" types). The registry is based around types not constructors, and it was deemed confusing listing these "private" types in the registry. Incidentally, documentation is generally attached to the constructor, not the types, but these are always the same for non-wrappers.

  2. Model wrappers do not have a kwarg constructor with defaults for all hyperparameters, because a default for the atomic model being wrapped is problematic. This poses a problem in integration tests. This issue more easily remedied as model wrappers are flagged by a trait, so we can just explicitly exclude them from integration tests.

Proposed solution

With the current entrenched type hierarchy, there's no getting around multiple "private" types for a single wrapper (constructor). In the proposed solution, the user will see these private types in the registry, but clicking on them in the Model Browser will send them to the central "constructor" (public) documentation. I am proposing to add a new trait constructor(model) so that we get a mapping from types to constructors for documentation purposes. It doesn't need to be overloaded for regular models, only for the wrappers at this time. However, I am also keen on this solution because it is more natural in the more "functional" setting I have been exploring in LearnAPI, a re-imagining of the MLJModelInterface for the future.

No breaking changes are required, but we will still need to rollout [compat] lower bound updates for the packages depending on the new trait.

Planned implementation

Then we need to overload constructor for the wrappers in these packages, in the order stated:

And finally:

  • Update MLJInterfaceTests.jl to explicitly exclude testing of wrappers (this suffices for MLJTestIntegration.jl) not needed
  • Update the ModelDescriptors.toml at MLJ/docs to include tags for the new wrappers. Regenerate MLJ documentation to update the Model Browser. Check integration tests at MLJ. New version: MLJ v0.20.6 JuliaRegistries/General#108360
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant