Skip to content

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

Draft
wants to merge 84 commits into
base: main
Choose a base branch
from

Conversation

bpkroth
Copy link
Contributor

@bpkroth bpkroth commented May 19, 2025

Pull Request

Title

Introduce mock_trial_data and add some basic Scheduler tests


Description

  • Adds mock_trial_data to MockEnv to allow testing changes to the base class more explicitly.
  • Adds some very basic Scheduler tests.
    • Especially for Scheduler book keeping and basic behavior (e.g., run the trial)

TODO: Split out:

  • Fix DB Schema for MySQL to store fractional seconds
  • Type coherce metrics and params as numerics instead of only strings upon retrieval

Type of Change

  • ✨ New feature
  • 🔄 Refactor
  • 🧪 Tests

Testing

  • Adds new tests and existing ones.

Additional Notes (optional)


@bpkroth bpkroth added the WIP Work in progress - do not merge yet label May 19, 2025
motus added a commit that referenced this pull request May 20, 2025
…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>
@bpkroth bpkroth added ready for review Ready for review and removed WIP Work in progress - do not merge yet labels May 22, 2025
@bpkroth bpkroth changed the title WIP: Introduce mock_trial_data for MockEnv and add some basic Scheduler tests Introduce mock_trial_data for MockEnv and add some basic Scheduler tests May 22, 2025
trial_data,
), f"Trial {trial_id} was not registered in the optimizer."

# TODO: And check the intermediary results.
Copy link
Contributor Author

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?

@@ -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
Copy link
Contributor Author

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")),
Copy link
Contributor Author

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,
Copy link
Contributor Author

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

Copy link
Contributor Author

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.

@bpkroth bpkroth removed the ready for review Ready for review label Jun 2, 2025
motus added a commit that referenced this pull request Jun 12, 2025
# 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>
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.

1 participant