Evalml: Automl data_checks parameter: how best to support disable vs default vs custom

[dsherry]

765 adds a data_checks param to automl search. This parameter was intended to support three options:

• Default value. Result in the use of DefaultDataChecks class.
• Disable the feature. In our design doc, this called for passing in an EmptyDataChecks object.
• Custom value. A DataChecks subclass.

What's confusing about the current proposal for supporting those three options: having the default value be None implies that no data checks will run, although in fact we'll use DefaultDataChecks. Also, having a class EmptyDataChecks which is a noop is non-intuitive.

We have a similar problem in at least one other area: #793 will make the tuner class a parameter, which will have default/custom values (disabling doesn't apply there). It's related because its a case where we pass classes as input args. So whatever decision we reach here, should impact all such sites.

[dsherry]

@angela97lin @kmax12 I liked your ideas in the PR. Playing off that, here's a proposal:

• In AutoSearchBase.search, rename this data_checks parameter to data_checks_class, supporting the following values
  
  
  
  • Default value in __init__: DefaultDataChecks
  
  
  • Disabled: None
  
  
  • Custom: any DataChecks subclass
  
  
• Then in the impl, say data_checks = data_checks_class() and continue

What do you think?

[angela97lin]

Hmm, I think this is something we had previously considered as well. This requires that users define their own DataChecks class though, which shouldn't necessarily be the case? We should allow users to pass in a reference to the base DataCheck class too, rather than enforcing them to implement a subclass. For example, I think the above API wouldn't work with this code snippet:

data_check = CustomDataCheck()
data_checks = DataChecks(data_checks=[data_check])
automl.search(X, y, data_checks=data_checks)

Here, a user would instead have to write something like:

class CustomDataChecks(DataChecks):
    def __init__(self):
        self.data_checks = [CustomDataChecks()]

automl.search(X, y, data_checks_class=CustomDataChecks)

I feel like that's too limiting :/

[angela97lin]

(I guess more broadly, any DataChecks class that requires some sort of parameter for initialization wouldn't work very well?)

[dsherry]

Next step: agree on the plan. Seems like we got close here.

[dsherry]

A note: in relation to #848 (default params getter for components), @freddyaboulton and I just talked about how to encode default values for components. So whatever solution we come up with for this may be helpful for components/pipelines too.

[freddyaboulton]

@dsherry @angela97lin what are your thoughts on the following idea?

We can change the data type of data_checks to Optional[Union[str, List[DataCheck]]. This encapsulates the three things the user might want to do:

1. Use a default value -> Pass in "default". We can then map this to DefaultDataChecks.
2. Use their own checks -> Pass a list of DataCheck instances. (I agree with @angela97lin that it will be more user friendly if we just wrap their list in a DataChecks class).
3. Don't do any checks -> Pass in "disabled" or None. We can map this to EmptyDataChecks internally.

I think we should consider changing our defaults from None to something more explicit like "default" and only let the parameters that are truly optional be None. For example, as a user, I would be confused that setting bootstrap_type=None in CatBoostClassifier actually means "use a default bootstrapping strategy" as opposed to "don't do any bootstrapping".

[dsherry]

@freddyaboulton that makes sense!

Yes, I'm starting to like this idea of moving towards using string values to encode defaults rather than None. We're putting something similar in for objective and eventually problem_type in the automl search init (#871 is the first step)

So for the data_checks param to automl search:

• 'default' will specify the use of DefaultDataChecks, and will be the new default value
• We accept either a list of DataCheck instances or a DataChecks collection (and in either case our impl can standardize to DataChecks so we can call DataChecks.validate)
• None will disable the checks entirely

@freddyaboulton this lines up with what you said, yes?

As for elsewhere in the codebase: let's get this done initially just for the data checks. But if we like the pattern and find it applicable elsewhere, we should make it happen.

[kmax12]

in any situation where we might do something different based on the data type, i think auto would be better than default. in the case of data checks, we might run different data checks based on the problem type and i feel auto captures that aspect better than default which could be assumed to be static.

[freddyaboulton]

Good point!

[dsherry]

@kmax12 that's a good point. For data_check I'm totally on board with using 'auto' to signify the default.

To me, 'default' implies retrieving a default value whereas 'auto' implies there could be more logic going on behind the scenes, which makes it a good for some of the automl search parameters.

Thanks!