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

Refactor type hierarchy #20

Closed
pat-alt opened this issue Oct 20, 2022 · 6 comments
Closed

Refactor type hierarchy #20

pat-alt opened this issue Oct 20, 2022 · 6 comments
Labels
enhancement New feature or request

Comments

@pat-alt
Copy link
Member

pat-alt commented Oct 20, 2022

Now already looking at quite a few abstract types. It may be more useful to reduce the use of that and instead rely on THTT.

@pat-alt pat-alt added the enhancement New feature or request label Oct 20, 2022
@pat-alt
Copy link
Member Author

pat-alt commented Oct 20, 2022

Addressed in #21

@ablaom is this what you had in mind? Instead of ConformalSet <: MMI.Supervised we'd ideally have ConformalSet <: MMI.Set in the future.

mermaid-figure-1

@ablaom
Copy link

ablaom commented Oct 25, 2022

Yeah, mostly:

  • At present Interval is a subtype of Supervised.
  • I don't see a need for a separate ConformalInterval <: Interval subtype in MLJModelInterface. If it were useful to you, that could live in ConformalPrediction.jl, no?
  • I would make make Set and ProbababilisticSet subtypes of Supervised.

In other words, we currently have

Probabilistic <: Supervised
Deterministic <: Supervised
Interval <: Supervised

and I would propose extending this with

Set <: Supervised # if you think you even need this
ProbabilisticSet <: Supervised

(In the new LearnAPI.jl, we would add two target proxy types, Set and ProbabilisticSet. These are possible values of a trait target_proxy and there would be no need to restrict to supervised models, but that's another story...)

@ablaom
Copy link

ablaom commented Oct 25, 2022

Would that work for you?

pat-alt added a commit that referenced this issue Oct 27, 2022
Develop

- Augmented test suite to now include various different type of MLJ models.
- Added minor suggestion by @ablaom (see [here](#20 (comment)))
- Some doc stuff
@pat-alt
Copy link
Member Author

pat-alt commented Oct 27, 2022

Hi Anthony! Yes, absolutely that works. In fact, that's what I had in mind and also reflect the way things are currently implemented (I think the diagram needs some clarification perhaps).

Also, good point ProbabilisticSet <: Supervised, have added a placeholder for this now in #26. To be clear, at this point we have the following:

"An abstract base type for conformal models that produce interval-valued predictions. This includes most conformal regression models."
abstract type ConformalInterval <: MMI.Interval end

"An abstract base type for conformal models that produce set-valued deterministic predictions. This includes most conformal classification models."
abstract type ConformalSet <: MMI.Supervised end                    # ideally we'd have MMI.Set

"An abstract base type for conformal models that produce set-valued probabilistic predictions. This includes most conformal classification models."
abstract type ConformalProbabilisticSet <: MMI.Supervised end       # ideally we'd have MMI.ProbabilisticSet

"An abstract base type for conformal models that produce probabilistic predictions. This includes some conformal classifier like Venn-ABERS."
abstract type ConformalProbabilistic <: MMI.Probabilistic end

For set-valued predictions, I am at this point still just subtyping <: MMI.Supervised, since the specific types still need to be added to LearnAPI.jl. In particular, for conformal models of type ConformalSet currently produce prediction of type UnivariateFinite (linking our related discussion on discourse).

I will close this issue now and open a new one related to set-valued predictions specifically.

Thanks!

@ablaom
Copy link

ablaom commented Oct 27, 2022

Yes, this sound good.

To clarify, MLJModelInterface.jl remains the home for MLJ's basement-level model interface. I just mention LearnAPI.jl because that is where I hope to move to later.

So we should add two new abstract types to MLJModelInterface: Set <: Supervised and ProbabilisticSet <: Supervised. We already have Interval <: Supervised. Would you be willing to make the PR? The additions go here and here.

@pat-alt
Copy link
Member Author

pat-alt commented Oct 31, 2022

Thanks @ablaom 👍🏽

Yes, I'll make the PR as soon as I can (have a few commitments these next two weeks, but will get to it eventually 😅)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants