Right now a model definition requires that any metrics be Callable class.
See this line in vak.models.definition:
|
inspect.isclass(metrics_dict_val) and callable(metrics_dict_val) |
This can lead to cryptic errors when implementing a model--see this discussion with @marisbasha on #699
#699 (comment)
I think we should instead require that any metrics be a subclass of torchmetrics.Metric. Basically, it should be a metric that's built into the library, or a custom metric that's implemented using the library (as described here).
This way we are explicit in the code, can give an explicit error about why it didn't work, and can help ensure we have consistent behavior for metrics.
The one thing to consider here is how to handle losses, that are often included as part of metrics so that one can get the loss on the validation set. E.g., we use the cross entropy loss class now for TweetyNet. Forcing people to write a metric subclass for every loss would be kind of annoying
Right now a model definition requires that any
metricsbeCallableclass.See this line in
vak.models.definition:vak/src/vak/models/definition.py
Line 260 in 206c734
This can lead to cryptic errors when implementing a model--see this discussion with @marisbasha on #699
#699 (comment)
I think we should instead require that any
metricsbe a subclass oftorchmetrics.Metric. Basically, it should be a metric that's built into the library, or a custom metric that's implemented using the library (as described here).This way we are explicit in the code, can give an explicit error about why it didn't work, and can help ensure we have consistent behavior for metrics.
The one thing to consider here is how to handle losses, that are often included as part of metrics so that one can get the loss on the validation set. E.g., we use the cross entropy loss class now for TweetyNet. Forcing people to write a metric subclass for every loss would be kind of annoying