Fix checkpoint warning for floats

[ZhaofengWu]

#1819 added this warning when saving checkpoints. https://github.com/PyTorchLightning/pytorch-lightning/blob/bd49b07fbba09b1e7d8851ee5a1ffce3d5925e9e/pytorch_lightning/callbacks/model_checkpoint.py#L259-L263

1. Is there a reason why native Python floats aren't supported? I think it's actually quite common especially when libraries e.g. scipy are involved when computing the metrics.
2. It says checkpoint not saved, but actually checkpoints are still saved. This check doesn't actually change the saving logic besides giving that warning.

[Borda]

@justusschock @SkafteNicki mind have a look :]

[SkafteNicki]

The reason it is still working is due to the fact that it is being converted to a torch.tensor later on:
https://github.com/PyTorchLightning/pytorch-lightning/blob/bd49b07fbba09b1e7d8851ee5a1ffce3d5925e9e/pytorch_lightning/callbacks/model_checkpoint.py#L176-L181
so in that sense they are supported. I guess the warning is there due to:

• If you are returning something other than a native float, like a dict, you will get a non-informative error (RuntimeError: Could not infer dtype of dict). The warning could hopefully help you figure out what is going on. So the only way we can guarantee that this will work, is if you return a torch.Tensor.
• The PR that implemented this, tried to fix some ddp bugs. I cannot completely figure out how this is related to ddp right now, but I guess that again torch.Tensor is the only type that guarantee that this works in ddp mode.

[ZhaofengWu]

If types such as floats are supported, should they be excluded from the check and don't show warnings? And IMHO the warning message saying checkpoints won't be saved while they actually are is not desirable.

[Borda]

sounds reasonable, mind draft a PR? 🐰

[edenlightning]

@ZhaofengWu Checking in :)

[ZhaofengWu]

Ah sorry I overlooked this. I might not have the time recently to do this. Sorry :(

[ddrevicky]

You can assign this to me :).

[ddrevicky]

Logging floats actually doesn't work with the current version.

I tried logging a metric with different dtypes (tensor, float, int, dict, list) and it only works with tensors. See this colab.

For the other types it fails even before reaching the model_checkpoint.py code because of reduction of the per validation step results at the end of a validation epoch in step_result.py.

    873 def weighted_mean(result, weights):
--> 874     weights = weights.to(result.device)
    875     numerator = torch.dot(result.float(), weights.transpose(-1, 0).float())
    876     result = numerator / weights.sum().float()

AttributeError: 'list' object has no attribute 'device'

For example if in 2 validation steps the logged metric is 42.0 and 43.0 (floats) the collected metric values to be reduced are [42.0, 43.0] and they do not have the device method.

I think this issue is connected with #3678 and #2636.

Basically, shouldn't we ensure that the metrics that user logs fulfill some basic properties? Then later in the program we can assume that they are valid and avoid later extra checks like above in this issue:

• tensor (clone and detach the tensor if possible to deal with EvalResult does not clone logged tensors #3678) or convertible to tensor (in which case convert straight away)
• non nan/inf (to deal with nan metric breaking ModelCheckpoint #2636)
• scalar (perhaps only for a monitored metric since logging 2D tensors is a desired functionality EvalResult can't log 2D tensor #3697 )

[Borda]

For example if in 2 validation steps the logged metric is 42.0 and 43.0 (floats) the collected metric values to be reduced are [42.0, 43.0] and they do not have the device method.

then you need to make a tensor which has those attributes... :]

[ddrevicky]

Yeah :) Just found out this PR which could fix float logging when merged.