Skip to content

Commit

Permalink
Test consumer ignore_code_changes if evc enabled
Browse files Browse the repository at this point in the history
Why:

If the EVC is not enabled, the consumer should always ignore the code
changes. It would not make sense to raise an error between 2 trials
because user's script code changed while EVC is disabled.

How:

If EVC is disabled, force ignore_code_changes to True in Consumer.
  • Loading branch information
bouthilx committed Jul 23, 2021
1 parent 2426897 commit b716612
Show file tree
Hide file tree
Showing 6 changed files with 224 additions and 80 deletions.
12 changes: 7 additions & 5 deletions src/orion/core/cli/hunt.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,10 @@ def main(args):
if config.get("worker"):
worker_config.update(config.get("worker"))

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
13 changes: 9 additions & 4 deletions src/orion/core/worker/consumer.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,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 @@ -255,9 +256,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 @@ -268,10 +266,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
211 changes: 147 additions & 64 deletions tests/functional/configuration/test_all_options.py
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ def test_db_config(self, tmp_path):
with self.setup_db_config(tmp_path):
self.check_db_config()

@pytest.mark.usefixtures("with_user_userxyz")
@pytest.mark.usefixtures("with_user_userxyz", "version_XYZ")
def test_local_config(self, tmp_path, monkeypatch):
"""Test that local config overrides db/global config"""
update_singletons()
Expand Down Expand Up @@ -767,7 +767,7 @@ class TestEVCConfig(ConfigurationTestSuite):
"enable": False,
"manual_resolution": True,
"non_monitored_arguments": ["test", "local"],
"ignore_code_changes": True,
"ignore_code_changes": False,
"algorithm_change": True,
"code_change_type": "break",
"cli_change_type": "break",
Expand All @@ -780,7 +780,7 @@ class TestEVCConfig(ConfigurationTestSuite):
"enable-evc": True,
"manual-resolution": False,
"non-monitored-arguments": "test:cmdargs",
"ignore-code-changes": False,
"ignore-code-changes": True,
"algorithm-change": False,
"code-change-type": "noeffect",
"cli-change-type": "unsure",
Expand Down Expand Up @@ -830,14 +830,30 @@ def check_global_config(self, tmp_path, monkeypatch):
tmp_path, name, command, change_type="noeffect"
)

self._check_code_change(
monkeypatch,
name,
command,
mock_ignore_code_changes=None,
ignore_code_changes=self.config["evc"]["ignore_code_changes"],
change_type=self.config["evc"]["code_change_type"],
)
# EVC not enabled, code change should be ignored even if option is set to True
assert self.config["evc"]["enable"] is False
with monkeypatch.context() as m:
self._check_code_change(
m,
name,
command.replace("--enable-evc ", ""),
mock_ignore_code_changes=True,
ignore_code_changes=True,
change_type=self.config["evc"]["code_change_type"],
enable_evc=False,
)

# EVC is enabled, option should be honored
with monkeypatch.context() as m:
self._check_code_change(
m,
name,
command,
mock_ignore_code_changes=None,
ignore_code_changes=self.config["evc"]["ignore_code_changes"],
change_type=self.config["evc"]["code_change_type"],
enable_evc=True,
)

def check_env_var_config(self, tmp_path, monkeypatch):
"""Check that env vars overrides global configuration"""
Expand All @@ -856,7 +872,9 @@ def check_env_var_config(self, tmp_path, monkeypatch):
assert not orion.core.config.evc.orion_version_change

name = "env-var-test"
command = f"hunt --enable-evc --worker-max-trials 0 -n {name} python {script} -x~uniform(0,1)"
command = (
f"hunt --worker-max-trials 0 -n {name} python {script} -x~uniform(0,1)"
)
assert orion.core.cli.main(command.split(" ")) == 0

self._check_enable(name, command, enabled=True)
Expand All @@ -867,14 +885,34 @@ def check_env_var_config(self, tmp_path, monkeypatch):
)
self._check_script_config_change(tmp_path, name, command, change_type="unsure")

self._check_code_change(
monkeypatch,
name,
command,
mock_ignore_code_changes=None,
ignore_code_changes=bool(self.env_vars["ORION_EVC_IGNORE_CODE_CHANGES"]),
change_type=self.env_vars["ORION_EVC_CODE_CHANGE"],
)
# Enable EVC, ignore_code_changes is False
with monkeypatch.context() as m:
self._check_code_change(
m,
name,
command,
mock_ignore_code_changes=None,
ignore_code_changes=bool(
self.env_vars["ORION_EVC_IGNORE_CODE_CHANGES"]
),
change_type=self.env_vars["ORION_EVC_CODE_CHANGE"],
enable_evc=True,
)

# Disable EVC, ignore_code_changes is True for Consumer
os.environ["ORION_EVC_ENABLE"] = ""
with monkeypatch.context() as m:
self._check_code_change(
m,
name,
command,
mock_ignore_code_changes=None,
ignore_code_changes=bool(
self.env_vars["ORION_EVC_IGNORE_CODE_CHANGES"]
),
change_type=self.env_vars["ORION_EVC_CODE_CHANGE"],
enable_evc=False,
)

def check_db_config(self):
"""No Storage config in DB, no test"""
Expand Down Expand Up @@ -911,14 +949,30 @@ def check_local_config(self, tmp_path, conf_file, monkeypatch):
command,
change_type=self.local["evc"]["config_change_type"],
)
self._check_code_change(
monkeypatch,
name,
command,
mock_ignore_code_changes=True,
ignore_code_changes=self.local["evc"]["ignore_code_changes"],
change_type=self.local["evc"]["code_change_type"],
)

# enabled evc, ignore code changes so to True
with monkeypatch.context() as m:
self._check_code_change(
m,
name,
command,
mock_ignore_code_changes=False,
ignore_code_changes=self.local["evc"]["ignore_code_changes"],
change_type=self.local["evc"]["code_change_type"],
enable_evc=True,
)

# disabled evc, ignore code changes so to True
with monkeypatch.context() as m:
self._check_code_change(
m,
name,
command.replace("--enable-evc ", ""),
mock_ignore_code_changes=False,
ignore_code_changes=self.local["evc"]["ignore_code_changes"],
change_type=self.local["evc"]["code_change_type"],
enable_evc=False,
)

def check_cmd_args_config(self, tmp_path, conf_file, monkeypatch):
"""Check that cmdargs configuration overrides global/envvars/local configuration"""
Expand All @@ -927,29 +981,27 @@ def check_cmd_args_config(self, tmp_path, conf_file, monkeypatch):
f"hunt --worker-max-trials 0 -c {conf_file} -n {name} "
"--enable-evc "
"--auto-resolution "
"--non-monitored-arguments test:cmdargs "
"--code-change-type noeffect "
"--cli-change-type unsure "
"--config-change-type break "
f"python {script} -x~uniform(0,1)"
)
assert orion.core.cli.main(command.split(" ")) == 0

self._check_enable(name, command, enabled=True)

self._check_cli_change(
name, command, change_type=self.cmdargs["cli-change-type"]
name,
command="hunt --cli-change-type unsure " + command[5:],
change_type=self.cmdargs["cli-change-type"],
)
self._check_non_monitored_arguments(
name,
command,
command="hunt --non-monitored-arguments test:cmdargs " + command[5:],
non_monitored_arguments=self.cmdargs["non-monitored-arguments"].split(":"),
)

self._check_script_config_change(
tmp_path,
name,
command,
command="hunt --config-change-type break " + command[5:],
change_type=self.cmdargs["config-change-type"],
)

Expand All @@ -964,26 +1016,42 @@ def mock_local(cmdargs):
monkeypatch.setattr(orion.core.io.resolve_config, "fetch_config", mock_local)

# Check that ignore_code_changes is rightly False
self._check_code_change(
monkeypatch,
name,
command,
mock_ignore_code_changes=False,
ignore_code_changes=False,
change_type=self.cmdargs["code-change-type"],
)

command = "hunt --ignore-code-changes " + command[5:]

# Check that ignore_code_changes is now True
self._check_code_change(
monkeypatch,
name,
command,
mock_ignore_code_changes=True,
ignore_code_changes=True,
change_type=self.cmdargs["code-change-type"],
)
with monkeypatch.context() as m:
self._check_code_change(
m,
name,
command="hunt --code-change-type noeffect " + command[5:],
mock_ignore_code_changes=False,
ignore_code_changes=False,
change_type=self.cmdargs["code-change-type"],
enable_evc=True,
)

# Check that ignore_code_changes is now True because --ignore-code-changes was added
with monkeypatch.context() as m:
self._check_code_change(
m,
name,
command="hunt --ignore-code-changes --code-change-type noeffect "
+ command[5:],
mock_ignore_code_changes=True,
ignore_code_changes=True,
change_type=self.cmdargs["code-change-type"],
enable_evc=True,
)

# Check that ignore_code_changes is forced to True in consumer
# even if --ignore-code-changes is not passed
with monkeypatch.context() as m:
self._check_code_change(
m,
name,
command.replace("--enable-evc ", ""),
mock_ignore_code_changes=False,
ignore_code_changes=False,
change_type=self.cmdargs["code-change-type"],
enable_evc=False,
)

@with_storage_fork
def _check_enable(self, name, command, enabled):
Expand Down Expand Up @@ -1032,7 +1100,11 @@ def _check_code_change(
mock_ignore_code_changes,
ignore_code_changes,
change_type,
enable_evc,
):
"""Check if code changes are correctly ignored during experiment build and by consumer
between two trial executions.
"""

# Test that code change is handled with 'no-effect'
def fixed_dictionary(user_script):
Expand All @@ -1058,24 +1130,35 @@ def mock_detect(old_config, new_config, branching_config=None):
assert (
branching_config["ignore_code_changes"] is mock_ignore_code_changes
)
branching_config["ignore_code_changes"] = False
# branching_config["ignore_code_changes"] = False
return detect(old_config, new_config, branching_config)

monkeypatch.setattr(
orion.core.evc.conflicts.CodeConflict, "detect", mock_detect
)

experiment = get_experiment(name)

assert orion.core.cli.main(command.split(" ")) == 0
self._check_consumer({"ignore_code_changes": ignore_code_changes})
self._check_consumer(
{
"ignore_code_changes": (
(enable_evc and ignore_code_changes) or not enable_evc
)
}
)

new_experiment = get_experiment(name)
assert new_experiment.version == experiment.version + 1
assert new_experiment.refers["adapter"].configuration[0] == {
"of_type": "codechange",
"change_type": change_type,
}

monkeypatch.undo()
if enable_evc and not ignore_code_changes:
assert new_experiment.version == experiment.version + 1
assert new_experiment.refers["adapter"].configuration[0] == {
"of_type": "codechange",
"change_type": change_type,
}
elif enable_evc: # But code change ignored, so no branching event.
assert new_experiment.version == experiment.version
else:
assert new_experiment.version == experiment.version

@with_storage_fork
def _check_script_config_change(self, tmp_path, name, command, change_type):
Expand Down
Loading

0 comments on commit b716612

Please sign in to comment.