pytorch / pytorch

Tensors and Dynamic neural networks in Python with strong GPU acceleration
https://pytorch.org
Other
83.19k stars 22.44k forks source link

Group losses in a common namespace #88325

Open Atcold opened 1 year ago

Atcold commented 1 year ago

🚀 The feature, motivation and pitch

It would be nice to remove the need to go fish losses from the nn module and have them all in a common nn.loss namespace.

So, instead of having

nn.L1Loss
nn.MSELoss
nn.CrossEntropyLoss
nn.CTCLoss
nn.NLLLoss

we would have

nn.loss.L1
nn.loss.MSE
nn.loss.CrossEntropy
nn.loss.CTC
nn.loss.NLL

Such a hierarchy (Containers, Convolution Layers, Pooling layers, Padding Layers, etc…) already exists in the nn documentation and, IMHO, should be reflected in the API as well. Is there a reason why this is not the case?

cc @albanD @mruberry @jbschlosser @walterddr @kshitij12345 @saketh-are

albanD commented 1 year ago

Fun fact: this mostly exists today already!

>>> dir(torch.nn.modules.loss)
['BCELoss', 'BCEWithLogitsLoss', 'CTCLoss', 'Callable', 'CosineEmbeddingLoss', 'CrossEntropyLoss', 'F', 'GaussianNLLLoss', 'HingeEmbeddingLoss', 'HuberLoss', 'KLDivLoss', 'L1Loss', 'MSELoss', 'MarginRankingLoss', 'Module', 'MultiLabelMarginLoss', 'MultiLabelSoftMarginLoss', 'MultiMarginLoss', 'NLLLoss', 'NLLLoss2d', 'Optional', 'PairwiseDistance', 'PoissonNLLLoss', 'SmoothL1Loss', 'SoftMarginLoss', 'Tensor', 'TripletMarginLoss', 'TripletMarginWithDistanceLoss', '_Loss', '_Reduction', '_WeightedLoss', '__all__', '__builtins__', '__cached__', '__doc__', '__file__', '__loader__', '__name__', '__package__', '__spec__', 'warnings']

We just re-expose all of these to torch.nn as well for simplicity. Does that address your question? Do you think we should better document that these submodules of nn do exist?

Atcold commented 1 year ago

I think the names should all lose the Loss ending since they are already in the correct namespace. The alias can keep the ending. I'd suggest doing this before updating the docs. Then, update docs and point out the more principled way of accessing the API.

Atcold commented 1 year ago

So, can we clean up a little the API? I'd suggest having nn.loss followed by loss-less names. They could simply be links to the loss'ed names in nn.modules.loss. Later, the loss'ed names can be deprecated in favour of the more structured ones.