Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle checkpoint dirpath suffix in NeptuneLogger #18863

Merged
merged 33 commits into from
Nov 25, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
51f0b92
handle checkpoint dirpath suffix
AleksanderWWW Oct 25, 2023
1adfb10
handle without relying on system separator
AleksanderWWW Oct 25, 2023
923b6fe
fix for windows
AleksanderWWW Oct 25, 2023
c3971de
fix
AleksanderWWW Oct 25, 2023
c1cfcfa
Apply suggestions from code review
AleksanderWWW Oct 25, 2023
5af40d7
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Oct 25, 2023
08cbda8
Update src/lightning/pytorch/loggers/neptune.py
AleksanderWWW Oct 26, 2023
66bd7ab
Merge branch 'master' into aw/fix-path-suffix
AleksanderWWW Oct 26, 2023
3ab88c0
add doctest + function rename
AleksanderWWW Oct 27, 2023
fdd2728
single quotes in doctest
AleksanderWWW Oct 27, 2023
a513dcb
Merge branch 'master' into aw/fix-path-suffix
AleksanderWWW Oct 27, 2023
1ec4687
Apply suggestions from code review
AleksanderWWW Oct 27, 2023
84b4b32
use os.path.normpath
AleksanderWWW Oct 29, 2023
6d7f9f7
Merge branch 'master' into aw/fix-path-suffix
AleksanderWWW Oct 29, 2023
1db1922
fix unit tests
AleksanderWWW Oct 30, 2023
d4b696f
fix for windows
AleksanderWWW Oct 30, 2023
804d59d
Merge branch 'master' into aw/fix-path-suffix
AleksanderWWW Oct 30, 2023
1a20eee
Update src/lightning/pytorch/loggers/neptune.py
AleksanderWWW Oct 30, 2023
72ed887
Merge branch 'master' into aw/fix-path-suffix
AleksanderWWW Oct 30, 2023
c4f0eb2
Merge branch 'master' into aw/fix-path-suffix
AleksanderWWW Oct 30, 2023
0ef1ff5
Merge branch 'master' into aw/fix-path-suffix
AleksanderWWW Nov 2, 2023
6e9d09c
Merge branch 'master' into aw/fix-path-suffix
AleksanderWWW Nov 2, 2023
9f8c816
Merge branch 'master' into aw/fix-path-suffix
AleksanderWWW Nov 6, 2023
05a27ec
Merge branch 'master' into aw/fix-path-suffix
Borda Nov 18, 2023
8de17fe
use pathlib
AleksanderWWW Nov 22, 2023
cba5a3c
Merge branch 'master' into aw/fix-path-suffix
AleksanderWWW Nov 22, 2023
3a127eb
typing
AleksanderWWW Nov 22, 2023
404acf9
typing
AleksanderWWW Nov 22, 2023
bdb978a
norm the path at return
AleksanderWWW Nov 22, 2023
e0f246d
fix for windows
AleksanderWWW Nov 22, 2023
9f2c968
add test case
awaelchli Nov 25, 2023
e711a13
Merge branch 'master' into aw/fix-path-suffix
awaelchli Nov 25, 2023
1f5130e
chlog
awaelchli Nov 25, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/lightning/pytorch/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,9 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
- Fixed `ModelCheckpoint` not expanding the `dirpath` if it has the `~` (home) prefix ([#19058](https://github.com/Lightning-AI/lightning/pull/19058))


- Fixed handling checkpoint dirpath suffix in NeptuneLogger ([#18863](https://github.com/Lightning-AI/lightning/pull/18863))



## [2.1.2] - 2023-11-15

Expand Down
9 changes: 5 additions & 4 deletions src/lightning/pytorch/loggers/neptune.py
Original file line number Diff line number Diff line change
Expand Up @@ -557,13 +557,14 @@ def after_save_checkpoint(self, checkpoint_callback: Checkpoint) -> None:
def _get_full_model_name(model_path: str, checkpoint_callback: Checkpoint) -> str:
"""Returns model name which is string `model_path` appended to `checkpoint_callback.dirpath`."""
if hasattr(checkpoint_callback, "dirpath"):
expected_model_path = f"{checkpoint_callback.dirpath}{os.path.sep}"
model_path = os.path.normpath(model_path)
expected_model_path = os.path.normpath(checkpoint_callback.dirpath)
if not model_path.startswith(expected_model_path):
AleksanderWWW marked this conversation as resolved.
Show resolved Hide resolved
raise ValueError(f"{model_path} was expected to start with {expected_model_path}.")
# Remove extension from filepath
filepath, _ = os.path.splitext(model_path[len(expected_model_path) :])
return filepath
return model_path
filepath, _ = os.path.splitext(model_path[len(expected_model_path) + 1 :])
return filepath.replace(os.sep, "/")
return model_path.replace(os.sep, "/")

@classmethod
def _get_full_model_names_from_exp_structure(cls, exp_structure: Dict[str, Any], namespace: str) -> Set[str]:
Expand Down
6 changes: 4 additions & 2 deletions tests/tests_pytorch/loggers/test_neptune.py
Original file line number Diff line number Diff line change
Expand Up @@ -284,10 +284,12 @@ def test_get_full_model_name():
os.path.join("foo", "bar", "key/in/parts.ext"),
SimpleCheckpoint(dirpath=os.path.join("foo", "bar")),
),
("key", os.path.join("../foo", "bar", "key.ext"), SimpleCheckpoint(dirpath=os.path.join("../foo", "bar"))),
("key", os.path.join("foo", "key.ext"), SimpleCheckpoint(dirpath=os.path.join("./foo", "bar/../"))),
]

for expected_model_name, *key_and_path in test_input_data:
assert NeptuneLogger._get_full_model_name(*key_and_path) == expected_model_name
for expected_model_name, model_path, checkpoint in test_input_data:
assert NeptuneLogger._get_full_model_name(model_path, checkpoint) == expected_model_name


def test_get_full_model_names_from_exp_structure():
Expand Down