ModelCheckpoint does not create full path

[ydcjeff]

🐛 Bug

To Reproduce

Run checkpoint_callback = ModelCheckpoint('my/path/')
Only my folder is created.

I think this line discard the last trailing slash. So the directories are not created as intended when the paths are getting split.

Expected behavior

Path should be fully created.

[awaelchli]

I do not think that the trailing slash is the root problem. We also want that "my/path" should be interpreted as a path to a folder.
I think a reasonable fix is to force the user to provide the extension when they want a file. This way, we can easily differentiate between folders and files.

[rohitgr7]

how about having two parameters, dirpath and filename? dirpath should be required (or maybe default to '.') and filename can be optional?

[awaelchli]

I don't think we can make it required, because we have default_root_dir in Trainer, which should work as expected.

[ydcjeff]

@awaelchli I mean trailing slash isn't the problem, discarding the last trailing slash is the problem.

[rohitgr7]

yeah it can be optional, no problem

[rohitgr7]

but @ydcjeff it should work with both 'my/path/' and 'my/path'.

[awaelchli]

I think what jeff wants to say is that

path, filename = os.path.split("a/b/c")

vs.

path, filename = os.path.split("a/b/c/")

in the former, filename = "c", and in the latter filename = ""

[ydcjeff]

yes, it's that.
When we give filepath=mnist/ckpt/ or filepath=mnist/ckpt, os.path.realpath() outputs mnist/ckpt
and when we do splitting, it becomes like path=mnist and filename=ckpt

[rohitgr7]

I think separate parameters dirpath and filename would be more flexible for the user. Not sure if it will break any backward compatibility with other parameters from Trainer or logger.

[edenlightning]

Anyone can send a PR?

[rohitgr7]

let's discuss it on #3583. The approach suggested there will solve this issue.