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

Refactor mlos_bench.storage and add TunableConfigTrialGroup property for TrialData and ExperimentData #648

Merged
merged 36 commits into from
Jan 23, 2024

Conversation

bpkroth
Copy link
Contributor

@bpkroth bpkroth commented Jan 19, 2024

Useful for grouping trials by the config they used. In use by upcoming #633 for generating graphs with variance error bars for repeated configs.

Also refactors a number of other things:

  • Standardize on experiment_id instead of exp_id
    (but not the db schema for now)
  • Standardize on tunable_config_id instead of config_id in the API since we also call it tunable_config for the object fetching property to distinguish from the config dict used internally.
    (but not the db schema for now)
  • Rework the idea of TunableConfigTrialGroup as an object inaddition to an ID (further methods can be added later to move back and forth between types when doing interactive analysis).
  • Rework the idea of a TunableConfig as an object for fetching tunable value assignments (similar justification - easier grouping in the future by fetching trial across experiments based on config - eventually could be used to house the experiment merge logic).
  • Rename results APIs to results_df (similar for others that return pandas.DataFrame) to match the results_dict that return dict
  • Refactor test fixtures to match other styles and for future use (moved to Remove TunableGroups from Storage classes and add TrialData tests #644).
  • Expand tests

NOTE:

  • We cut a new version with this commit since there are potentially breaking API changes (e.g., results -> to results_df and exp_id -> experiment_id).

Currently builds off of #644 and splits work out of #633

bpkroth and others added 9 commits January 19, 2024 00:12
Work split out from microsoft#633

- Reorgs the unit test fixtures for reuse by mlos_viz (future PR)
- Removes TunablesGroups from Storage classes.
  Currently unnecessary, belongs in Experiment, and causes inconsistencies with ExperimentData interactions.
- Adjusts the initialization code to match.
@bpkroth bpkroth requested a review from a team as a code owner January 19, 2024 21:54
@bpkroth bpkroth marked this pull request as draft January 22, 2024 14:50
bpkroth and others added 6 commits January 22, 2024 08:50
- Standardize on experiment_id instead of exp_id
  (but not the db schema for now)
- Standardize on tunable_config_id instead of config_id in the API
  (but not the db schema for now)
- Rework the idea of TunableConfigTrialGroup as an object.
- Rework the idea of a TunableConfig as an object.
- Rename results APIs to results_df (similar for others that return dataframes)
- Refactor test fixtures to match other styles and for future use.
- Expand tests
@bpkroth bpkroth changed the title Add config_trial_group_id property for TrialData and ExperimentData Add TunableConfigTrialGroup property for TrialData and ExperimentData Jan 23, 2024
@bpkroth bpkroth marked this pull request as ready for review January 23, 2024 13:17
@bpkroth bpkroth changed the title Add TunableConfigTrialGroup property for TrialData and ExperimentData Refactor mlos_bench.storage and add TunableConfigTrialGroup property for TrialData and ExperimentData Jan 23, 2024
Copy link
Member

@motus motus left a comment

Choose a reason for hiding this comment

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

LGTM!

@bpkroth bpkroth enabled auto-merge (squash) January 23, 2024 23:04
@bpkroth bpkroth merged commit d8ea64f into microsoft:main Jan 23, 2024
11 checks passed
@bpkroth bpkroth deleted the add-config-trial-group-id branch January 23, 2024 23:13
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.

2 participants