rename valid_dataloader [discussion]

[Borda]

What does this PR do?

I have found the shortcut val for validation quite confusing, so I would rename it as valid

• rename val_dataloader -> valid_dataloader
• rename get_val_dataloader -> get_valid_dataloader

PR review

Anyone in the community is free to review the PR once the tests have passed.
If we didn't discuss your PR in Github issues there's a high chance it will not be merged.

[jeffling]

I would argue that it should be 'validation_dataloader' instead of 'val', since 'valid' means something and could cause confusion (valid/invalid).

[Borda]

I would argue that it should be 'validation_dataloader' instead of 'val', since 'valid' means something and could cause confusion (valid/invalid).

I am fine with validation_dataloader just feeling so confused with the val

[awaelchli]

Why rename val_dataloader and not train_dataloader? If val_dataloader is confusing, then also train_dataloader should confuse you. Is it the verb or the vehicle for transportation? xD
So what I mean to say is: I agree that it needs to be renamed to remove confusion, but your reasoning is not complete.
Another argument would be that we have training_step and validation_step, and not train_step, val_step.

[williamFalcon]

p(word|context) makes it very clear it’s not about the transportation noun ;)

[Borda]

Why rename val_dataloader and not train_dataloader? If val_dataloader is confusing, then also train_dataloader should confuse you. Is it the verb or the vehicle for transportation? xD

for me val is a natural shortcut fro value, but what can you do with train to be still in CS

So what I mean to say is: I agree that it needs to be renamed to remove confusion, but your reasoning is not complete.
Another argument would be that we have training_step and validation_step, and not train_step, val_step.

so what renaming would you propose? :]

[neggert]

We've already changed this stuff once and it confused a lot of users (tng_dataloader -> train_dataloader). I'd prefer to avoid changing it again. train/val/test is pretty common parlance anyway, e.g. torchvision.dataset.ImageNet.

If we do change it, it should be to get rid of the abbreviation entirely, rather than switching to a different abbreviation.

[Borda]

let's make it as a small poll:

• validate_dataloader -> 👀
• valid_dataloader -> 🚀
• validation_dataloader -> 🎉
• nothing -> 👎
  @williamFalcon @jeffling @neggert @Ir1d @tullie ^^

[kuynzereb]

I agree with @jeffling that valid_dataloader sounds even more ambiguous than val_dataloader. And I agree with @neggert that val is a very common abbreviation for validation. Maybe val is confusing sometimes (for example, val_loss or loss_val), but in the context of val_dataloader it doesn't seem to be ambiguous...

So it looks like that val_dataloader → valid_dataloader will be only worse. And I don't see any obvious pros for val_dataloader → validation_dataloader.

[jeffling]

based on torchvision using val, i vote for keeping as is