-
Notifications
You must be signed in to change notification settings - Fork 5.8k
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
[Tune] Add repr for ResultGrid class #31941
Changes from 1 commit
fac8f78
6f3257b
136b624
47800e2
ec61001
3bc5f8c
e417093
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -240,3 +240,10 @@ def _trial_to_result(self, trial: Trial) -> Result: | |
best_checkpoints=best_checkpoints, | ||
) | ||
return result | ||
|
||
def __repr__(self) -> str: | ||
all_results_repr = "" | ||
for result in self: | ||
result_repr = " " + result.__repr__().replace("\n", "\n ") | ||
all_results_repr += f"{result_repr},\n" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
return f"[\n{all_results_repr}]" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we still have |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -224,6 +224,39 @@ def f(config): | |
assert not any(key in representation for key in AUTO_RESULT_KEYS) | ||
|
||
|
||
def test_result_grid_repr(ray_start_2_cpus, tmpdir): | ||
woshiyyya marked this conversation as resolved.
Show resolved
Hide resolved
|
||
from ray.air import session | ||
|
||
def f(config): | ||
if config["x"] == 1: | ||
raise RuntimeError | ||
|
||
metrics = {"loss": 1} | ||
session.report(metrics, checkpoint=Checkpoint.from_dict(metrics)) | ||
|
||
tuner = tune.Tuner( | ||
f, | ||
run_config=air.RunConfig( | ||
name="test_result_grid_repr", | ||
local_dir=str(tmpdir / "test_result_grid_repr_results"), | ||
), | ||
param_space={"x": tune.grid_search([1, 2])}, | ||
) | ||
result_grid = tuner.fit() | ||
|
||
representation = result_grid.__repr__() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we instead construct the result grid manually and assert the layout? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree. Manually creating a result grid will be way faster than running a full tuning experiment. |
||
|
||
from ray.tune.result import AUTO_RESULT_KEYS | ||
|
||
assert not any(key in representation for key in AUTO_RESULT_KEYS) | ||
woshiyyya marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
assert len(result_grid) == 2 | ||
assert representation.count("metrics=") == 2 | ||
assert representation.count("log_dir=") == 2 | ||
assert representation.count("checkpoint=") == 2 | ||
assert representation.count("error=") == 1 and "RuntimeError" in representation | ||
woshiyyya marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
|
||
def test_no_metric_mode(ray_start_2_cpus): | ||
def f(config): | ||
tune.report(x=1) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's very hard to imagine how this looks like, can we add a comment what we expect here?