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

GH-16184 Add possibility to set auc_type in AutoML #16367

Draft
wants to merge 2 commits into
base: rel-3.46.0
Choose a base branch
from

Conversation

maurever
Copy link
Contributor

Issue: #16184

@maurever maurever added this to the 3.46.0.6 milestone Aug 15, 2024
@maurever maurever self-assigned this Aug 15, 2024
Copy link
Contributor

@tomasfryda tomasfryda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So far it looks good but could you add an R test and some documentation.

I think it would be also a good idea to expose it the same way we expose monotone_constraints .

And last but not the least, could you make sure it works with stopping_metric and that the metric actually ends up in the leaderboard? (In the multinomial case)

@maurever
Copy link
Contributor Author

@tomasfryda, thanks for your suggestions!

It is the question if it is necessary to expose it similarly to the monotone_constraints parameter. I found out we can add algo parameters in this way, but it is nowhere documented. So, it's a good point to do that.

Why adding a new parameter to the AutoML class is better than using the algo_parameters parameter?

Definitely a good point to add an R test and test with early stopping.

@tomasfryda
Copy link
Contributor

I think algo_parameters is more for "hacks" than regular use. You can change any parameter if you enable it using the algo_parameters. It's not documented because it can cause some problems so it's intended only for expert users.

@maurever
Copy link
Contributor Author

@tomasfryda, thanks for explanation. Ok, I will do it like the monotone_contraint parameter, no problem.

I am not a fan of many parameters, so I tried to find a different solution. :)

@maurever maurever removed the 4RELEASE label Aug 19, 2024
@maurever maurever removed this from the 3.46.0.6 milestone Aug 19, 2024
@@ -138,6 +139,7 @@ h2o.automl <- function(x, y, training_frame,
sort_metric = c("AUTO", "deviance", "logloss", "MSE", "RMSE", "MAE", "RMSLE", "AUC", "AUCPR", "mean_per_class_error"),
export_checkpoints_dir = NULL,
verbosity = "warn",
auc_type="NONE",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be:

Suggested change
auc_type="NONE",
auc_type=c("NONE", "MACRO_OVO", "WEIGHTED_OVO", "MACRO_OVR", "WEIGHTED_OVR", "AUTO"),

And please see match.arg (https://www.rdocumentation.org/packages/base/versions/3.6.2/topics/match.arg) or you can use case-insensitive version from explain module

#' Works like match.arg but ignores case
#' @param arg argument to match that should be declared as a character vector containing possible values
#' @param choices argument to choose from (OPTIONAL)
#' @return matched arg
case_insensitive_match_arg <- function(arg, choices) {
var_name <- as.character(substitute(arg))
if (missing(choices))
choices <- eval(formals(sys.function(-1))[[var_name]])
orig_choices <- choices
if (identical(arg, eval(formals(sys.function(-1))[[var_name]])))
arg <- choices[[1]]
choices <- tolower(choices)
if (length(arg) != 1)
stop(sprintf("'%s' must be of length 1", var_name), call. = FALSE)
arg <- tolower(arg)
i <- pmatch(arg, choices, nomatch = 0L, duplicates.ok = FALSE)
if (all(i == 0L) || length(i) != 1)
stop(sprintf("'%s' should be one of %s", var_name, paste(dQuote(orig_choices), collapse = ", ")), call. = FALSE)
return(orig_choices[[i]])
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank @tomasfryda for the suggestion. I am still working on this PR. I converted it to a draft. I would like you to review it after I finish it. Thanks!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm always happy to help with automl or R so feel free to ask if there is something unclear.

@maurever maurever marked this pull request as draft September 4, 2024 09:07
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 this pull request may close these issues.

2 participants