h2o-3 icon indicating copy to clipboard operation
h2o-3 copied to clipboard

Published 20 hours ago • verified publisher icon h2oai

Fix Isolation Forest grid search test and API [nocheck]

Open valenad1 opened this issue 3 years ago • 6 comments

I have noticed some inconsistency in IF grid search test for R.

The R API treat IF as supervised algorithm and flag is_supervised isn't working properly. This PR provide fix for is_supervised flag and change that IF will be unsupervised by default.

I have created second PR in case that we cannot suddenly change IF to unsupervised for grid search. This PR (https://github.com/h2oai/h2o-3/pull/5661) only fix is_supervised flag and add test that IF can be used with is_supervised = FALSE flag.

valenad1 avatar Aug 31 '21 16:08 valenad1

Please do not introduce a flag is_supervised. Note: API changes like this one should be discussed in #h2o-3-api-updates.

I am not introducing new one, flag is_supervised is already there, I am just using it properly

valenad1 avatar Aug 31 '21 21:08 valenad1

@valenad1 I am sorry - my bad - I misunderstood the PR.

I am surprised to see the option there - I don't think we should have it

Would there be a solution that doesn't utilize this flag and works out of the box?

michalkurka avatar Aug 31 '21 21:08 michalkurka

Would there be a solution that doesn't utilize this flag and works out of the box?

I'll figure something out :)

valenad1 avatar Aug 31 '21 21:08 valenad1

@valenad1 this looks like this was a code change (in models.R) and not just a test change - in this case we should have a jira

Agree, I will make a JIRA

So I merge this fix and after implement the change that will drop is_supervised parameter. Is that correct? @michalkurka

valenad1 avatar Aug 31 '21 21:08 valenad1

So I merge this fix and after implement the change that will drop is_supervised parameter. Is that correct? @michalkurka

If I read this PR correctly, the option didn't work before, or at least not in recent years. If so - it was unused - and thus could be gracefully deprecated (= eg. warning in 3.34 and then removal in 3.36)

michalkurka avatar Aug 31 '21 22:08 michalkurka

If I read this PR correctly, the option didn't work before, or at least not in recent years.

I agree.

If so - it was unused - and thus could be gracefully deprecated (= eg. warning in 3.34 and then removal in 3.36)

Ok, I'll try to use a different way (somehow it works for python without this parameter) and make this parameter depracated.

valenad1 avatar Aug 31 '21 22:08 valenad1