Skip to content

Commit 5c25751

Browse files
committed
Skip experiments with no metric when picking the best autotuning record
get_best_space_record compared metric_val > best_space_record[1] unconditionally. When an experiment produces no metric (for example an OOM during autotuning), metric_val is None, and once the first record in a space is None the very next comparison raises TypeError: '>' not supported between instances of 'NoneType' and 'NoneType'. This crashes the whole tuning run and throws away every valid result gathered so far, in both the fast-mode and full-tuning call sites in tune_space, plus get_best_space_records. Records with a None metric are no longer eligible to become the best record. They still count toward the space's total experiment count, they just cannot win the comparison, which matches how a failed run should be treated everywhere the function is called from. Fixes #3475 Signed-off-by: rakshit-gen <sisodiarakshit456@gmail.com>
1 parent 05daf05 commit 5c25751

2 files changed

Lines changed: 24 additions & 0 deletions

File tree

deepspeed/autotuning/autotuner.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -719,6 +719,9 @@ def get_best_space_record(self, space_name):
719719
space_num_exps = 0
720720
for (exp, metric_val, num_exps) in space_records:
721721
space_num_exps += num_exps
722+
if metric_val is None:
723+
# a run that did not produce a metric (e.g. OOM) is not a valid candidate
724+
continue
722725
if best_space_record is None or metric_val > best_space_record[1]:
723726
best_space_record = (exp, metric_val)
724727
if best_space_record:

tests/unit/autotuning/test_autotuning.py

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,27 @@ def test_autotuner_resources(tmpdir, active_resources):
7979
assert expected_num_gpus == tuner.exp_num_gpus
8080

8181

82+
def test_get_best_space_record_ignores_runs_without_a_metric(tmpdir):
83+
# A run that fails to produce a metric (e.g. OOM) is recorded with metric_val=None.
84+
# get_best_space_record used to crash with "TypeError: '>' not supported between
85+
# instances of 'NoneType' and 'NoneType'" whenever every run in a space had no
86+
# metric, discarding all previously gathered results for the whole tuning run.
87+
config_dict = {"autotuning": {"enabled": True, "exps_dir": os.path.join(tmpdir, 'exps_dir'), "arg_mappings": {}}}
88+
config_path = create_config_from_dict(tmpdir, config_dict)
89+
args = dsrun.parse_args(args=f'--autotuning {TUNE_OPTION} foo.py --deepspeed_config {config_path}'.split())
90+
tuner = Autotuner(args=args, active_resources={"worker-0": [0, 1]})
91+
92+
tuner.update_records("z0_space", {"name": "exp1"}, None, 1)
93+
tuner.update_records("z0_space", {"name": "exp2"}, None, 1)
94+
assert tuner.get_best_space_record("z0_space") is None
95+
96+
tuner.update_records("z0_space", {"name": "exp3"}, 3.5, 1)
97+
best = tuner.get_best_space_record("z0_space")
98+
assert best[0]["name"] == "exp3"
99+
assert best[1] == 3.5
100+
assert best[2] == 3
101+
102+
82103
def test_get_val_by_key_searches_all_nested_subdicts():
83104
# get_val_by_key must mirror its sibling set_val_by_key: both walk every
84105
# nested subdict, not just the first one. Here 'device' lives in the SECOND

0 commit comments

Comments
 (0)