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

Unsupervised detectors and missing values in provided target #34

Closed
ablaom opened this issue Sep 22, 2022 · 4 comments
Closed

Unsupervised detectors and missing values in provided target #34

ablaom opened this issue Sep 22, 2022 · 4 comments
Labels
bug Something isn't working

Comments

@ablaom
Copy link

ablaom commented Sep 22, 2022

As I recall, we allow the specification of a target y when training an unsupervised detector, even though it is ignored, so that we may buy into MLJ's evaluate apparatus: If you have test labels, but no train labels, you just pad y with missings in the training indices.

It seems to me that the target_scitype should therefore be AbstractVector{<:Union{OrderedFactor{2},Missing}} in those cases, but I see that it is actually AbstractVector{OrderedFactor{2}} (at least it is for ABODDetector that I have been playing with). The bottom line is that MLJ is complaining when checking scitypes for these models, if the target presented has missings. (Actually, MLJBase <= 0.19.8 is just throwing an error, because of a bug; MLJBase >=0.20 would throw a warning, I guess, if it was compatible with OutlierDetection, which is isn't currently.)

@davnn Do you see any reason not to expand the scitype?

@josephsdavid

@ablaom ablaom added the bug Something isn't working label Sep 22, 2022
@davnn
Copy link
Member

davnn commented Sep 22, 2022

Hi, I recall it as a temporary fix, because missing values were causing some problems when using other MLJ features, e.g. ensemble models, but I think you have addressed that in JuliaAI/MLJEnsembles.jl@3e55cb6. Otherwise, I think we can expand the scitype and fix potential problems in MLJ down the line.

@ablaom
Copy link
Author

ablaom commented Sep 22, 2022

but I think you have addressed that in JuliaAI/MLJEnsembles.jl@3e55cb6.

Yes, I see. That should be fixed as you say. And if for some reason it's not, then that should be regarded as a bug to be promptly addressed.

Are you able to make the necessary PR to expand the scitypes?

@davnn
Copy link
Member

davnn commented Sep 24, 2022

I will expand the scitypes for the next release (which should be 0.20 compatible).

@davnn
Copy link
Member

davnn commented Oct 18, 2022

@davnn davnn closed this as completed Oct 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants