Fix autotuner crash when an experiment produces no metric (#8421)
Fixes #3475
## The bug
`get_best_space_record` picks the best record in a tuning space with:
```python
if best_space_record is None or metric_val > best_space_record[1]:
```
An experiment that produces no metric (an OOM during autotuning is the
common case) is still recorded, with `metric_val` set to `None`. Once a
`None`-valued record becomes `best_space_record`, the next comparison
does `metric_val > best_space_record[1]`, and if both sides happen to
be `None` (or the incoming one is a real number compared against a
`None` best) this raises:
```
TypeError: '>' not supported between instances of 'NoneType' and 'NoneType'
```
This isn't limited to `get_best_space_record` itself: `tune_space`
calls it both in fast-tuning mode and after full tuning to pick the
overall best micro batch size, and `get_best_space_records` calls it
once per tuning space to build the final results table. A single space
where every experiment OOM'd is enough to crash the whole autotuning
run and lose every valid result gathered from the spaces tuned before
it.
## The fix
A record with a `None` metric is no longer eligible to become the
best record. It's still counted toward the space's total number of
experiments (`space_num_exps`), it just can't win the comparison. This
means `get_best_space_record` now returns `None` only when a space has
no experiments with a usable metric at all, which the existing callers
already handle correctly (they fall back to `0` or `-1` in that case),
so no other call site needed to change.
## Testing
Added `test_get_best_space_record_ignores_runs_without_a_metric` to
`tests/unit/autotuning/test_autotuning.py`, covering:
- every recorded experiment in a space has `metric_val=None` -> no
crash, returns `None`
- a mix of `None` and real metrics -> the real one wins regardless of
position, and the experiment count still includes the failed runs
Confirmed this test fails with the exact `TypeError` above on current
master and passes with the fix. Also ran the full existing
`tests/unit/autotuning/test_autotuning.py` suite (12 tests) and it
passes. Ran `pre-commit` on both changed files (yapf, flake8,
check-torchdist, check-license, codespell) with no issues. This is a
pure CPU, no-GPU-needed unit test path, no integration/training-loop
behavior is touched.
cc @loadams (CODEOWNERS for `deepspeed/autotuning/`)
---------
Signed-off-by: rakshit-gen <sisodiarakshit456@gmail.com>
Co-authored-by: Olatunji Ruwase <tunji.ruwase@snowflake.com>