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

Sweeper PR #214

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 67 additions & 0 deletions tango/sweeps.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
import itertools
from collections import OrderedDict
from dataclasses import dataclass

from tango.common import Params, Registrable
from tango.common.testing import run_experiment

main_config_path = (
"/Users/sabhyac/Desktop/sabhya/tango/test_fixtures/sweeps/basic_test/config.jsonnet"
)
sweeps_config_path = (
"/Users/sabhyac/Desktop/sabhya/tango/test_fixtures/sweeps/basic_test/sweeps-config.jsonnet"
)
components = (
"/Users/sabhyac/Desktop/sabhya/tango/test_fixtures/sweeps/basic_test/basic_arithmetic.py"
)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
main_config_path = (
"/Users/sabhyac/Desktop/sabhya/tango/test_fixtures/sweeps/basic_test/config.jsonnet"
)
sweeps_config_path = (
"/Users/sabhyac/Desktop/sabhya/tango/test_fixtures/sweeps/basic_test/sweeps-config.jsonnet"
)
components = (
"/Users/sabhyac/Desktop/sabhya/tango/test_fixtures/sweeps/basic_test/basic_arithmetic.py"
)



class Sweeper(Registrable):
def __init__(self, main_config_path: str, sweeps_config_path: str, components: str):
Copy link
Member

Choose a reason for hiding this comment

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

The Sweeper will also need to be aware of the target_step that spits out metric we're trying to optimize for, and the workspace.

Suggested change
def __init__(self, main_config_path: str, sweeps_config_path: str, components: str):
def __init__(self, main_config_path: str, sweeps_config_path: str, components: str, target_step: str, workspace: Workspace):

Copy link
Member

Choose a reason for hiding this comment

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

You may want to start by implementing #142 in a separate PR

super(Registrable, self).__init__()
Copy link
Member

Choose a reason for hiding this comment

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

In recent versions of Python you can just do this:

Suggested change
super(Registrable, self).__init__()
super().__init__()

self.main_config_path = main_config_path
self.sweep_config = load_config(sweeps_config_path)
self.main_config_path = main_config_path
self.components = components

# returns all the combinations of hyperparameters in the form of a list of lists
def get_combinations(self):
hyperparams = self.sweep_config.config["hyperparameters"]
hyperparams_lsts = []
for val in hyperparams.values():
hyperparams_lsts.append(val)
hyperparam_combos = list(itertools.product(*hyperparams_lsts))
return hyperparam_combos

# TODO: trying to figure the best path forward? should i use tests?
def run_experiments(self):
hyperparam_combos = self.get_combinations()
for combination in hyperparam_combos:
main_config = self.override_hyperparameters(combination)
# TODO: need to figure where & how to store results / way to track runs
with run_experiment(main_config, include_package=[self.components]) as run_dir:
# TODO: fill in something here?
pass
Copy link
Member

Choose a reason for hiding this comment

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

run_experiment is just for testing. We probably want to run our sweep experiments in subprocesses so that we can execute them in parallel. You could do something like this:

Suggested change
main_config = self.override_hyperparameters(combination)
# TODO: need to figure where & how to store results / way to track runs
with run_experiment(main_config, include_package=[self.components]) as run_dir:
# TODO: fill in something here?
pass
subprocess.call(["tango", "run", self.main_config_path, "--overrides", json.dumps(overrides)])

Of course this doesn't make anything run in parallel, but would be a good first step.


# TODO: wondering if this function should be here or in a test_file?
def override_hyperparameters(self, experiment_tuple: tuple):
# Override all the hyperparameters in the current experiment_config
overrides = {}
for (i, key) in enumerate(self.sweep_config.config["hyperparameters"].keys()):
overrides[key] = experiment_tuple[i]
# load the config & override it
main_config = Params.from_file(self.main_config_path, params_overrides=overrides)
return main_config


# function that loads the config from a specified yaml or jasonnet file
# TODO: how do I read "wandb" form config and call appropriate class
def load_config(sweeps_config_path: str):
return SweepConfig.from_file(sweeps_config_path)


# data class that loads the parameters
# TODO: unsure about how to specify a default here?
@dataclass(frozen=True)
class SweepConfig(Params):
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
class SweepConfig(Params):
class SweepConfig(FromParams):

Copy link
Member

Choose a reason for hiding this comment

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

See

class TangoGlobalSettings(FromParams):
for example

config: OrderedDict
Loading