From 03b0f34940409bb29907410f0ebf208737266d4f Mon Sep 17 00:00:00 2001 From: Kai Fricke Date: Wed, 26 Apr 2023 15:57:17 +0100 Subject: [PATCH 1/8] Unit test Signed-off-by: Kai Fricke --- python/ray/tune/experimental/output.py | 125 ++++++++++++++------ python/ray/tune/tests/output/test_output.py | 57 +++++++++ 2 files changed, 147 insertions(+), 35 deletions(-) diff --git a/python/ray/tune/experimental/output.py b/python/ray/tune/experimental/output.py index 37d2ba5418c8..0c0e808cc148 100644 --- a/python/ray/tune/experimental/output.py +++ b/python/ray/tune/experimental/output.py @@ -1,4 +1,4 @@ -from typing import List, Dict, Optional, Tuple, Any, TYPE_CHECKING +from typing import List, Dict, Optional, Tuple, Any, TYPE_CHECKING, Collection import contextlib import collections @@ -26,7 +26,6 @@ from ray._private.dict import unflattened_lookup from ray.air._internal.checkpoint_manager import _TrackedCheckpoint from ray.tune.callback import Callback -from ray.tune.logger import pretty_print from ray.tune.result import ( AUTO_RESULT_KEYS, EPISODE_REWARD_MEAN, @@ -356,6 +355,62 @@ def _best_trial_str( ) +def _render_table_item(key: str, item: Any, prefix: str = ""): + key = prefix + key + if isinstance(item, float): + # tabulate does not work well with mixed-type columns, so we format + # numbers ourselves. + yield key, f"{item:.5f}".rstrip("0") + elif isinstance(item, list): + yield key, str(item) + elif isinstance(item, Dict): + yield key, None + for sk, sv in item.items(): + yield from _render_table_item(sk, sv, prefix=prefix + "/") + else: + yield key, item + + +def _get_dict_as_table_data( + data: Dict, + exclude: Optional[Collection] = None, + upper_keys: Optional[Collection] = None, +): + exclude = exclude or set() + upper_keys = upper_keys or set() + + upper = [] + lower = [] + + for key, value in sorted(data.items()): + if key in exclude: + continue + + for k, v in _render_table_item(key, value): + if key in upper_keys: + upper.append([k, v]) + else: + lower.append([k, v]) + + if not upper: + return lower + + else: + return upper + [[None, None]] + lower + + +def _print_dict_as_table( + data: Dict, + exclude: Optional[Collection] = None, + division: Optional[Collection] = None, +): + table_data = _get_dict_as_table_data( + data=data, exclude=exclude, upper_keys=division + ) + + print(tabulate(table_data, colalign=("left", "right"), tablefmt="simple")) + + class ProgressReporter: """Periodically prints out status update.""" @@ -594,6 +649,7 @@ def _print_heartbeat(self, trials, *args): # These keys are blacklisted for printing out training/tuning intermediate/final result! BLACKLISTED_KEYS = { + "config", "date", "done", "hostname", @@ -650,12 +706,23 @@ class AirResultProgressCallback(Callback): def __init__(self, verbosity): self._verbosity = verbosity self._start_time = time.time() + self._trial_last_printed_results = {} + + def _print_result(self, trial, result: Optional[Dict] = None, force: bool = False): + """Only print result if a different result has been reported, or force=True""" + result = result or trial.last_result + + last_result_iter = self._trial_last_printed_results.get(trial.trial_id, -1) + this_iter = result.get(TRAINING_ITERATION, 0) - def _print_result(self, trial, result=None): - print(pretty_print(result or trial.last_result, BLACKLISTED_KEYS)) + if this_iter != last_result_iter or force: + _print_dict_as_table( + result, exclude=BLACKLISTED_KEYS, division=AUTO_RESULT_KEYS + ) + self._trial_last_printed_results[trial.trial_id] = this_iter def _print_config(self, trial): - print(pretty_print(trial.config)) + _print_dict_as_table(trial.config) def on_trial_result( self, @@ -669,13 +736,9 @@ def on_trial_result( return curr_time, running_time = _get_time_str(self._start_time, time.time()) print( - " ".join( - [ - self._addressing_tmpl.format(trial), - f"finished iter {result[TRAINING_ITERATION]} " - f"at {curr_time} (running for {running_time})", - ] - ) + f"{self._addressing_tmpl.format(trial)} " + f"finished iteration {result[TRAINING_ITERATION]} " + f"at {curr_time} (running for {running_time})." ) self._print_result(trial, result) @@ -689,13 +752,9 @@ def on_trial_complete( if trial.last_result and TRAINING_ITERATION in trial.last_result: finished_iter = trial.last_result[TRAINING_ITERATION] print( - " ".join( - [ - self._addressing_tmpl.format(trial), - f"({finished_iter} iters) " - f"finished at {curr_time} (running for {running_time})", - ] - ) + f"{self._addressing_tmpl.format(trial)} " + f"completed training after {finished_iter} iterations " + f"at {curr_time} (running for {running_time})." ) self._print_result(trial) @@ -714,30 +773,26 @@ def on_checkpoint( if trial.last_result and TRAINING_ITERATION in trial.last_result: saved_iter = trial.last_result[TRAINING_ITERATION] print( - " ".join( - [ - self._addressing_tmpl.format(trial), - f"saved checkpoint for iter {saved_iter}" - f" at {checkpoint.dir_or_data}", - ] - ) + f"{self._addressing_tmpl.format(trial)} " + f"saved a checkpoint for iteration {saved_iter} " + f"at: {checkpoint.dir_or_data}" ) - print() def on_trial_start(self, iteration: int, trials: List[Trial], trial: Trial, **info): if self._verbosity < self._start_end_verbosity: return has_config = bool(trial.config) - print( - " ".join( - [ - self._addressing_tmpl.format(trial), - "started with configuration:" if has_config else "started.", - ] - ) - ) + if has_config: + print( + f"{self._addressing_tmpl.format(trial)} " f"started with configuration:" + ) self._print_config(trial) + else: + print( + f"{self._addressing_tmpl.format(trial)} " + f"started without custom configuration." + ) class TuneResultProgressCallback(AirResultProgressCallback): diff --git a/python/ray/tune/tests/output/test_output.py b/python/ray/tune/tests/output/test_output.py index c13248b79068..95a285fea2a5 100644 --- a/python/ray/tune/tests/output/test_output.py +++ b/python/ray/tune/tests/output/test_output.py @@ -13,6 +13,7 @@ _current_best_trial, _best_trial_str, _get_trial_table_data, + _get_dict_as_table_data, ) from ray.tune.experiment.trial import Trial @@ -164,5 +165,61 @@ def test_get_trial_table_data_more_than_20(): assert table_data[2].more_info == "... and 5 more PENDING ..." +def test_result_table_no_divison(): + data = _get_dict_as_table_data( + { + "b": 6, + "a": 8, + "x": 19.123123123, + "c": 5, + "ignore": 9, + "y": 20, + "z": {"m": 4, "n": {"o": "p"}}, + }, + exclude={"ignore"}, + ) + + assert data == [ + ["a", 8], + ["b", 6], + ["c", 5], + ["x", "19.12312"], + ["y", 20], + ["z", None], + ["/m", 4], + ["/n", None], + ["//o", "p"], + ] + + +def test_result_table_divison(): + data = _get_dict_as_table_data( + { + "b": 6, + "a": 8, + "x": 19.123123123, + "c": 5, + "ignore": 9, + "y": 20, + "z": {"m": 4, "n": {"o": "p"}}, + }, + exclude={"ignore"}, + upper_keys={"x", "y", "z"}, + ) + + assert data == [ + ["x", "19.12312"], + ["y", 20], + ["z", None], + ["/m", 4], + ["/n", None], + ["//o", "p"], + [None, None], + ["a", 8], + ["b", 6], + ["c", 5], + ] + + if __name__ == "__main__": sys.exit(pytest.main(["-v", __file__])) From bd6879ee5991d287fa405588f2927ed6ad6ecdba Mon Sep 17 00:00:00 2001 From: Kai Fricke Date: Thu, 27 Apr 2023 10:00:28 +0100 Subject: [PATCH 2/8] Tabulate style Signed-off-by: Kai Fricke --- python/ray/tune/experimental/output.py | 44 +++++++++++++++++++++++--- 1 file changed, 40 insertions(+), 4 deletions(-) diff --git a/python/ray/tune/experimental/output.py b/python/ray/tune/experimental/output.py index 7ccbf9107ca1..a41c049acb21 100644 --- a/python/ray/tune/experimental/output.py +++ b/python/ray/tune/experimental/output.py @@ -11,7 +11,6 @@ import numpy as np import os import pandas as pd -from ray._private.thirdparty.tabulate.tabulate import tabulate import textwrap import time @@ -24,6 +23,12 @@ import ray from ray._private.dict import unflattened_lookup +from ray._private.thirdparty.tabulate.tabulate import ( + tabulate, + TableFormat, + Line, + DataRow, +) from ray.air._internal.checkpoint_manager import _TrackedCheckpoint from ray.tune.callback import Callback from ray.tune.result import ( @@ -394,13 +399,28 @@ def _get_dict_as_table_data( if not upper: return lower + elif not lower: + return upper else: return upper + [[None, None]] + lower +AIR_TABULATE_TABLEFMT = TableFormat( + lineabove=Line("╭", "─", "─", "╮"), + linebelowheader=Line("├", "─", "─", "┤"), + linebetweenrows=None, + linebelow=Line("╰", "─", "─", "╯"), + headerrow=DataRow("│", " ", "│"), + datarow=DataRow("│", " ", "│"), + padding=1, + with_header_hide=None, +) + + def _print_dict_as_table( data: Dict, + header: Optional[str] = None, exclude: Optional[Collection] = None, division: Optional[Collection] = None, ): @@ -408,7 +428,18 @@ def _print_dict_as_table( data=data, exclude=exclude, upper_keys=division ) - print(tabulate(table_data, colalign=("left", "right"), tablefmt="simple")) + headers = () + if header: + headers = [header, ""] + + print( + tabulate( + table_data, + headers=headers, + colalign=("left", "right"), + tablefmt=AIR_TABULATE_TABLEFMT, + ) + ) class ProgressReporter: @@ -717,12 +748,17 @@ def _print_result(self, trial, result: Optional[Dict] = None, force: bool = Fals if this_iter != last_result_iter or force: _print_dict_as_table( - result, exclude=BLACKLISTED_KEYS, division=AUTO_RESULT_KEYS + result, + header=f"{self._addressing_tmpl.format(trial)} result", + exclude=BLACKLISTED_KEYS, + division=AUTO_RESULT_KEYS, ) self._trial_last_printed_results[trial.trial_id] = this_iter def _print_config(self, trial): - _print_dict_as_table(trial.config) + _print_dict_as_table( + trial.config, header=f"{self._addressing_tmpl.format(trial)} config" + ) def on_trial_result( self, From 2adb21c8733a97a6c981c1a36f84bdddfd15554a Mon Sep 17 00:00:00 2001 From: Kai Fricke Date: Thu, 27 Apr 2023 11:59:44 +0100 Subject: [PATCH 3/8] Fix empty table data case Signed-off-by: Kai Fricke --- python/ray/tune/experimental/output.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/python/ray/tune/experimental/output.py b/python/ray/tune/experimental/output.py index a41c049acb21..b9085a26bcdd 100644 --- a/python/ray/tune/experimental/output.py +++ b/python/ray/tune/experimental/output.py @@ -432,6 +432,9 @@ def _print_dict_as_table( if header: headers = [header, ""] + if not table_data: + return + print( tabulate( table_data, From bfd83da05c69fa157f0c2f81af5531155fc3d29e Mon Sep 17 00:00:00 2001 From: Kai Fricke Date: Thu, 27 Apr 2023 14:20:50 +0100 Subject: [PATCH 4/8] Fix resource request comparison Signed-off-by: Kai Fricke --- python/ray/air/execution/resources/request.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/python/ray/air/execution/resources/request.py b/python/ray/air/execution/resources/request.py index 87455a6b3ad8..40dbd3858d85 100644 --- a/python/ray/air/execution/resources/request.py +++ b/python/ray/air/execution/resources/request.py @@ -154,7 +154,8 @@ def to_placement_group(self): def __eq__(self, other: "ResourceRequest"): return ( - self._bound == other._bound + isinstance(other, ResourceRequest) + and self._bound == other._bound and self.head_bundle_is_empty == other.head_bundle_is_empty ) From dd932e78b1341803020de7e002e2b86a5fdc936a Mon Sep 17 00:00:00 2001 From: Kai Fricke Date: Thu, 27 Apr 2023 15:36:39 +0100 Subject: [PATCH 5/8] key to str Signed-off-by: Kai Fricke --- python/ray/tune/experimental/output.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/python/ray/tune/experimental/output.py b/python/ray/tune/experimental/output.py index b9085a26bcdd..c41aff4c0680 100644 --- a/python/ray/tune/experimental/output.py +++ b/python/ray/tune/experimental/output.py @@ -371,7 +371,7 @@ def _render_table_item(key: str, item: Any, prefix: str = ""): elif isinstance(item, Dict): yield key, None for sk, sv in item.items(): - yield from _render_table_item(sk, sv, prefix=prefix + "/") + yield from _render_table_item(str(sk), sv, prefix=prefix + "/") else: yield key, item @@ -391,7 +391,7 @@ def _get_dict_as_table_data( if key in exclude: continue - for k, v in _render_table_item(key, value): + for k, v in _render_table_item(str(key), value): if key in upper_keys: upper.append([k, v]) else: From bdc9f3d3d620ae79e50f1db5ce3b4462017295fc Mon Sep 17 00:00:00 2001 From: Kai Fricke Date: Tue, 2 May 2023 09:28:17 +0100 Subject: [PATCH 6/8] Code review Signed-off-by: Kai Fricke --- python/ray/tune/experimental/output.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/python/ray/tune/experimental/output.py b/python/ray/tune/experimental/output.py index c41aff4c0680..dcfc3c41647e 100644 --- a/python/ray/tune/experimental/output.py +++ b/python/ray/tune/experimental/output.py @@ -401,11 +401,11 @@ def _get_dict_as_table_data( return lower elif not lower: return upper - else: - return upper + [[None, None]] + lower + return upper + lower +# Copied/adjusted from tabulate AIR_TABULATE_TABLEFMT = TableFormat( lineabove=Line("╭", "─", "─", "╮"), linebelowheader=Line("├", "─", "─", "┤"), @@ -428,9 +428,7 @@ def _print_dict_as_table( data=data, exclude=exclude, upper_keys=division ) - headers = () - if header: - headers = [header, ""] + headers = [header, ""] if header else [] if not table_data: return From 419e40768e07ef3d83fe596fae5072f127c3b496 Mon Sep 17 00:00:00 2001 From: Kai Fricke Date: Tue, 2 May 2023 10:09:36 +0100 Subject: [PATCH 7/8] Recursive list Signed-off-by: Kai Fricke --- python/ray/tune/experimental/output.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/python/ray/tune/experimental/output.py b/python/ray/tune/experimental/output.py index dcfc3c41647e..fe649658269d 100644 --- a/python/ray/tune/experimental/output.py +++ b/python/ray/tune/experimental/output.py @@ -367,7 +367,9 @@ def _render_table_item(key: str, item: Any, prefix: str = ""): # numbers ourselves. yield key, f"{item:.5f}".rstrip("0") elif isinstance(item, list): - yield key, str(item) + yield key, None + for sv in item: + yield from _render_table_item("", sv, prefix=prefix + "-") elif isinstance(item, Dict): yield key, None for sk, sv in item.items(): From 27d1af14098399f921f25a109ae69552ba7e1ae9 Mon Sep 17 00:00:00 2001 From: Kai Fricke Date: Tue, 2 May 2023 14:36:30 +0100 Subject: [PATCH 8/8] Fix test_output Signed-off-by: Kai Fricke --- python/ray/tune/tests/output/test_output.py | 1 - 1 file changed, 1 deletion(-) diff --git a/python/ray/tune/tests/output/test_output.py b/python/ray/tune/tests/output/test_output.py index 95a285fea2a5..7b45a8078446 100644 --- a/python/ray/tune/tests/output/test_output.py +++ b/python/ray/tune/tests/output/test_output.py @@ -214,7 +214,6 @@ def test_result_table_divison(): ["/m", 4], ["/n", None], ["//o", "p"], - [None, None], ["a", 8], ["b", 6], ["c", 5],