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

Added excluded_model_families parameter to AutoMLSearch() #4196

Merged
merged 15 commits into from
Jun 6, 2023

Conversation

christopherbunn
Copy link
Contributor

@christopherbunn christopherbunn commented May 31, 2023

Resolves #4197

@christopherbunn christopherbunn changed the title Add exclude_model_families parameter to AutoMLSearch() Added exclude_model_families parameter to AutoMLSearch() May 31, 2023
@christopherbunn christopherbunn marked this pull request as ready for review May 31, 2023 15:56
@codecov
Copy link

codecov bot commented May 31, 2023

Codecov Report

Merging #4196 (16395bd) into main (1591ca7) will decrease coverage by 0.0%.
The diff coverage is 98.7%.

@@           Coverage Diff           @@
##            main   #4196     +/-   ##
=======================================
- Coverage   99.7%   99.7%   -0.0%     
=======================================
  Files        349     349             
  Lines      38159   38229     +70     
=======================================
+ Hits       38042   38111     +69     
- Misses       117     118      +1     
Impacted Files Coverage Δ
...lml/automl/automl_algorithm/iterative_algorithm.py 97.5% <ø> (-<0.1%) ⬇️
...valml/automl/automl_algorithm/default_algorithm.py 99.7% <75.0%> (-0.3%) ⬇️
evalml/automl/automl_algorithm/automl_algorithm.py 100.0% <100.0%> (ø)
evalml/automl/automl_search.py 99.7% <100.0%> (+0.1%) ⬆️
evalml/pipelines/components/utils.py 96.5% <100.0%> (+0.2%) ⬆️
evalml/tests/automl_tests/test_automl.py 99.6% <100.0%> (+0.1%) ⬆️
...ts/automl_tests/test_automl_iterative_algorithm.py 100.0% <100.0%> (ø)
...valml/tests/automl_tests/test_default_algorithm.py 100.0% <100.0%> (ø)
...lml/tests/automl_tests/test_iterative_algorithm.py 100.0% <100.0%> (ø)
evalml/tests/pipeline_tests/test_pipeline_utils.py 99.6% <100.0%> (+0.1%) ⬆️

@eccabay
Copy link
Contributor

eccabay commented May 31, 2023

@christopherbunn what are your thoughts on removing allowed_model_families as part of this work?

If we don't, it may be possible to introduce some weird conflicts using the iterative algorithm where a model family is simultaneously allowed and disallowed. It's been a weird, semi-deprecated argument for a while anyways, since it's only usable in the iterative algorithm case but we still allow users to set it in the default algorithm case. Open to discussion, of course.

evalml/pipelines/components/utils.py Outdated Show resolved Hide resolved
evalml/automl/automl_search.py Outdated Show resolved Hide resolved
@christopherbunn christopherbunn changed the title Added exclude_model_families parameter to AutoMLSearch() Added excluded_model_families parameter to AutoMLSearch() Jun 1, 2023
@christopherbunn
Copy link
Contributor Author

@eccabay re:allowed_model_families, I think we can keep it and add logic to make sure that both of these parameters are not set. I can see cases where I would want to have this set, even if we don't currently use it anywhere else.

@christopherbunn christopherbunn force-pushed the TML-7567_exclude_model_families branch from bb94287 to 285999e Compare June 1, 2023 15:29
Copy link
Contributor

@eccabay eccabay left a comment

Choose a reason for hiding this comment

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

LGTM, just have some nits about our handling of the "not set" case

evalml/automl/automl_algorithm/iterative_algorithm.py Outdated Show resolved Hide resolved
evalml/automl/automl_algorithm/iterative_algorithm.py Outdated Show resolved Hide resolved
@christopherbunn christopherbunn force-pushed the TML-7567_exclude_model_families branch from 2ea42a0 to c0a13c2 Compare June 5, 2023 14:51
Copy link
Collaborator

@jeremyliweishih jeremyliweishih left a comment

Choose a reason for hiding this comment

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

LGTM just wondering if we should centralize the validation of allowing and excluding model families. LMK what you think!

evalml/automl/automl_algorithm/iterative_algorithm.py Outdated Show resolved Hide resolved
evalml/automl/automl_search.py Outdated Show resolved Hide resolved
evalml/automl/automl_search.py Show resolved Hide resolved
@christopherbunn christopherbunn force-pushed the TML-7567_exclude_model_families branch from e8368af to 16395bd Compare June 6, 2023 18:22
@christopherbunn christopherbunn enabled auto-merge (squash) June 6, 2023 18:41
@christopherbunn christopherbunn merged commit af53c15 into main Jun 6, 2023
@christopherbunn christopherbunn deleted the TML-7567_exclude_model_families branch June 6, 2023 18:54
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.

Enhance AutoMLSearch API to Exclude Estimators
3 participants