-
Notifications
You must be signed in to change notification settings - Fork 73
Introduce mock_trial_data for MockEnv and add some basic Scheduler tests #980
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
base: main
Are you sure you want to change the base?
Introduce mock_trial_data for MockEnv and add some basic Scheduler tests #980
Conversation
for more information, see https://pre-commit.ci
…cheduler config examples.
for more information, see https://pre-commit.ci
…cheduler config examples (#981) # Pull Request ## Title Refactor some test fixtures for better reuse so we can test loading Scheduler config examples ______________________________________________________________________ ## Description - Moves temporary file based sqlite Storage fixture to a reusable location. - Move Optimizer fixtures to reusable location following Storage fixtures pattern. - Uses both in setting up TrialRunner fixtures for testing Scheduler config examples. - Also be sure to load all test configs too. ______________________________________________________________________ ## Type of Change - 🔄 Refactor - 🧪 Tests ______________________________________________________________________ ## Testing - Introduces new CI test for Scheduler configs that follows the usual pattern for other objects (Storage, Optimizer, Environments, Services, etc.) ______________________________________________________________________ ## Additional Notes (optional) - Split out from #980, #973 ______________________________________________________________________ --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Sergiy Matusevych <sergiym@microsoft.com>
mlos_bench/mlos_bench/tests/config/schedulers/test_load_scheduler_config_examples.py
Outdated
Show resolved
Hide resolved
…ler_config_examples.py
trial_data, | ||
), f"Trial {trial_id} was not registered in the optimizer." | ||
|
||
# TODO: And check the intermediary results. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thinking I will push this part off to #973. Thoughts?
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
@@ -22,7 +22,7 @@ | |||
from mlos_bench.storage.sql.schema import DbSchema | |||
from mlos_bench.storage.sql.trial import Trial | |||
from mlos_bench.tunables.tunable_groups import TunableGroups | |||
from mlos_bench.util import utcify_timestamp | |||
from mlos_bench.util import try_parse_val, utcify_timestamp |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Work to return numeric values for metrics and configs if possible.
TODO: Split this change out to a separate PR.
Column("ts_start", DateTime), | ||
Column("ts_end", DateTime), | ||
Column("ts_start", DateTime().with_variant(mysql.DATETIME(fsp=6), "mysql")), | ||
Column("ts_end", DateTime().with_variant(mysql.DATETIME(fsp=6), "mysql")), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Work to allow MySQL to store fraction time.
TODO: Split this out to its own PR
"trial_runner_id": None, | ||
"experiment_id": "SomeExperimentName", | ||
"trial_id": 1, | ||
"trial_runner_id": 0, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes to handle a non-null trial_id requirement for MockEnv
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO: Maybe split this out to its own PR too.
# Pull Request ## Title Small refactor to `Status.parse` ______________________________________________________________________ ## Description - Rename `Status.from_dict` -> `Status.parse` - Allow `Status.parse` to accept a `Status` as an argument and return it as is. (complete convenience for some python comprehension code elsewhere) - Add a test for that. ______________________________________________________________________ ## Type of Change - 🔄 Refactor ______________________________________________________________________ ## Testing Small new CI check and existing tests. ______________________________________________________________________ ## Additional Notes (optional) For convenience in another PR - #980 ______________________________________________________________________ --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Sergiy Matusevych <sergiym@microsoft.com>
Pull Request
Title
Introduce
mock_trial_data
and add some basic Scheduler testsDescription
mock_trial_data
toMockEnv
to allow testing changes to the base class more explicitly.TODO: Split out:
Type of Change
Testing
Additional Notes (optional)
Status.parse
#982 first.