Lightning-Universe / lightning-transformers

Flexible components pairing 🤗 Transformers with :zap: Pytorch Lightning
https://lightning-transformers.readthedocs.io
Apache License 2.0
607 stars 77 forks source link

TextClassificationTransformer should log torchmetrics object instead of computed Tensors #276

Open stefan-schroedl opened 1 year ago

stefan-schroedl commented 1 year ago

🐛 Bug

In line 58 of the TextClassificationTransformer.common_step() method (https://github.com/Lightning-AI/lightning-transformers/blob/master/lightning_transformers/task/nlp/text_classification/model.py#L58), Logging is called with a dictionary of metric values computed for the current batch. I am new to this package but I believe the logger has to be called with the torchmetrics.Metric subclass for cases where the computed value is different from the average of the per-batch values, such as for class Precision - otherwise the aggregation gives wrong results. Similar code exists in TokenClassificationTransformer and ImageClassificationTransformer as well.

To Reproduce

Using TextClassificationTransformer with Precision and Recall metrics (as configured in the default) will result in inaccurate per-epoch values for validation and testing.

Environment

Borda commented 1 year ago

@rohitgr7, mind having a look? :rabbit: