Forward metric name to update_metric

[antoinebrl]

🚀 Feature Request

Motivation

get_metrics returns a mapping between strings and Metrics. However the name isn't available in the update_metric. Checking for types might not be sufficient to uniquely characteristic a metric.

Implementation

The proposed new signature for the update_metric method of the ComposerModel:

- def update_metric(self, batch: Any, outputs: Any, metric: Metric) -> None:
+ def update_metric(self, batch: Any, outputs: Any, metric: Metric, name: str) -> None:

[antoinebrl]

@mvpatel2000 is it a feature you would consider or it's considered an unnecessary breaking change? Happy to look into implementing it.

[mvpatel2000]

@antoinebrl apologies for delay, I missed this issue. Let me pass it around internally. I'll get back to you soon!

[mvpatel2000]

@antoinebrl do you have a use case in mind that would require this breaking change?

[antoinebrl]

Hi @mvpatel2000 !

We are doing multi-head training where we have, for example 2 classification heads that we train jointly. The metrics associated to the heads have the same type (ie torchmetrics.Precision). To assign the correct classification head to the correct metric object we would have to rely on a strong order assumption or an identifier to map it correctly. We propose here to use the metric name to do that matching.

[mvpatel2000]

Got it, thanks! That's very helpful.

The whole way we do metrics seems unideal... I have a rough design in mind that I need to flesh out which would revamp this. It is one of the major points I wish to revamp (along with checkpointing) ahead of 1.0. Unfortunately, this would take a bit of time since it is breaking.

As a short-term unblock, one trick you can do is attach name to metric as an attribute. It's pretty hacky and not ideal, but it should enable this workload. I'm open to other suggestions as well! I'd love to have something that doesn't require a large overhaul...

[antoinebrl]

Thanks for considering this feature request! We went with something fairly similar to your suggestion. We created a metric wrapper to be able to attach a name to any metric.

I'm happy to provide a fresh perspective on your refactoring around the metrics. I'm also curious to know what you have in mind regarding checkpointing.

[mvpatel2000]

We can send out a RFC when we're ready for it :)