Skip to content

Commit

Permalink
Merge pull request #626 from bouthilx/feature/optional_evc
Browse files Browse the repository at this point in the history
Add enable option for EVC, with default = false
  • Loading branch information
bouthilx committed Jul 29, 2021
2 parents e409a89 + bb4e2b6 commit 4d89c20
Show file tree
Hide file tree
Showing 21 changed files with 605 additions and 217 deletions.
8 changes: 8 additions & 0 deletions src/orion/core/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,14 @@ def define_evc_config(config):
# TODO: This should be built automatically like get_branching_args_group
# After this, the cmdline parser should be built based on config.

evc_config.add_option(
"enable",
option_type=bool,
default=False,
env_var="ORION_EVC_ENABLE",
help="Enable the Experiment Version Control. Defaults to False.",
)

evc_config.add_option(
"auto_resolution",
option_type=bool,
Expand Down
11 changes: 11 additions & 0 deletions src/orion/core/cli/evc.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,15 @@
from orion.core.evc.conflicts import Resolution


def _add_enable_argument(parser):
parser.add_argument(
"--enable-evc",
action="store_true",
default=None,
help="Enable the Experiment Version Control.",
)


def _add_auto_resolution_argument(parser):
parser.add_argument(
"--auto-resolution",
Expand Down Expand Up @@ -106,6 +115,7 @@ def _add_branch_to_argument(parser, resolution_class):


resolution_arguments = {
"enable": _add_enable_argument,
"auto_resolution": _add_auto_resolution_argument,
"manual_resolution": _add_manual_resolution_argument,
"non_monitored_arguments": _add_non_monitored_arguments_argument,
Expand Down Expand Up @@ -133,6 +143,7 @@ def get_branching_args_group(parser):
description="Arguments to automatically resolved branching events.",
)

_add_enable_argument(branching_args_group)
_add_manual_resolution_argument(branching_args_group)
_add_non_monitored_arguments_argument(branching_args_group)
_add_ignore_code_changes_argument(branching_args_group)
Expand Down
12 changes: 7 additions & 5 deletions src/orion/core/cli/hunt.py
Original file line number Diff line number Diff line change
Expand Up @@ -198,8 +198,10 @@ def main(args):

signal.signal(signal.SIGTERM, _handler)

workon(
experiment,
ignore_code_changes=config["branching"].get("ignore_code_changes"),
**worker_config
)
# If EVC is not enabled, we force Consumer to ignore code changes.
if not config["branching"].get("enable", orion.core.config.evc.enable):
ignore_code_changes = True
else:
ignore_code_changes = config["branching"].get("ignore_code_changes")

workon(experiment, ignore_code_changes=ignore_code_changes, **worker_config)
12 changes: 9 additions & 3 deletions src/orion/core/evc/conflicts.py
Original file line number Diff line number Diff line change
Expand Up @@ -1169,9 +1169,15 @@ def detect(cls, old_config, new_config, branching_config=None):
old_hash_commit = old_config["metadata"].get("VCS", None)
new_hash_commit = new_config["metadata"].get("VCS")

ignore_code_changes = branching_config is not None and branching_config.get(
"ignore_code_changes", False
)
# Will be overriden by global config if not set in branching_config
ignore_code_changes = None
# Try using user defined ignore_code_changes
if branching_config is not None:
ignore_code_changes = branching_config.get("ignore_code_changes", None)
# Otherwise use global conf's ignore_code_changes
if ignore_code_changes is None:
ignore_code_changes = orion.core.config.evc.ignore_code_changes

if ignore_code_changes:
log.debug("Ignoring code changes")
if (
Expand Down
7 changes: 5 additions & 2 deletions src/orion/core/io/experiment_branch_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,9 @@ class ExperimentBranchBuilder:
"""

def __init__(self, conflicts, manual_resolution=None, **branching_arguments):
def __init__(
self, conflicts, enabled=True, manual_resolution=None, **branching_arguments
):
# TODO: handle all other arguments
if manual_resolution is None:
manual_resolution = orion.core.config.evc.manual_resolution
Expand All @@ -69,7 +71,8 @@ def __init__(self, conflicts, manual_resolution=None, **branching_arguments):
self.branching_arguments = branching_arguments

self.conflicting_config.update(branching_arguments)
self.resolve_conflicts()
if enabled:
self.resolve_conflicts()

@property
def experiment_config(self):
Expand Down
59 changes: 38 additions & 21 deletions src/orion/core/io/experiment_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -200,28 +200,14 @@ def build(name, version=None, branching=None, **config):

conflicts = _get_conflicts(experiment, branching)
must_branch = len(conflicts.get()) > 1 or branching.get("branch_to")
if must_branch:
if len(conflicts.get()) > 1:
log.debug("Experiment must branch because of conflicts")
else:
assert branching.get("branch_to")
log.debug("Experiment branching forced with ``branch_to``")
branched_experiment = _branch_experiment(
experiment, conflicts, version, branching
)
log.debug("Now attempting registration of branched experiment in DB.")
try:
_register_experiment(branched_experiment)
log.debug("Branched experiment successfully registered in DB.")
except DuplicateKeyError as e:
log.debug(
"Experiment registration failed. This is likely due to a race condition "
"during branching. Now rolling back and re-attempting building "
"the branched experiment."
)
raise RaceCondition("There was a race condition during branching.") from e

return branched_experiment
if must_branch and branching.get("enable", orion.core.config.evc.enable):
return _attempt_branching(conflicts, experiment, version, branching)
else:
log.warning(
"Running experiment in a different state:\n%s",
_get_branching_status_string(conflicts, branching),
)

log.debug("No branching required.")

Expand Down Expand Up @@ -609,6 +595,36 @@ def _update_experiment(experiment):
log.debug("Experiment configuration successfully updated in DB.")


def _attempt_branching(conflicts, experiment, version, branching):
if len(conflicts.get()) > 1:
log.debug("Experiment must branch because of conflicts")
else:
assert branching.get("branch_to")
log.debug("Experiment branching forced with ``branch_to``")
branched_experiment = _branch_experiment(experiment, conflicts, version, branching)
log.debug("Now attempting registration of branched experiment in DB.")
try:
_register_experiment(branched_experiment)
log.debug("Branched experiment successfully registered in DB.")
except DuplicateKeyError as e:
log.debug(
"Experiment registration failed. This is likely due to a race condition "
"during branching. Now rolling back and re-attempting building "
"the branched experiment."
)
raise RaceCondition("There was a race condition during branching.") from e

return branched_experiment


def _get_branching_status_string(conflicts, branching_arguments):
experiment_brancher = ExperimentBranchBuilder(
conflicts, enabled=False, **branching_arguments
)
branching_prompt = BranchingPrompt(experiment_brancher)
return branching_prompt.get_status()


def _branch_experiment(experiment, conflicts, version, branching_arguments):
"""Create a new branch experiment with adapters for the given conflicts"""
experiment_brancher = ExperimentBranchBuilder(conflicts, **branching_arguments)
Expand Down Expand Up @@ -720,6 +736,7 @@ def build_from_args(cmdargs):
:func:`orion.core.io.experiment_builder.build` for more information on experiment creation.
"""

cmd_config = get_cmd_config(cmdargs)

if "name" not in cmd_config:
Expand Down
6 changes: 1 addition & 5 deletions src/orion/core/io/resolve_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -99,14 +99,10 @@ def fetch_config_from_cmdargs(cmdargs):
)
cmdargs_config["worker.max_trials"] = cmdargs.pop("worker_trials")

mappings = dict(
experiment=dict(exp_max_broken="max_broken", exp_max_trials="max_trials"),
worker=dict(worker_max_broken="max_broken", worker_max_trials="max_trials"),
)

mappings = dict(
experiment=dict(max_broken="exp_max_broken", max_trials="exp_max_trials"),
worker=dict(max_broken="worker_max_broken", max_trials="worker_max_trials"),
evc=dict(enable="enable_evc"),
)

global_config = orion.core.config.to_dict()
Expand Down
13 changes: 9 additions & 4 deletions src/orion/core/worker/consumer.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ def __init__(
if interrupt_signal_code is None:
interrupt_signal_code = orion.core.config.worker.interrupt_signal_code

# NOTE: If ignore_code_changes is None, we can assume EVC is enabled.
if ignore_code_changes is None:
ignore_code_changes = orion.core.config.evc.ignore_code_changes

Expand Down Expand Up @@ -235,9 +236,6 @@ def _consume(self, trial, workdirname):
return results_file

def _validate_code_version(self):
if self.ignore_code_changes:
return

old_config = self.experiment.configuration
new_config = copy.deepcopy(old_config)
new_config["metadata"]["VCS"] = infer_versioning_metadata(
Expand All @@ -248,10 +246,17 @@ def _validate_code_version(self):
from orion.core.evc.conflicts import CodeConflict

conflicts = list(CodeConflict.detect(old_config, new_config))
if conflicts:
if conflicts and not self.ignore_code_changes:
raise BranchingEvent(
f"Code changed between execution of 2 trials:\n{conflicts[0]}"
)
elif conflicts:
log.warning(
"Code changed between execution of 2 trials. Enable EVC with option "
"`ignore_code_changes` set to False to raise an error when trials are executed "
"with different versions. For more information, see documentation at "
"https://orion.readthedocs.io/en/stable/user/config.html#experiment-version-control"
)

# pylint: disable = no-self-use
def execute_process(self, cmd_args, environ):
Expand Down
4 changes: 2 additions & 2 deletions tests/functional/algos/test_algos.py
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ def test_with_evc(algorithm):
space=space_with_fidelity,
algorithms=algorithm,
max_trials=30,
branching={"branch_from": "exp"},
branching={"branch_from": "exp", "enable": True},
)

assert exp.version == 2
Expand Down Expand Up @@ -277,7 +277,7 @@ def test_parallel_workers(algorithm):
name=name,
space=space_with_fidelity,
algorithms=algorithm,
branching={"branch_from": name},
branching={"branch_from": name, "enable": True},
)

assert exp.version == 2
Expand Down
Loading

0 comments on commit 4d89c20

Please sign in to comment.