Issue 874 : add custom events for Metrics

[sdesrozis]

Fixes #874

Description:

Allow customization for Events of Metrics.

Check list:

• New tests are added (if a new feature is added)
• New doc strings: description and/or example code are in RST format
• Documentation is updated (if required)

[sdesrozis]

@vfdev-5 thoughts on this draft ?

[vfdev-5]

We may have to take this comment into account for the usage : #644 (comment)

[sdesrozis]

We may have to take this comment into account for the usage : #644 (comment)

Same issue??

[sdesrozis]

@vfdev-5

Major refactoring

• Add interface EngineMetric which defines methods used by Engine
• Add functions attach, detach and is_attached external to Metric using Engine, MetricUsage and EngineMetric
• MetricUsage moved from __init__ to attach, detach and is_attached (could simplify detach and is_attached)

The idea is to have Metric like an algorithm pluggable in any Engine wrt a MetricUsage. Today, attach, detach and is_attached are embedded in Metric but one could use external functions.

engine1 = ...
engine2 = ...

m = MyMetric()

# current API
m.attach(engine1, "m")
m.attach(engine2, "m", usage=BatchWise())

# possible API
attach(metric=m, engine=engine1, name="m")
attach(metric=m, engine=engine2, name="m", usage=BatchWise())

Thoughts ?

[vfdev-5]

@sdesrozis please, show an example of usage. For instance, I do not understand the need of another interface... Thanks

[sdesrozis]

The idea is to have Metric like an algorithm with API to be plugged to engine but no reason to have the plug in the metric.

This is a separation of concerns.

However, this is just a draft :) The interface EngineMetric could be removed. The main point is to have MetricUsage at attachment.

[vfdev-5]

@sdesrozis I'm not fan of "possible API", let's keep current API.
If I get it right, EngineMetric interface is only for "possible API" ?

[sdesrozis]

@sdesrozis I'm not fan of "possible API", let's keep current API.

If I get it right, EngineMetric interface is only for "possible API" ?

Yes, I can remove it, no problem 😊

[sdesrozis]

Removed

[vfdev-5]

@sdesrozis thanks ! What do you think about to enhance the following API by

engine1 = ...
engine2 = ...

m = MyMetric()

# current API
m.attach(engine1, "m")
m.attach(engine2, "m", usage="batch_wise")  # ~ BatchWise()
m.attach(engine2, "m", usage="epoch_wise")  # ~ EpochWise()

?

No need to import those classes.

[vfdev-5]

@sdesrozis code looks OK, documentation layout is not correct:

• missing docs for "MetricUsage", "EpochWise", "BatchWise", "BatchFiltered"
• missing indents renders badly docs, i'll comment out where.
  Please, try to render the docs locally and check it.

[vfdev-5]

Missing indent here

[vfdev-5]

Missing indent here

[sdesrozis]

@vfdev-5 thank you, I update soon :p

[sdesrozis]

Doc done. Next, I would like to refactor a last thing after merge of fix for #1004 (if ok).

[vfdev-5]

Looks good !