Skip to content

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
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

2 participants