[RFC] Move logger v_num information out of progress bar and into simple log statement

[daniellepintz]

Proposed refactor

Remove v_num from the progress bar here:
https://github.com/PyTorchLightning/pytorch-lightning/blob/5da065e287b44e2c1fe4f7951003813ed45365c9/pytorch_lightning/callbacks/progress/base.py#L217-L223

And add a simple Python log statement at the start of training with the version number for each logger.

Motivation

1. It is a bit unclear to me why we have the logger v_num on the progress bar - there is no direct relation between the two. Why not have this information in a simple log statement at the start of training, and leave more space in the progress bar for other information? As @carmocca put it, the progress bar is "prime real estate".

2. When we have multiple loggers, we have the strange behavior of concatenating the two version numbers (relic of LoggerCollection) and if the concatenated version numbers is longer than 4 digits we truncate it:
   https://github.com/PyTorchLightning/pytorch-lightning/blob/5da065e287b44e2c1fe4f7951003813ed45365c9/pytorch_lightning/callbacks/progress/base.py#L220-L222

Instead, we can just have log statements like this at the start of training:

Initialized TensorBoardLogger with version X.
Initialized WandbLogger with version Y.

cc @justusschock @awaelchli @rohitgr7 @edward-io @Borda @ananthsub @kamil-kaczmarek @Raalsky @Blaizzy @carmocca

[semaphore-egg]

If my guess is correct:

My understanding:

When multiple training tasks are launched and we want to stop one of them based on the logging information, we need to idenfity which console corresponding to a target log/tensorboard.

Without an identifier, all consoles/terminals will look the same.
Of course, we can locate the console based on the training process (current batch index), but that might not be accurate if multiple task progresses are similar.

My Concern for proposed refactor:

Log statements at the start of training has some limitations: If too many information printed in the console, it can be diffcult to find out something printed in the beginning of training process. When
virtual terminal, e.g., screen command is used, it is impossible to scroll up to find out previous prints.

My Suggestion:

However, I feel v_num does not convey an obvious meaning. Something like training ID, logger ID or training version might be more clear for us, although they are too long for our precious single-line space.

Since this v_num is fixed in a training process, moving it to the begining of a line might be another choice. In this way, I prefer to totally remove v_num= and warp the value with a bracket.
From:
Epoch 0: 100%|########| 50/50 [00:25<00:00, 2.00it/s, loss=0.0468, v_num=7a22]
To:
[7a22] Epoch 0: 100%|########| 50/50 [00:25<00:00, 2.00it/s, loss=0.0468]

This modification follows the idea that: fixed items come first, mutable values follow.

[daniellepintz]

@semaphore-egg thanks for your input! @awaelchli @carmocca do you have any thoughts?

[awaelchli]

I agree with @semaphore-egg and his/her understanding is correct. We have the v_num for exactly this reason. I agree v_num is just a different name for "ver" or "version", so why not call it version directly? A removal in my opinion is not desired, as I have expressed in the past. I consider this a good default. @carmocca is in favor of removing it.

[carmocca]

it can be diffcult to find out something printed in the beginning of training process

But this is a problem of the tool used to launch the script (e.g. screen). I wouldn't design our interface around their specific papercuts considering all better alternatives that exist today.

prefer to totally remove v_num= and warp the value with a bracket.

Note that doing this or renaming it to version or ver doesn't address the point that the progress bar row length is very limited so it's better to move out everything we can so log(prog_bar=True) values can have more space.

@carmocca is in favor of removing it.

Yes 😁

[carmocca]

A removal in my opinion is not desired, as I have expressed in the past

Note that the OP advocates for moving it, not removing it. Did you mean that moving it off the pbar is also not desired?

[daniellepintz]

Thanks @awaelchli and @carmocca for providing your thoughts. I agree with @carmocca and still think we should move this out of the pbar.

When multiple training tasks are launched and we want to stop one of them based on the logging information, we need to idenfity which console corresponding to a target log/tensorboard.

I could be wrong, but this seems to me like not a super common case, and when this is the case the user can still easily add the logger version to the pbar. That is why it is customizable to the user's preferences.