Filter --include against the real slots, not against itself (#8239)
## Problem
`parse_inclusion_exclusion()` builds the `host_info` dict that
`parse_resource_filter()` validates against, and it seeded that dict
from the inclusion string:
```python
node_configs = parse_node_config_list(inclusion)
for hostname, slots in resource_pool.items():
active_resources[hostname] = node_configs[hostname] if hostname in node_configs else list(range(slots))
```
So for any host named in `--include`, the filter checked the request
against a copy of itself. Two things follow.
**A bare hostname resolves to no slots at all.**
`parse_node_config_list("worker-0")` gives `{"worker-0": []}`, so
`active_resources["worker-0"]` became `[]`, `parse_resource_filter`'s
whole-node branch copied that empty list back, and the post-processing
dropped the host for being empty. `parse_resource_filter`'s own
docstring uses `worker-0@worker-1:0,2` as its example of "use all slots
on worker-0 and slots [0, 2] on worker-1":
```
pool = {"worker-0": 4, "worker-1": 4}
--include worker-0 -> {} (expected all 4 slots)
--include worker-0@worker-1:0,2 -> {'worker-1': [0, 2]} (expected worker-0 too)
```
The job then launches on 2 GPUs instead of 6, with no error and no
warning, and the node the user asked for first is the one that
disappears.
**The slot check could never fire for `--include`.** The same input is
rejected through `--exclude` and accepted through `--include`:
```
--include worker-0:99 -> ACCEPTED, {'worker-0': [99]}
--exclude worker-0:99 -> ValueError: No slot '99' specified on host 'worker-0'
--include worker-0:0,4 -> ACCEPTED, {'worker-0': [0, 4]} (off-by-one on a 4 GPU host)
--exclude worker-0:0,4 -> ValueError: No slot '4' specified on host 'worker-0'
```
`{'worker-0': [99]}` goes on to `CUDA_VISIBLE_DEVICES`, so a typo
surfaces as a CUDA error from inside torch rather than as the launcher
error that already exists for it. Hostname validation is symmetric and
works; only the slot check is affected.
## Fix
Hand `parse_resource_filter()` the slots the machines actually have and
let it do the filtering it was written to do. `parse_resource_filter` is
unchanged: it already sets `filtered_hosts[hostname] = slots` for an
explicit slot list and `host_info[hostname]` for a bare hostname, both
of which are now correct.
```
--include worker-0 -> {'worker-0': [0, 1, 2, 3]}
--include worker-0@worker-1:0,2 -> {'worker-0': [0, 1, 2, 3], 'worker-1': [0, 2]}
--include worker-0:99 -> ValueError: No slot '99' specified on host 'worker-0'
```
## Test
`tests/unit/launcher/test_run.py` has good coverage of
`parse_resource_filter`, but every one of those tests hands it a correct
`host_info` dict directly, so nothing exercised the wrapper that builds
it. That is why this was invisible: the function under test was fine,
and the caller was not.
Two tests added, `test_parse_inclusion_exclusion` and
`test_parse_inclusion_exclusion_errors`, covering the bare hostname, the
docstring's own example, the mixed form, exclusion, and out-of-range
slots through both `--include` and `--exclude`.
```
before after
existing 7 tests pass pass
test_parse_inclusion_exclusion FAIL pass (OrderedDict() != {'worker-0': [0, 1, 2, 3]})
test_parse_inclusion_exclusion_errors FAIL pass (DID NOT RAISE ValueError)
```
`python -m pytest tests/unit/launcher/test_run.py` gives 2 failed / 7
passed with the source change reverted and 9 passed with it, on CPU.
`yapf --style .style.yapf` and `flake8 --config .flake8` are clean on
both changed files, and clean on the unmodified tree as a control.
---------
Signed-off-by: Vineeth Sai <vineethsai4444@gmail.com>
Co-authored-by: Masahiro Tanaka <81312776+tohtana@users.noreply.github.com>
DeepSpeed
3b1c14ac - Filter --include against the real slots, not against itself (#8239)
