Repository: pytorch/ignite

Issue:
Improve Frequency
## 🚀 Feature

If we would like to log datapoints/second every 100 iterations, we most probably do like this 
```python
wps_metric = Frequency(output_transformer=lambda x: x['ntokens'])
wps_metric.attach(trainer, name='wps', event_name=Events.ITERATION_COMPLETED(every=100))
```
however, seems like this wont take into account all other iterations while computing the total number of tokens.
```python
class Frequency(Metric):
    ....
    def attach(self, engine, name, event_name=Events.ITERATION_COMPLETED):
        engine.add_event_handler(Events.EPOCH_STARTED, self.started)
        engine.add_event_handler(event_name, self.iteration_completed)
        engine.add_event_handler(event_name, self.completed, name)
```
IMO, should be 
```python
class Frequency(Metric):
    ....
    def attach(self, engine, name, event_name=Events.ITERATION_COMPLETED):
        engine.add_event_handler(Events.EPOCH_STARTED, self.started)
        engine.add_event_handler(Events.ITERATION_COMPLETED, self.iteration_completed)
        engine.add_event_handler(event_name, self.completed, name)
```

cc @erip 


Discussion:
erip: Hopefully I understood the issue correctly - the tests seem to be OK. If these tests look representative, I think they can (and should) be refactored to generalize the original frequency tests w/ the full engine setup.
vfdev-5: Yes, you are right, we can refactor original and new tests into a single code. 

And there is a docstring update.


erip: This looks good now. 😄 Which docstring should be updated? Just adding another example in the `Frequency` docstring?
vfdev-5: > Just adding another example in the Frequency docstring?

Yes, another example with the explanation of what it does :)
vfdev-5: Elijah, I was also wondering how's going integrations of training examples of fairseq with ignite you were working on previously ? 
erip: It's going slowly, unfortunately. Fairseq has been working on moving from a framework to a library and it's made development outside of adding new architecture slow. I have a handful of pending PRs that would hopefully move the needle a bit, but there hasn't been much movement.
vfdev-5: I see, thanks for the info. Yes, maybe also they do not have a point to use such libs like ignite (and introduce it as a dependency)...

Are you at PyTorch slack where we could discuss in more fluent way ?

Patch:
diff --git a/ignite/metrics/frequency.py b/ignite/metrics/frequency.py
index 3a127995..7ae1e0ce 100644
--- a/ignite/metrics/frequency.py
+++ b/ignite/metrics/frequency.py
@@ -19,8 +19,21 @@ class Frequency(Metric):
             wps_metric.attach(trainer, name='wps')
             # Logging with TQDM
             ProgressBar(persist=True).attach(trainer, metric_names=['wps'])
-            # Progress bar will looks like
+            # Progress bar will look like
             # Epoch [2/10]: [12/24]  50%|█████      , wps=400 [00:17<1:23]
+
+
+        To compute examples processed per second every 50th iteration:
+
+        .. code-block:: python
+
+            # Compute number of tokens processed
+            wps_metric = Frequency(output_transform=lambda x: x['ntokens'])
+            wps_metric.attach(trainer, name='wps', event_name=Events.ITERATION_COMPLETED(every=50))
+            # Logging with TQDM
+            ProgressBar(persist=True).attach(trainer, metric_names=['wps'])
+            # Progress bar will look like
+            # Epoch [2/10]: [50/100]  50%|█████      , wps=400 [00:17<00:35]
     """
 
     def __init__(self, output_transform=lambda x: x, device=None):
@@ -59,5 +72,5 @@ class Frequency(Metric):
 
     def attach(self, engine, name, event_name=Events.ITERATION_COMPLETED):
         engine.add_event_handler(Events.EPOCH_STARTED, self.started)
-        engine.add_event_handler(event_name, self.iteration_completed)
+        engine.add_event_handler(Events.ITERATION_COMPLETED, self.iteration_completed)
         engine.add_event_handler(event_name, self.completed, name)


Test patch:
diff --git a/tests/ignite/metrics/test_frequency.py b/tests/ignite/metrics/test_frequency.py
index 369ad13f..03f01a1b 100644
--- a/tests/ignite/metrics/test_frequency.py
+++ b/tests/ignite/metrics/test_frequency.py
@@ -21,7 +21,7 @@ def test_nondistributed_average():
     assert average_lower_bound < average < average_upper_bound
 
 
-def _test_frequency_with_engine(device, workers):
+def _test_frequency_with_engine(device, workers, every=1):
 
     artificial_time = 0.1 / workers  # seconds
     total_tokens = 1200 // workers
@@ -35,9 +35,10 @@ def _test_frequency_with_engine(device, workers):
 
     engine = Engine(update_fn)
     wps_metric = Frequency(output_transform=lambda x: x["ntokens"], device=device)
-    wps_metric.attach(engine, "wps")
+    event = Events.ITERATION_COMPLETED(every=every)
+    wps_metric.attach(engine, "wps", event_name=event)
 
-    @engine.on(Events.ITERATION_COMPLETED)
+    @engine.on(event)
     def assert_wps(e):
         wps = e.state.metrics["wps"]
         assert estimated_wps * 0.80 < wps < estimated_wps, "{}: {} < {} < {}".format(
@@ -57,3 +58,18 @@ def test_frequency_with_engine():
 def test_frequency_with_engine_distributed(distributed_context_single_node_gloo):
     device = "cpu"
     _test_frequency_with_engine(device, workers=dist.get_world_size())
+
+
+def test_frequency_with_engine_with_every():
+    device = "cpu"
+    _test_frequency_with_engine(device, workers=1, every=1)
+    _test_frequency_with_engine(device, workers=1, every=2)
+    _test_frequency_with_engine(device, workers=1, every=10)
+
+
+@pytest.mark.distributed
+def test_frequency_with_engine_distributed_with_every(distributed_context_single_node_gloo):
+    device = "cpu"
+    _test_frequency_with_engine(device, workers=dist.get_world_size(), every=1)
+    _test_frequency_with_engine(device, workers=dist.get_world_size(), every=2)
+    _test_frequency_with_engine(device, workers=dist.get_world_size(), every=10)