ModelCheckpoint filename unable to use metrics that contain a slash

[its-dron]

🐛 Bug

ModelCheckpoint is unable to save filenames that reference a metric with a slash in their name. I use grouped metrics for tensorboard, and would like to save my files containing my loss: val/loss. However, ModelCheckpoint uses os.path.split, which splits the file name: https://github.com/PyTorchLightning/pytorch-lightning/blob/6ac0958166c66ed599c96737b587232b7a33d89e/pytorch_lightning/callbacks/model_checkpoint.py#L258

If I try to use

ModelCheckpoint("root/dir/{epoch}_{val/loss:.5f}")

The above evaluates to

self.dirpath = "root/dir/{epoch}_{val" 
self.filename = "loss:.5f}"

This inevitably causes failure when attempting to format the output path.

To Reproduce

As above, log a metric with a slash, then use it in model checkpoint output

Code sample

class Module(pl.LightningModule):
...
    def validation_step(self, batch, batch_idx):
        x, y = batch
        logits = self.forward(x)
        loss = self.loss_fn(logits, y)
        self.log('val/loss', loss, on_epoch=True)
        return loss

...
def main():
    trainer = pl.Trainer(checkpoint_callback=ModelCheckpoint("{epoch}_{val/loss:.5f}"))

Expected behavior

Split only along file path boundaries, ignoring variable names yet-to-be-formatted.
Per the previous example, we'd expect:

self.dirpath = "root/dir" 
self.filename = "{epoch}_{val/loss:.5f}"

Environment

• CUDA:
  • GPU:
    • Tesla V100-SXM2-16GB
    • Tesla V100-SXM2-16GB
    • Tesla V100-SXM2-16GB
    • Tesla V100-SXM2-16GB
  • available: True
  • version: 10.2
• Packages:
  • numpy: 1.19.1
  • pyTorch_debug: False
  • pyTorch_version: 1.6.0
  • pytorch-lightning: 0.10.0
  • tqdm: 4.50.0
• System:
  • OS: Linux
  • architecture:
    • 64bit
    • ELF
  • processor: x86_64
  • python: 3.8.5
  • version: Proposal for help #1 SMP Fri Sep 4 14:19:36 UTC 2020

[github-actions]

Hi! thanks for your contribution!, great first issue!

[ndrplz]

Cool, I was actually here to open this issue :)

I also encountered this problem. I think it's quite common to use the forward slash to group metrics in tensorboard, but this obviously clashes with this formatting.

I also tried to escape it, but with little success.

Edit

A negative look-ahead regex like this should do the job (assuming there are no nested brackets), but feels a bit an overkill.

[edenlightning]

Thanks for raising the issue! would you like submit a PR for the fix?

[ozen]

#4213 doesn't fix this. Although the callback can now parse the metric names containing slashes, slashes in the resulting file name create directories during saving for obvious reasons. Since file names cannot contain slashes, the way the callback formats the file name must be changed.

[its-dron]

@ozen Am I correct that the issue now is that checkpoint names automatically include the metric name in addition to the value?
Could we make that configurable, so I can enable/disable automatic name insertion?

[ozen]

@its-dron exactly. Making it configurable could be ugly though. Maybe the slashes can be automatically converted to something like underscores.

[its-dron]

Personally, I'd prefer to not have the auto-inserted metric names at all. I'd prefer to have fine-grain control over what my files are named instead of having the library decide for me.

But that seems like a drastic change to drop at this point.

[ozen]

@rohitgr7 could you reopen this issue?

[rohitgr7]

Is it still an issue or not working as expected?