From 6a9c15b97a7f410ae5e0cd0447f5e5407fd158be Mon Sep 17 00:00:00 2001 From: Setepenre Date: Thu, 18 Aug 2022 13:31:24 -0400 Subject: [PATCH 1/4] Revert Subprocess pipe (#985) * Revert Subprocess pipe * Test that child stdout is printed --- src/orion/core/worker/consumer.py | 10 +------ tests/functional/demo/black_box.py | 12 ++++++++ tests/functional/demo/orion_black_box.py | 37 ++++++++++++++++++++++++ tests/functional/demo/test_demo.py | 23 +++++++++++++++ 4 files changed, 73 insertions(+), 9 deletions(-) create mode 100644 tests/functional/demo/orion_black_box.py diff --git a/src/orion/core/worker/consumer.py b/src/orion/core/worker/consumer.py index 187d32cca..c5e2bc1f9 100644 --- a/src/orion/core/worker/consumer.py +++ b/src/orion/core/worker/consumer.py @@ -253,21 +253,13 @@ def execute_process(self, cmd_args, environ): try: # pylint: disable = consider-using-with - process = subprocess.Popen( - command, - env=environ, - stdout=subprocess.PIPE, - stderr=subprocess.STDOUT, - ) + process = subprocess.Popen(command, env=environ) except PermissionError as exc: log.debug("Script is not executable") raise InexecutableUserScript(" ".join(cmd_args)) from exc - stdout, _ = process.communicate() - return_code = process.wait() log.debug(f"Script finished with return code {return_code}") if return_code != 0: - log.debug("%s", stdout.decode("utf-8")) raise ExecutionError(return_code) diff --git a/tests/functional/demo/black_box.py b/tests/functional/demo/black_box.py index d2ee5e79f..dbae0ce05 100755 --- a/tests/functional/demo/black_box.py +++ b/tests/functional/demo/black_box.py @@ -2,9 +2,12 @@ """Simple one dimensional example for a possible user's script.""" import argparse import os +import sys from orion.client import report_results +PRINT = os.environ.get("ORION_PRINT", "False") == "True" + def function(x): """Evaluate partial information of a quadratic.""" @@ -14,6 +17,9 @@ def function(x): def execute(): """Execute a simple pipeline as an example.""" + if PRINT: + print("Start Child Process") + # 1. Receive inputs as you want parser = argparse.ArgumentParser() parser.add_argument("-x", type=float, required=True) @@ -36,6 +42,9 @@ def execute(): # 2. Perform computations y, dy = function(inputs.x) + if PRINT: + print("Child Error", file=sys.stderr) + # 3. Gather and report results results = list() results.append(dict(name="example_objective", type="objective", value=y)) @@ -43,6 +52,9 @@ def execute(): report_results(results) + if PRINT: + print("End Child Process") + if __name__ == "__main__": execute() diff --git a/tests/functional/demo/orion_black_box.py b/tests/functional/demo/orion_black_box.py new file mode 100644 index 000000000..ed7432f29 --- /dev/null +++ b/tests/functional/demo/orion_black_box.py @@ -0,0 +1,37 @@ +import os +import sys + +import orion +import orion.core.cli +from orion.testing import OrionState + + +def main(): + """This is needed because the user script runs in a separate process, so python + cannot capture its output. + + Here we run the entire orion script in its own process so we can capture all of its output. + """ + os.chdir(os.path.dirname(os.path.abspath(__file__))) + + print("Start main process") + os.environ["ORION_PRINT"] = "True" + with OrionState(): + orion.core.cli.main( + [ + "hunt", + "-n", + "default_algo", + "--exp-max-trials", + "5", + "./black_box.py", + "-x~uniform(0, 5, discrete=True)", + ] + ) + + print("Main process Error", file=sys.stderr) + print("End main process") + + +if __name__ == "__main__": + main() diff --git a/tests/functional/demo/test_demo.py b/tests/functional/demo/test_demo.py index bb400156b..385e23e9b 100644 --- a/tests/functional/demo/test_demo.py +++ b/tests/functional/demo/test_demo.py @@ -3,6 +3,7 @@ import os import shutil import subprocess +import sys import tempfile from collections import defaultdict from contextlib import contextmanager @@ -52,6 +53,28 @@ def test_demo_with_default_algo_cli_config_only(storage, monkeypatch): assert trials[-1].status == "completed" +def test_demo_stdout_stderr(): + """Check that script output is printed and shown to the end user""" + + dir = os.path.dirname(os.path.abspath(__file__)) + + process = subprocess.Popen( + [sys.executable, os.path.join(dir, "orion_black_box.py")], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + ) + stdout, stderr = process.communicate() + + stdout = stdout.decode("utf-8") + stderr = stderr.decode("utf-8") + + assert "Start main process" in stdout + assert "Start Child Process" in stdout + + assert "Child Error" in stderr + assert "Main process Error" in stderr + + def test_demo(storage, monkeypatch): """Test a simple usage scenario.""" monkeypatch.chdir(os.path.dirname(os.path.abspath(__file__))) From 90051884b1d7ceaaaa31f8a8985561a48ed3a17d Mon Sep 17 00:00:00 2001 From: Setepenre Date: Thu, 18 Aug 2022 13:53:02 -0400 Subject: [PATCH 2/4] Release preparations --- .pre-commit-config.yaml | 8 ++++---- README.rst | 4 ++-- ROADMAP.md | 8 +++++++- .../plotting/plot_4_partial_dependencies.py | 2 +- src/orion/core/evc/conflicts.py | 2 +- .../core/io/experiment_branch_builder.py | 2 +- src/orion/storage/track.py | 19 ++++++------------- .../backward_compatibility/versions.txt | 2 ++ .../unittests/core/io/test_resolve_config.py | 2 +- 9 files changed, 25 insertions(+), 24 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 03b51bde9..8859488b5 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -5,12 +5,12 @@ repos: hooks: - id: black - repo: https://github.com/codespell-project/codespell - rev: v2.1.0 + rev: v2.2.1 hooks: - id: codespell - args: ["--skip", "*.html,*.ipynb,dashboard/src/.yarn/**,dashboard/build/**,dashboard/src/src/__tests__/**", "--ignore-words-list=hist,wont"] + args: ["--skip", "*.html,*.ipynb,dashboard/src/.yarn/**,dashboard/src/yarn.lock,dashboard/build/**,dashboard/src/src/__tests__/**", "--ignore-words-list=hist,wont"] - repo: https://gitlab.com/pycqa/flake8 - rev: 3.9.2 + rev: 5.0.4 hooks: - id: flake8 args: @@ -25,7 +25,7 @@ repos: - id: isort args: ["--profile", "black"] - repo: https://github.com/asottile/pyupgrade - rev: v2.34.0 + rev: v2.37.3 hooks: - id: pyupgrade args: ["--py37-plus"] diff --git a/README.rst b/README.rst index 7905cba6e..56bfb9b9c 100644 --- a/README.rst +++ b/README.rst @@ -118,7 +118,7 @@ If you use Oríon for published work, please cite our work using the following b .. code-block:: bibtex - @software{xavier_bouthillier_2022_0_2_5, + @software{xavier_bouthillier_2022_0_2_6, author = {Xavier Bouthillier and Christos Tsirigotis and François Corneau-Tremblay and @@ -146,7 +146,7 @@ If you use Oríon for published work, please cite our work using the following b month = august, year = 2022, publisher = {Zenodo}, - version = {v0.2.5}, + version = {v0.2.6, doi = {10.5281/zenodo.3478592}, url = {https://doi.org/10.5281/zenodo.3478592} } diff --git a/ROADMAP.md b/ROADMAP.md index 1d1261c8c..81a591e06 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -1,10 +1,14 @@ # Roadmap -Last update August 2nd, 2022 +Last update August 18th, 2022 ## Next releases - Short-Term +### v0.2.7 + ### v0.2.6 +- Fix; make sure stdout from user script is printed back and not captured + #### Simple dashboard specific to monitoring and benchmarking of Black-Box optimization - Specific to hyper parameter optimizations - Provide status of experiments @@ -13,6 +17,8 @@ Last update August 2nd, 2022 Leveraging the knowledge base contained in the EVC of previous trials to optimize and drive new trials. +## Next releases - Mid-Term + ## Next releases - Long-Term #### Conditional Space diff --git a/examples/plotting/plot_4_partial_dependencies.py b/examples/plotting/plot_4_partial_dependencies.py index afed96440..eae85ff5d 100644 --- a/examples/plotting/plot_4_partial_dependencies.py +++ b/examples/plotting/plot_4_partial_dependencies.py @@ -63,7 +63,7 @@ # parameters # : # -# Even for a simple 2-d search space, the partial dependency is very useful. We see very cleary in this example +# Even for a simple 2-d search space, the partial dependency is very useful. We see very clearly in this example # the optimal regions for both hyperparameters and we can see as well that the optimal region for # learning rates is larger when the dropout is low, and narrower when dropout approaches 0.5. # diff --git a/src/orion/core/evc/conflicts.py b/src/orion/core/evc/conflicts.py index 92128254d..33ff34ef7 100644 --- a/src/orion/core/evc/conflicts.py +++ b/src/orion/core/evc/conflicts.py @@ -267,7 +267,7 @@ def try_resolve(self, conflict, *args, **kwargs): Parameters ---------- - conflict: `orion.ore.evc.conflicts.Conflict` + conflict: `orion.core.evc.conflicts.Conflict` Conflict object to call `try_resolve`. silence_errors: bool If True, errors raised on execution of conflict.try_resolve will be caught and diff --git a/src/orion/core/io/experiment_branch_builder.py b/src/orion/core/io/experiment_branch_builder.py index ae509d782..26c10a649 100644 --- a/src/orion/core/io/experiment_branch_builder.py +++ b/src/orion/core/io/experiment_branch_builder.py @@ -33,7 +33,7 @@ class ExperimentBranchBuilder: Parameters ---------- conflicts: Conflicts - Object reprenting a group of conflicts + Object representing a group of conflicts manual_resolution: bool, optional Starts the prompt to resolve manually the conflicts. Use system's default if not provided. branch_from: str, optional diff --git a/src/orion/storage/track.py b/src/orion/storage/track.py index 0dc84b3ca..e3a698829 100644 --- a/src/orion/storage/track.py +++ b/src/orion/storage/track.py @@ -150,11 +150,12 @@ def experiment(self): return self.storage.group_id @property - def hearbeat(self): - """See `~orion.core.worker.trial.Trial`""" - return datetime.datetime.utcfromtimestamp( - self.storage.metadata.get("heartbeat", 0) - ) + def heartbeat(self): + """Trial Heartbeat""" + heartbeat = self.storage.metadata.get("heartbeat") + if heartbeat: + return datetime.datetime.utcfromtimestamp(heartbeat) + return None @property def id(self): @@ -311,14 +312,6 @@ def end_time(self, value): """See `~orion.core.worker.trial.Trial`""" self.storage.metadata["end_time"] = value - @property - def heartbeat(self): - """Trial Heartbeat""" - heartbeat = self.storage.metadata.get("heartbeat") - if heartbeat: - return datetime.datetime.utcfromtimestamp(heartbeat) - return None - @property def parents(self): """See `~orion.core.worker.trial.Trial`""" diff --git a/tests/functional/backward_compatibility/versions.txt b/tests/functional/backward_compatibility/versions.txt index 484484568..4d512bf29 100644 --- a/tests/functional/backward_compatibility/versions.txt +++ b/tests/functional/backward_compatibility/versions.txt @@ -13,3 +13,5 @@ 0.2.1 0.2.3 0.2.4.post1 +0.2.5 +0.2.6 diff --git a/tests/unittests/core/io/test_resolve_config.py b/tests/unittests/core/io/test_resolve_config.py index 9501902c7..f3f23bca7 100644 --- a/tests/unittests/core/io/test_resolve_config.py +++ b/tests/unittests/core/io/test_resolve_config.py @@ -71,7 +71,7 @@ def test_fetch_metadata_python_users_script(script_path): def test_fetch_metadata_not_existed_path(): - """Verfiy the raise of error when user_script path does not exist""" + """Verify the raise of error when user_script path does not exist""" path = "dummy/path" with pytest.raises(OSError) as exc_info: resolve_config.fetch_metadata(user_args=[path]) From 7f698e7e869d02bb43e287679f12f40a59388657 Mon Sep 17 00:00:00 2001 From: Setepenre Date: Thu, 18 Aug 2022 15:57:11 -0400 Subject: [PATCH 3/4] Update versions.txt --- tests/functional/backward_compatibility/versions.txt | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/functional/backward_compatibility/versions.txt b/tests/functional/backward_compatibility/versions.txt index 4d512bf29..5435b590c 100644 --- a/tests/functional/backward_compatibility/versions.txt +++ b/tests/functional/backward_compatibility/versions.txt @@ -14,4 +14,3 @@ 0.2.3 0.2.4.post1 0.2.5 -0.2.6 From a6010e917ef83662128d84ae1758da7a3c51c8a9 Mon Sep 17 00:00:00 2001 From: Setepenre Date: Fri, 19 Aug 2022 11:45:38 -0400 Subject: [PATCH 4/4] Update versions.txt --- tests/functional/backward_compatibility/versions.txt | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/functional/backward_compatibility/versions.txt b/tests/functional/backward_compatibility/versions.txt index 5435b590c..484484568 100644 --- a/tests/functional/backward_compatibility/versions.txt +++ b/tests/functional/backward_compatibility/versions.txt @@ -13,4 +13,3 @@ 0.2.1 0.2.3 0.2.4.post1 -0.2.5