Wrong logic in `finalize_agg_metrics` routine
🐛 Bug
1. TrainerLoggingMixin.log_metrics methos makes a call:

  
    
      pytorch-lightning/pytorch_lightning/trainer/logging.py
    
    
        Lines 73 to 74
      in
      7919624
    
  
  
    

        
          
           self.logger.agg_and_log_metrics(scalar_metrics, step=step) 
        

        
          
           self.logger.save() 
        
    
  


2. Now, let's check this save method in LightningLoggerBase:

  
    
      pytorch-lightning/pytorch_lightning/loggers/base.py
    
    
        Lines 223 to 225
      in
      7919624
    
  
  
    

        
          
           def save(self) -> None: 
        

        
          
               """Save log data.""" 
        

        
          
               self._finalize_agg_metrics() 
        
    
  


3. Go deeper. Now, _finalize_agg_metrics in LightningLoggerBase:

  
    
      pytorch-lightning/pytorch_lightning/loggers/base.py
    
    
        Lines 108 to 111
      in
      7919624
    
  
  
    

        
          
           def _finalize_agg_metrics(self): 
        

        
          
               """This shall be called before save/close.""" 
        

        
          
               agg_step, metrics_to_log = self._reduce_agg_metrics() 
        

        
          
               self._metrics_to_agg = [] 
        
    
  


Please, pay attention to:
self._metrics_to_agg = []

At this point, we clean up the self._metrics_to_agg.
Now, let's recall the first statement, where the TrainerLoggingMixin performs self.logger.save().
The Mixin performs this call on each step. But not on each accumulation step! So, we will execute steps 2 and 3 before an actual metrics aggregation. It means, that we'll not aggregate metrics, but instead, we will log them on each NOT-accumulation step.
To Reproduce
Steps to reproduce the behavior:
Copy this test and execute it in tests/loggers/test_base.py harness.
def test_with_accumulate_grad_batches_and_trainer():
    class StoreHistoryLogger(CustomLogger):
        def __init__(self):
            super().__init__()
            self.steps_logged = []

        @rank_zero_only
        def log_metrics(self, metrics, step):
            self.steps_logged.append(step)

    hparams = tutils.get_default_hparams()
    model = LightningTestModel(hparams)
    logger = StoreHistoryLogger()
    trainer = Trainer(
        accumulate_grad_batches=5,
        logger=logger,
        max_epochs=2,
        row_log_interval=1
    )
    trainer.fit(model)

    number_of_unique_steps_logged = len(set(logger.steps_logged))
    number_of_total_steps_logged = len(logger.steps_logged)

    assert number_of_unique_steps_logged == number_of_total_steps_logged
It will fail because the actual number of logged steps is greater than the required (required is equal to the actual divided by accumulate_grad_batches).
Expected behavior
Metrics must be aggregated and logged on each n_accum steps, but not logged on each step.
Environment
Environment
OS: Linux
architecture: 64bit
processor: x86_64
python: 3.7.6
version: #97~16.04.1-Ubuntu SMP Wed Apr 1 03:03:31 UTC 2020
pytorch-lightning: 0.7.5
Additional context
@Borda , I think, you can help me with this issue. It's something wrong with save and finalize logger routines.