b8b4480965
## The bug `CommsLogger.comms_dict` stores parallel lists per message size, `[count, latencies, algbws, busbws]`, where index `i` is the i-th recorded op. `get_operation_summary()` and `log_all()` hand each of those lists to `trim_mean`, which sorted **in place**: ```python data.sort() k = int(round(n * (trim_percent))) return mean(data[k:n - k]) ``` So summarising sorts the three lists independently and destroys the correspondence between them. Driving the real `CommsLogger` with latencies `[3.0, 1.0, 2.0]`: ``` BEFORE latency: [3.0, 1.0, 2.0] algbw: [0.0055, 0.0164, 0.0082] AFTER latency: [1.0, 2.0, 3.0] algbw: [0.0055, 0.0082, 0.0164] stored data mutated by a read-only summary call: True latency[i] * algbw[i], constant by construction: [0.0055, 0.0164, 0.0492] ``` `algbw` is computed from `latency` in `calc_bw_log`, so `latency[i] * algbw[i]` is constant for a fixed message size. After summarising it is not, because row 0 now pairs the fastest op (1.0 ms) with the lowest bandwidth. `get_raw_data()` hands that out to anyone consuming the log. Two things make this look unintended rather than a quirk: - `get_operation_summary()` already does `op_data = self.comms_dict[operation_name].copy()` with the comment "Create a snapshot to avoid concurrent modification issues". The intent not to disturb the stored records is explicit; the copy is just shallow, so the inner lists are shared and the sort reaches them anyway. - A summary is a read. Calling it should not rewrite what was recorded, and nothing documents that it does. ## It also breaks the straggler breakdown, which is the bigger effect Raised in review and worth stating here, because it is worse than the reordering above. `log_all()` reads `vals[1]` twice. The summary loop calls `trim_mean(vals[1], 0.1)`, which sorts it, and the `show_straggler` block afterwards builds both `lats` and `min_lats` from that same, now-sorted list: ```python lats = torch.tensor(vals[1], device=device) min_lats = torch.tensor(vals[1], device=device) dist.all_reduce(min_lats, op=ReduceOp.MIN) total_straggler = (lats - min_lats).sum().item() ``` `all_reduce(..., MIN)` is elementwise, so it assumes index `i` is the same collective on every rank. Each rank has sorted its own list independently by then, so index `i` is "the i-th fastest op on *that* rank" and the reduction takes the minimum across unrelated operations. `comms_dict_snapshot = self.comms_dict.copy()` at the top of `log_all()` does not help, for the same reason the copy in `get_operation_summary()` does not: it is shallow. That is not a cosmetic reordering, it changes the number. Two ranks, four index-aligned ops: ``` rank0 = [2.30, 1.60, 3.60, 1.29] rank1 = [3.14, 2.46, 1.23, 3.03] true total_straggler per rank [2.37, 3.44] with the in-place sort [0.52, 1.59] <- what gets reported ``` Over 20000 random four-op cases, 16246 report a different total, and the error has a direction: sorting both ranks aligns them as closely as their values allow, so the differences shrink and the straggler effect is systematically **under**-reported. Small cases can coincide (a three-op example I tried happened to agree), which is part of why this is easy to miss. ## Fix `data = sorted(data)`. That fixes every caller at the shared function rather than patching `log_all` and `get_operation_summary` separately, and the trimmed mean it returns is unchanged. ## Tests Added to `tests/unit/comm/test_comms_logger.py`: - `test_get_operation_summary_does_not_reorder_the_stored_records` populates `comms_dict` directly and asserts the three stored lists survive a summary call unchanged, that `latency[i] * algbw[i]` is still constant, and that `avg_latency_ms` is still the correct trimmed mean. - `test_trim_mean_does_not_mutate_its_argument` pins the contract at the function itself. Both are dist-free, like the existing test in that file. `comms_dict` is populated directly rather than through `append()` because `append()` calls `calc_bw_log`, which needs a live process group. Fail-before / pass-after against the unmodified `timer.py`: ``` upstream timer.py, new tests present PASS test_stop_profiling_comms_disables_prof_all FAIL test_get_operation_summary_does_not_reorder_the_stored_records FAIL test_trim_mean_does_not_mutate_its_argument with this fix PASS all three ``` The pre-existing test passing either way is deliberate: this is a distinct failure from the one it covers. How these were run, since I would rather say than imply a normal pytest run: I do not have a GPU or a built DeepSpeed here, so I executed `deepspeed/utils/timer.py` and `deepspeed/utils/comms_logging.py` from source against stub `deepspeed.comm` / `deepspeed.accelerator` modules and ast-extracted the tests. That exercises the real `CommsLogger` and the real `trim_mean`; CI is the runner for the suite proper. No existing issue or PR covers this; searching `trim_mean` across open and closed returns only the merged PR that introduced the current form. --------- Signed-off-by: Vineeth Sai <vineethsai4444@gmail.com> Co-authored-by: Olatunji Ruwase <tunji.ruwase@snowflake.com>
317 lines
11 KiB
Python
317 lines
11 KiB
Python
# Copyright (c) Microsoft Corporation.
|
|
# SPDX-License-Identifier: Apache-2.0
|
|
|
|
# DeepSpeed Team
|
|
|
|
import time
|
|
from numpy import mean
|
|
from deepspeed.utils.logging import print_dist
|
|
from deepspeed.accelerator import get_accelerator
|
|
|
|
FORWARD_MICRO_TIMER = 'fwd_microstep'
|
|
FORWARD_GLOBAL_TIMER = 'fwd'
|
|
BACKWARD_MICRO_TIMER = 'bwd_microstep'
|
|
BACKWARD_GLOBAL_TIMER = 'bwd'
|
|
BACKWARD_INNER_MICRO_TIMER = 'bwd_inner_microstep'
|
|
BACKWARD_INNER_GLOBAL_TIMER = 'bwd_inner'
|
|
BACKWARD_REDUCE_MICRO_TIMER = 'bwd_allreduce_microstep'
|
|
BACKWARD_REDUCE_GLOBAL_TIMER = 'bwd_allreduce'
|
|
STEP_MICRO_TIMER = 'step_microstep'
|
|
STEP_GLOBAL_TIMER = 'step'
|
|
TIME_EPSILON = 1e-6
|
|
|
|
try:
|
|
import psutil
|
|
|
|
PSUTILS_INSTALLED = True
|
|
except ImportError:
|
|
PSUTILS_INSTALLED = False
|
|
pass
|
|
|
|
|
|
class CudaEventTimer(object):
|
|
|
|
def __init__(self, start_event: get_accelerator().Event, end_event: get_accelerator().Event):
|
|
self.start_event = start_event
|
|
self.end_event = end_event
|
|
|
|
def get_elapsed_msec(self):
|
|
get_accelerator().current_stream().wait_event(self.end_event)
|
|
self.end_event.synchronize()
|
|
return self.start_event.elapsed_time(self.end_event)
|
|
|
|
|
|
class SynchronizedWallClockTimer:
|
|
"""Group of timers. Borrowed from Nvidia Megatron code"""
|
|
|
|
class Timer:
|
|
"""Timer."""
|
|
|
|
def __init__(self, name):
|
|
self.name_ = name
|
|
self.started_ = False
|
|
self.event_timers = []
|
|
self.use_host_timer = get_accelerator().use_host_timers()
|
|
self.start_event = None
|
|
self.elapsed_records = None
|
|
self.start_time = 0.0
|
|
self.end_time = 0.0
|
|
|
|
def start(self):
|
|
"""Start the timer."""
|
|
assert not self.started_, f"{self.name_} timer has already been started"
|
|
if self.use_host_timer:
|
|
self.start_time = time.time()
|
|
else:
|
|
event_class = get_accelerator().Event
|
|
self.start_event = event_class(enable_timing=True)
|
|
self.start_event.record()
|
|
self.started_ = True
|
|
|
|
def stop(self, reset=False, record=False):
|
|
"""Stop the timer."""
|
|
assert self.started_, "timer is not started"
|
|
event_class = get_accelerator().Event
|
|
if self.use_host_timer:
|
|
self.end_time = time.time()
|
|
self.event_timers.append(self.end_time - self.start_time)
|
|
else:
|
|
event_class = get_accelerator().Event
|
|
end_event = event_class(enable_timing=True)
|
|
end_event.record()
|
|
self.event_timers.append(CudaEventTimer(self.start_event, end_event))
|
|
self.start_event = None
|
|
self.started_ = False
|
|
|
|
def _get_elapsed_msec(self):
|
|
if self.use_host_timer:
|
|
self.elapsed_records = [et * 1000.0 for et in self.event_timers]
|
|
else:
|
|
self.elapsed_records = [et.get_elapsed_msec() for et in self.event_timers]
|
|
return sum(self.elapsed_records)
|
|
|
|
def reset(self):
|
|
"""Reset timer."""
|
|
self.started_ = False
|
|
self.start_event = None
|
|
self.elapsed_records = None
|
|
self.event_timers.clear()
|
|
|
|
def elapsed(self, reset=True):
|
|
"""Calculate the elapsed time."""
|
|
started_ = self.started_
|
|
# If the timing in progress, end it first.
|
|
if self.started_:
|
|
self.stop()
|
|
# Get the elapsed time.
|
|
elapsed_ = self._get_elapsed_msec()
|
|
# Reset the elapsed time
|
|
if reset:
|
|
self.reset()
|
|
# If timing was in progress, set it back.
|
|
if started_:
|
|
self.start()
|
|
return elapsed_
|
|
|
|
def mean(self):
|
|
self.elapsed(reset=False)
|
|
return trim_mean(self.elapsed_records, 0.1)
|
|
|
|
def __init__(self):
|
|
self.timers = {}
|
|
|
|
def get_timers(self):
|
|
return self.timers
|
|
|
|
def __call__(self, name):
|
|
if name not in self.timers:
|
|
self.timers[name] = self.Timer(name)
|
|
return self.timers[name]
|
|
|
|
@staticmethod
|
|
def memory_usage():
|
|
alloc = "mem_allocated: {:.4f} GB".format(get_accelerator().memory_allocated() / (1024 * 1024 * 1024))
|
|
max_alloc = "max_mem_allocated: {:.4f} GB".format(get_accelerator().max_memory_allocated() /
|
|
(1024 * 1024 * 1024))
|
|
cache = "cache_allocated: {:.4f} GB".format(get_accelerator().memory_cached() / (1024 * 1024 * 1024))
|
|
max_cache = "max_cache_allocated: {:.4f} GB".format(get_accelerator().max_memory_cached() /
|
|
(1024 * 1024 * 1024))
|
|
return " | {} | {} | {} | {}".format(alloc, max_alloc, cache, max_cache)
|
|
|
|
def log(self, names, normalizer=1.0, reset=True, memory_breakdown=False, ranks=None):
|
|
"""Log a group of timers."""
|
|
assert normalizer > 0.0
|
|
string = "time (ms)"
|
|
for name in names:
|
|
if name in self.timers:
|
|
elapsed_time = (self.timers[name].elapsed(reset=reset) / normalizer)
|
|
string += " | {}: {:.2f}".format(name, elapsed_time)
|
|
|
|
# timers logging should be independent of the global log level it's already conditional on wall_clock_breakdown being True, so using use_logger=False will always print the stats
|
|
print_dist(string, ranks=ranks or [0])
|
|
|
|
def get_mean(self, names, normalizer=1.0, reset=True):
|
|
"""Get the mean of a group of timers."""
|
|
assert normalizer > 0.0
|
|
means = {}
|
|
for name in names:
|
|
if name in self.timers:
|
|
elapsed_time = (self.timers[name].mean() * 1000.0 / normalizer)
|
|
means[name] = elapsed_time
|
|
return means
|
|
|
|
|
|
class NoopTimer:
|
|
|
|
class Timer:
|
|
|
|
def start(self):
|
|
...
|
|
|
|
def reset(self):
|
|
...
|
|
|
|
def stop(self, **kwargs):
|
|
...
|
|
|
|
def elapsed(self, **kwargs):
|
|
return 0
|
|
|
|
def mean(self):
|
|
return 0
|
|
|
|
def __init__(self):
|
|
self.timer = self.Timer()
|
|
|
|
def __call__(self, name):
|
|
return self.timer
|
|
|
|
def get_timers(self):
|
|
return {}
|
|
|
|
def log(self, names, normalizer=1.0, reset=True, memory_breakdown=False, ranks=None):
|
|
...
|
|
|
|
def get_mean(self, names, normalizer=1.0, reset=True):
|
|
...
|
|
|
|
|
|
class ThroughputTimer:
|
|
|
|
def __init__(self, config, batch_size, start_step=2, steps_per_output=None, monitor_memory=False, logging_fn=None):
|
|
from deepspeed.utils import logger
|
|
self.config = config
|
|
self.start_time = 0
|
|
self.end_time = 0
|
|
self.started = False
|
|
self.batch_size = 1 if batch_size is None else batch_size
|
|
self.start_step = start_step
|
|
self.epoch_count = 0
|
|
self.micro_step_count = 0
|
|
self.global_step_count = 0
|
|
self.total_elapsed_time = 0
|
|
self.step_elapsed_time = 0
|
|
self.steps_per_output = steps_per_output
|
|
self.monitor_memory = monitor_memory
|
|
self.logging = logging_fn
|
|
if self.logging is None:
|
|
self.logging = logger.info
|
|
self.initialized = False
|
|
|
|
if self.monitor_memory and not PSUTILS_INSTALLED:
|
|
raise ImportError("Unable to import 'psutils', please install package")
|
|
|
|
def update_epoch_count(self):
|
|
self.epoch_count += 1
|
|
self.micro_step_count = 0
|
|
|
|
def _init_timer(self):
|
|
self.initialized = True
|
|
|
|
def start(self):
|
|
if not self.config.enabled:
|
|
return
|
|
self._init_timer()
|
|
self.started = True
|
|
if self.global_step_count >= self.start_step:
|
|
if self.config.synchronized:
|
|
get_accelerator().synchronize()
|
|
self.start_time = time.time()
|
|
|
|
def _is_report_boundary(self):
|
|
if self.steps_per_output is None:
|
|
return False
|
|
return self.global_step_count % self.steps_per_output == 0
|
|
|
|
def stop(self, global_step=False, report_speed=True):
|
|
if not self.config.enabled or not self.started:
|
|
return
|
|
self.started = False
|
|
self.micro_step_count += 1
|
|
if global_step:
|
|
self.global_step_count += 1
|
|
|
|
if self.start_time > 0:
|
|
if self.config.synchronized:
|
|
get_accelerator().synchronize()
|
|
self.end_time = time.time()
|
|
duration = self.end_time - self.start_time
|
|
self.total_elapsed_time += duration
|
|
self.step_elapsed_time += duration
|
|
|
|
if global_step:
|
|
if report_speed and self._is_report_boundary():
|
|
self.logging(
|
|
"epoch={}/micro_step={}/global_step={}, RunningAvgSamplesPerSec={}, CurrSamplesPerSec={}, "
|
|
"MemAllocated={}GB, MaxMemAllocated={}GB".format(
|
|
self.epoch_count,
|
|
self.micro_step_count,
|
|
self.global_step_count,
|
|
self.avg_samples_per_sec(),
|
|
self.batch_size / (self.step_elapsed_time + TIME_EPSILON),
|
|
round(get_accelerator().memory_allocated() / 1024**3, 2),
|
|
round(get_accelerator().max_memory_allocated() / 1024**3, 2),
|
|
))
|
|
if self.monitor_memory:
|
|
virt_mem = psutil.virtual_memory()
|
|
swap = psutil.swap_memory()
|
|
self.logging("epoch={}/micro_step={}/global_step={}, vm %: {}, swap %: {}".format(
|
|
self.epoch_count,
|
|
self.micro_step_count,
|
|
self.global_step_count,
|
|
virt_mem.percent,
|
|
swap.percent,
|
|
))
|
|
self.step_elapsed_time = 0
|
|
|
|
def avg_samples_per_sec(self):
|
|
if self.global_step_count > 0:
|
|
total_step_offset = self.global_step_count - self.start_step
|
|
avg_time_per_step = self.total_elapsed_time / total_step_offset
|
|
# training samples per second
|
|
return self.batch_size / avg_time_per_step
|
|
return float("-inf")
|
|
|
|
|
|
def trim_mean(data, trim_percent):
|
|
"""Compute the trimmed mean of a list of numbers.
|
|
|
|
Args:
|
|
data (list): List of numbers.
|
|
trim_percent (float): Percentage of data to trim.
|
|
|
|
Returns:
|
|
float: Trimmed mean.
|
|
"""
|
|
assert 0.0 <= trim_percent <= 1.0
|
|
n = len(data)
|
|
# Account for edge case of empty list
|
|
if len(data) == 0:
|
|
return 0
|
|
# sorted(), not data.sort(): CommsLogger passes its stored latency/algbw/busbw
|
|
# lists straight in, and sorting them in place reorders each independently and
|
|
# destroys the index correspondence between them.
|
|
data = sorted(data)
|
|
k = int(round(n * (trim_percent)))
|
|
return mean(data[k:n - k])
|