Skip to content

Commit f7133b4

Browse files
committed
Fix FSDP remote checkpointing tests by using local:// instead of memory://
The `memory://` fsspec filesystem is process-local and cannot be used across separately-spawned ranks in distributed tests. Switch to `local://` which simulates a remote filesystem but is backed by a shared disk, and broadcast the path from rank 0. Additionally: - Skip FSDP GPU tests on Windows. - Conditionally import `_fsspec_filesystem` in reader/writer selection tests to support older PyTorch versions. TAG=agy CONV=90d4b033-0f6c-4895-bf1c-4184200442cd
1 parent 7ec8a6b commit f7133b4

3 files changed

Lines changed: 23 additions & 12 deletions

File tree

tests/tests_fabric/strategies/test_fsdp.py

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -499,17 +499,21 @@ def test_is_sharded_checkpoint_remote_memory():
499499

500500
def test_distributed_checkpoint_reader_writer_selection(tmp_path):
501501
from torch.distributed.checkpoint import FileSystemReader, FileSystemWriter
502-
from torch.distributed.checkpoint._fsspec_filesystem import FsspecReader, FsspecWriter
503502

504503
from lightning.fabric.strategies.fsdp import (
505504
_get_distributed_checkpoint_reader,
506505
_get_distributed_checkpoint_writer,
507506
)
508507

508+
# the local file-system writer/reader are always available
509509
assert isinstance(_get_distributed_checkpoint_writer(str(tmp_path)), FileSystemWriter)
510510
assert isinstance(_get_distributed_checkpoint_reader(str(tmp_path)), FileSystemReader)
511-
assert isinstance(_get_distributed_checkpoint_writer("memory:///w/ckpt"), FsspecWriter)
512-
assert isinstance(_get_distributed_checkpoint_reader("memory:///w/ckpt"), FsspecReader)
511+
512+
# the fsspec writer/reader live in a private torch module; skip the remote assertions if the
513+
# installed torch does not ship it (keeps the oldest-supported-torch CI green)
514+
fsspec_fs = pytest.importorskip("torch.distributed.checkpoint._fsspec_filesystem")
515+
assert isinstance(_get_distributed_checkpoint_writer("memory:///w/ckpt"), fsspec_fs.FsspecWriter)
516+
assert isinstance(_get_distributed_checkpoint_reader("memory:///w/ckpt"), fsspec_fs.FsspecReader)
513517

514518

515519
def test_distributed_checkpoint_save_load_remote_memory():
@@ -581,10 +585,10 @@ def test_load_raw_module_state_from_path_remote(monkeypatch):
581585
assert "via_lazy" not in called
582586

583587

584-
@RunIf(min_cuda_gpus=2, standalone=True)
588+
@RunIf(min_cuda_gpus=2, skip_windows=True, standalone=True)
585589
@pytest.mark.parametrize("state_dict_type", ["full", "sharded"])
586-
def test_fsdp_remote_memory_roundtrip(state_dict_type):
587-
"""Save to and load from an fsspec ``memory://`` URL for both state-dict types."""
590+
def test_fsdp_remote_memory_roundtrip(state_dict_type, tmp_path):
591+
"""Save to and load from an fsspec remote-style URL for both state-dict types."""
588592
from lightning.fabric import Fabric
589593

590594
fabric = Fabric(accelerator="cuda", devices=2, strategy=FSDPStrategy(state_dict_type=state_dict_type))
@@ -595,7 +599,11 @@ def test_fsdp_remote_memory_roundtrip(state_dict_type):
595599
optimizer = torch.optim.SGD(model.parameters(), lr=0.1)
596600
optimizer = fabric.setup_optimizers(optimizer)
597601

598-
url = "memory:///ckpt-" + state_dict_type
602+
# `local://` is treated as a remote fsspec URL (it exercises the FsspecWriter/Reader path) but is
603+
# backed by real shared disk visible to every rank. `memory://` cannot be used here because its
604+
# store is process-local, so the separately-spawned ranks would not see each other's shards.
605+
# Broadcast rank 0's path so all ranks save/load from the same location.
606+
url = fabric.broadcast("local://" + str(tmp_path / ("ckpt-" + state_dict_type)))
599607
fabric.save(url, {"model": model, "optimizer": optimizer, "step": 1})
600608

601609
loaded = {"model": model, "optimizer": optimizer, "step": 0}

tests/tests_fabric/utilities/test_cloud_io.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,7 @@ def test_checkpoint_join_does_not_corrupt_remote_urls():
182182
def test_is_checkpoint_dir_local(tmp_path):
183183
d = tmp_path / "adir"
184184
d.mkdir()
185-
f = tmp_path / "afile"
185+
f = tmp_path / "a_file"
186186
f.write_text("x")
187187
assert _is_checkpoint_dir(d) is True
188188
assert _is_checkpoint_dir(f) is False

tests/tests_pytorch/strategies/test_fsdp.py

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1004,10 +1004,10 @@ def test_pl_save_checkpoint_does_not_corrupt_remote_path(monkeypatch):
10041004
assert captured["path"] == "gs://bucket/run/ckpt"
10051005

10061006

1007-
@RunIf(min_cuda_gpus=2, standalone=True)
1007+
@RunIf(min_cuda_gpus=2, skip_windows=True, standalone=True)
10081008
@pytest.mark.parametrize("state_dict_type", ["full", "sharded"])
1009-
def test_pl_fsdp_remote_memory_roundtrip(state_dict_type):
1010-
"""Save to and load from an fsspec ``memory://`` URL via the Trainer for both state-dict types."""
1009+
def test_pl_fsdp_remote_memory_roundtrip(state_dict_type, tmp_path):
1010+
"""Save to and load from an fsspec remote-style URL via the Trainer for both state-dict types."""
10111011
from lightning.pytorch import Trainer
10121012
from lightning.pytorch.demos.boring_classes import BoringModel
10131013

@@ -1021,7 +1021,10 @@ def test_pl_fsdp_remote_memory_roundtrip(state_dict_type):
10211021
logger=False,
10221022
)
10231023
trainer.fit(model)
1024-
url = "memory:///pl-ckpt-" + state_dict_type
1024+
# `local://` exercises the remote fsspec path but is backed by shared disk visible to all ranks;
1025+
# `memory://` is process-local and unusable across the separately-spawned ranks. Broadcast rank 0's
1026+
# path so every rank uses the same location.
1027+
url = trainer.strategy.broadcast("local://" + str(tmp_path / ("pl-ckpt-" + state_dict_type)))
10251028
trainer.save_checkpoint(url)
10261029
# loading back must not raise and must restore the path uncorrupted
10271030
trainer.strategy.load_checkpoint(url)

0 commit comments

Comments
 (0)