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

Add support for storing the optimization targets and direction of an experiment #628

Merged

Conversation

bpkroth
Copy link
Contributor

@bpkroth bpkroth commented Jan 10, 2024

This PR is useful for mlos-viz and dabl wrapper (#624) to be able to automatically graph the results for a given optimization target, for instance via something like the following:

for opt_target in exp.objectives:
    dabl.plot(exp.results, opt_target)

Since the prior efforts on capturing this data in the Trial metadata are somewhat problematic (allow conflicting changes between runs of an experiment, don't support multi-objective), we extend them to also store values directly as a part of the Experiment, which is a somewhat more appropriate location. Upon retrieval, an attempt is also made to merge the two data sources for backwards compatibility.

This PR does not enforce strictness on that metadata, but future versions could (e.g., disallow resuming an Experiment if it looks like the objective targets have changed. In that case the prior Trial results can potentially still be used to prewarm a new Experiment's optimizer).

@bpkroth bpkroth requested a review from a team as a code owner January 10, 2024 21:46
@motus
Copy link
Member

motus commented Jan 10, 2024

wait, we already store that data in the trial_param table - see

trial = exp.new_trial(tunables, config={
"optimizer": opt.name,
"opt_target": opt.target,
"opt_direction": "min" if opt.is_min else "max",
})

you can add more key/value pairs to store with each trial there

@bpkroth bpkroth added the WIP Work in progress - do not merge yet label Jan 10, 2024
@bpkroth
Copy link
Contributor Author

bpkroth commented Jan 11, 2024

wait, we already store that data in the trial_param table - see

trial = exp.new_trial(tunables, config={
"optimizer": opt.name,
"opt_target": opt.target,
"opt_direction": "min" if opt.is_min else "max",
})

you can add more key/value pairs to store with each trial there

We had a conversation about this offline. I think the conclusion was that:

  1. The optimization configuration is really a property of the Experiment and not the Trial, so if something about the Experiment's optimization goal's configuration changes, we should really require a new Experiment, though previously we didn't store or enforce this.
  2. That doesn't preclude reusing some TrialData from a previous Experiment to pre-warm a new one.
  3. However, storing metadata about the Optimization goals used during a given Trial, which potentially redundant, may still be useful.
    Though, with that in mind, we may also want to store other parameter data used too ...

I think with that in mind, I will still pursue this PR and a future one can enforce the strictness aspects of not changing things, perhaps after we make it easier to merge other "related" TrialData (though capturing which those are might itself be tricky).

@bpkroth bpkroth marked this pull request as draft January 11, 2024 18:48
@bpkroth bpkroth marked this pull request as ready for review January 11, 2024 21:34
@bpkroth bpkroth changed the title Add support for storing the optimization target and direction of an experiment Add support for storing the optimization targets and direction of an experiment Jan 11, 2024
@bpkroth bpkroth added ready for review Ready for review and removed WIP Work in progress - do not merge yet labels Jan 11, 2024
mlos_bench/mlos_bench/run.py Outdated Show resolved Hide resolved
mlos_bench/mlos_bench/storage/sql/trial.py Outdated Show resolved Hide resolved
mlos_bench/mlos_bench/storage/base_storage.py Outdated Show resolved Hide resolved
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.

Looks good! I leave it up to you to keep the optimization direction nullable or roll back to the non-nullable variant

mlos_bench/mlos_bench/storage/base_experiment_data.py Outdated Show resolved Hide resolved
bpkroth and others added 2 commits January 16, 2024 18:00
@bpkroth bpkroth enabled auto-merge (squash) January 16, 2024 18:02
@bpkroth bpkroth merged commit 221cee3 into microsoft:main Jan 16, 2024
11 checks passed
@bpkroth bpkroth deleted the store-and-expose-optimization-target-info branch January 16, 2024 18:11
bpkroth added a commit that referenced this pull request Jan 16, 2024
Adds a basic `mlos_viz.plot(exp)` style API for simple visualizations of
`ExperimentData` results relative to the experiment's objectives
(building off of #628 and dabl/dabl#335).

Note: this PR currently omits unit tests for the new module due to the
complexity of testing visualizations. We intend to add this in future
PRs. There is however, a working example of its use here right now:
Microsoft-CISL/sqlite-autotuning#41

---------

Co-authored-by: Sergiy Matusevych <sergiy.matusevych@gmail.com>
motus added a commit that referenced this pull request Jan 26, 2024
#628 mistakenly included an early attempt at adding
`optimization_target` and `optimization_direction` to the `experiment`
table in the `mlos_bench.storage.sql` backend.

In that PR we later moved it to its own `objectives` table to eventually
support multi-objectives.

Nothing accesses those columns now, however including them in the
metadata makes it impossible to load storage backends previously created
with the old schema since adjusting columns with sqlalchemy's
`create_all()` API only considers table existence.

On the contrary, the latter means that we will automatically support old
storage backends with the new code for the `objectives` table.

Removing these two columns in the `metadata` schema description simply
allows that to proceed without error.

See Also: #649

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
ready for review Ready for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants