Skip to content

Commit 3a47d1d

Browse files
Kymi808tohtanasfc-gh-truwase
authored
activation_checkpointing: default num_layers to None so configure() assert fires (#8041)
## Summary Calling `deepspeed.checkpointing.configure(contiguous_checkpointing=True, partition_activations=True)` *without* setting `num_checkpoints` (or otherwise providing a layer count) is supposed to fail fast with the assert at `deepspeed/runtime/activation_checkpointing/checkpointing.py:1108`: ```python if CONTIGUOUS_CHECKPOINTING: assert num_layers is not None, "Must specify the number of layers with contiguous memory checkpointing" ``` That assert never fires because `_configure_defaults()` (called inside `configure()` at line 1079) initialized: ```python num_layers = False ``` `False is not None` is `True`, so the assert silently passes. The user instead hits a much later cryptic `IndexError` from `range(num_layers)` (lines 399 / 406) or a 0-element allocation from `numel * num_layers` (lines 457 / 461). The module-level default at line 46 is already `num_layers = None`, and every other path that sets it (`set_num_layers`, `config.number_checkpoints`) assigns an integer — only `_configure_defaults` used `False`, which looks like a copy-paste from the surrounding `PARTITION_ACTIVATIONS = False` etc. block. ## Fix ```diff - num_layers = False + num_layers = None ``` One-character change. No callers compare `num_layers` to `False` (downstream uses are `range(num_layers)` and `numel * num_layers`, both requiring an int), so the only path this changes is the broken one: users now get the documented "Must specify the number of layers" assert instead of a downstream `IndexError`. ## Test Adds `test_configure_with_contiguous_checkpointing_requires_num_checkpoints` to the existing `tests/unit/runtime/activation_checkpointing/test_activation_checkpointing.py` (consolidating per `AGENTS.md`, not a new file). It calls `configure(contiguous_checkpointing=True, partition_activations=True)` and asserts the expected `AssertionError` matches `"number of layers"`. On `main` the assert silently passes and the test fails; with this change it passes. ## CI/lint - `pre-commit run --files <changed files>`: all hooks pass (yapf, flake8, `check-torchdist`, `check-license`, codespell, `check-torchcuda`). - DCO `Signed-off-by` on the commit. - No competing PR (`gh pr list --search "num_layers checkpointing OR _configure_defaults OR contiguous_checkpointing assert"`). --------- Signed-off-by: Kymi808 <zeng.kyle13@gmail.com> Co-authored-by: Masahiro Tanaka <81312776+tohtana@users.noreply.github.com> Co-authored-by: Olatunji Ruwase <tunji.ruwase@snowflake.com>
1 parent 4421665 commit 3a47d1d

2 files changed

Lines changed: 43 additions & 1 deletion

File tree

deepspeed/runtime/activation_checkpointing/checkpointing.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1019,7 +1019,7 @@ def _configure_defaults():
10191019

10201020
PARTITION_ACTIVATIONS = False
10211021
CONTIGUOUS_CHECKPOINTING = False
1022-
num_layers = False
1022+
num_layers = None
10231023
CPU_CHECKPOINT = False
10241024
SYNCHRONIZE = False
10251025
PROFILE_TIME = False

tests/unit/runtime/activation_checkpointing/test_activation_checkpointing.py

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -309,3 +309,45 @@ def __init__(self):
309309
assert model._is_checkpointable([layers[0]]) == True # ParallelTransformerLayerPipe
310310
assert model._is_checkpointable([layers[1]]) == True # GMLPBlock
311311
assert model._is_checkpointable([layers[2]]) == False # Linear layer
312+
313+
314+
def test_configure_with_contiguous_checkpointing_requires_num_checkpoints():
315+
# Regression: ``_configure_defaults`` previously initialized ``num_layers``
316+
# to ``False`` while the assert below uses ``is not None``; ``False is not
317+
# None`` is True, so the missing-config assert silently passed and a
318+
# cryptic ``IndexError`` surfaced later from ``range(num_layers)``. With
319+
# the default switched to ``None`` (matching the module-level default),
320+
# the helpful assert message fires at the configure() call site.
321+
#
322+
# ``configure()`` mutates module globals before raising, so snapshot and
323+
# restore them around the call to avoid order-dependent failures in other
324+
# activation-checkpointing tests sharing the same pytest worker.
325+
cp = deepspeed.checkpointing
326+
saved = (
327+
cp.PARTITION_ACTIVATIONS,
328+
cp.CONTIGUOUS_CHECKPOINTING,
329+
cp.num_layers,
330+
cp.CPU_CHECKPOINT,
331+
cp.SYNCHRONIZE,
332+
cp.PROFILE_TIME,
333+
cp.mpu,
334+
cp.deepspeed_checkpointing_enabled,
335+
)
336+
try:
337+
with pytest.raises(AssertionError, match="number of layers"):
338+
deepspeed.checkpointing.configure(
339+
mpu_=None,
340+
partition_activations=True,
341+
contiguous_checkpointing=True,
342+
)
343+
finally:
344+
(
345+
cp.PARTITION_ACTIVATIONS,
346+
cp.CONTIGUOUS_CHECKPOINTING,
347+
cp.num_layers,
348+
cp.CPU_CHECKPOINT,
349+
cp.SYNCHRONIZE,
350+
cp.PROFILE_TIME,
351+
cp.mpu,
352+
cp.deepspeed_checkpointing_enabled,
353+
) = saved

0 commit comments

Comments
 (0)