Fix saving relative symlink for ModelCheckpoint callback (#19303)

Co-authored-by: shenmishajing <[email protected]>

Author: 2 people

src/lightning/pytorch/callbacks/model_checkpoint.py

@@ -390,7 +390,7 @@ def _link_checkpoint(trainer: "pl.Trainer", filepath: str, linkpath: str) -> Non
             elif os.path.isdir(linkpath):
                 shutil.rmtree(linkpath)
             try:
-                os.symlink(filepath, linkpath)
+                os.symlink(os.path.relpath(filepath, os.path.dirname(linkpath)), linkpath)
             except OSError:
                 # on Windows, special permissions are required to create symbolic links as a regular user
                 # fall back to copying the file

tests/tests_pytorch/checkpointing/test_model_checkpoint.py

@@ -524,20 +524,23 @@ def test_model_checkpoint_link_checkpoint(tmp_path):
     ModelCheckpoint._link_checkpoint(trainer, filepath=str(file), linkpath=str(link))
     assert os.path.islink(link)
     assert os.path.realpath(link) == str(file)
+    assert not os.path.isabs(os.readlink(link))
 
     # link exists (is a file)
     new_file1 = tmp_path / "new_file1"
     new_file1.touch()
     ModelCheckpoint._link_checkpoint(trainer, filepath=str(new_file1), linkpath=str(link))
     assert os.path.islink(link)
     assert os.path.realpath(link) == str(new_file1)
+    assert not os.path.isabs(os.readlink(link))
 
     # link exists (is a link)
     new_file2 = tmp_path / "new_file2"
     new_file2.touch()
     ModelCheckpoint._link_checkpoint(trainer, filepath=str(new_file2), linkpath=str(link))
     assert os.path.islink(link)
     assert os.path.realpath(link) == str(new_file2)
+    assert not os.path.isabs(os.readlink(link))
 
     # link exists (is a folder)
     folder = tmp_path / "folder"
@@ -547,13 +550,15 @@ def test_model_checkpoint_link_checkpoint(tmp_path):
     ModelCheckpoint._link_checkpoint(trainer, filepath=str(folder), linkpath=str(folder_link))
     assert os.path.islink(folder_link)
     assert os.path.realpath(folder_link) == str(folder)
+    assert not os.path.isabs(os.readlink(folder_link))
 
     # link exists (is a link to a folder)
     new_folder = tmp_path / "new_folder"
     new_folder.mkdir()
     ModelCheckpoint._link_checkpoint(trainer, filepath=str(new_folder), linkpath=str(folder_link))
     assert os.path.islink(folder_link)
     assert os.path.realpath(folder_link) == str(new_folder)
+    assert not os.path.isabs(os.readlink(folder_link))
 
     # simulate permission error on Windows (creation of symbolic links requires privileges)
     file = tmp_path / "win_file"
@@ -565,6 +570,22 @@ def test_model_checkpoint_link_checkpoint(tmp_path):
     assert os.path.isfile(link)  # fall back to copying instead of linking
 
 
+def test_model_checkpoint_link_checkpoint_relative_path(tmp_path, monkeypatch):
+    """Test that linking a checkpoint works with relative paths."""
+    trainer = Mock()
+    monkeypatch.chdir(tmp_path)
+
+    folder = Path("x/z/z")
+    folder.mkdir(parents=True)
+    file = folder / "file"
+    file.touch()
+    link = folder / "link"
+    ModelCheckpoint._link_checkpoint(trainer, filepath=str(file.absolute()), linkpath=str(link.absolute()))
+    assert os.path.islink(link)
+    assert Path(os.readlink(link)) == file.relative_to(folder)
+    assert not os.path.isabs(os.readlink(link))
+
+
 def test_invalid_top_k(tmpdir):
     """Make sure that a MisconfigurationException is raised for a negative save_top_k argument."""
     with pytest.raises(MisconfigurationException, match=r".*Must be >= -1"):