Populate hyperopt defaults using schema #2968
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
arnavgarg1
requested review from
justinxzhao,
tgaddair,
jeffkinnison and
ksbrar
justinxzhao
reviewed
Jan 20, 2023
abidwael
reviewed
Jan 20, 2023
justinxzhao
approved these changes
Jan 20, 2023
| @@ -207,7 +208,9 @@ def run_hyperopt_executor( | |||
| def test_hyperopt_executor(scenario, csv_filename, tmpdir, ray_cluster_4cpu): | |||
| search_alg = scenario["search_alg"] | |||
| executor = scenario["executor"] | |||
| run_hyperopt_executor(search_alg, executor, csv_filename, tmpdir) | |||
| # When using the hb_bohb scheduler, num_epochs must equal max_t (which is 81 by default) | |||
There was a problem hiding this comment.
How did you come to discover this? Is this worth adding as an auxiliary validation?
There was a problem hiding this comment.
I didn't know about this either, but found out that we've added this logic here in our ModelConfig object creation logic: https://github.com/ludwig-ai/ludwig/blob/master/ludwig/schema/model_config.py#L532
tgaddair
reviewed
Jan 20, 2023
tgaddair
reviewed
Jan 20, 2023
jeffkinnison
approved these changes
Jan 20, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds a new step to ensure that all of the defaults for search algorithm, split, executor etc. are correctly populated using the schema object before accessing them to prevent key errors. All of these parameters are required by hyperopt and expected to be present downstream for config validation, executor creation, and trial execution.
The reason we've not noticed this before in our tests is because all of our tests specify
search_algin the hyperopt config.Co-authored-by: Kabir Brar kabir@predibase.com
Co-authored-by: Travis Addair tgaddair@gmail.com