self.log: `strategy.reduce` vs `reduce_fx`
#17 831 ouverte le 15 juin 2023
Métriques du dépôt
- Stars
- (26 687 stars)
- Métriques de merge PR
- (Merge moyen 9j 15h) (3 PRs mergées en 30 j)
Description
Description & Motivation
self.log used to have separate parameters sync_dist_op and reduce_fx: The reduce_fx callable was used to reduce all of the logged values over an entire epoch, whereas sync_dist_op was used to reduce values across multiple GPUs. Currently it is not possible to set sync_dist_op. Instead, strategy.reduce is used but there is no way to pass the optional reduce_op parameter to this function. I think this means you always get the default, which is 'mean' for DDPStrategy. I think it would be useful for self.log to expose both the between-gpu reduction and the over-epoch reductions. For example, if one wants to log the maximum value encountered on any GPU over an entire epoch, one would call self.log(..., on_epoch=True, sync_dist=True, sync_dist_op='max', reduce_fx='max'). self.log can then create a partial(strategy.reduce, reduce_op=sync_dist_op) so that the strategy reduce is used, but with the desired reduce_op. Today, if you do self.log(..., on_epoch=True, sync_dist=True, reduce_fx='max') you will get the maximum over the epoch of the mean value across all GPUs, which is not what is desired, and could be confusing.
Pitch
self.log (and log_dict) should take a sync_dist_op parameter and use it as the reduce_op kwarg for strategy.reduce to improve logging values over multiple GPUs and multiple steps per epoch. The sync_dist_op and reduce_fx parameters should be renamed to make it clear why they are different and what they do.
Alternatives
Use a torchmetrics.Metric instead.
Additional context
No response
cc @borda @carmocca @Blaizzy