Add average_all_summaries context manager#1829
Conversation
alf/summary/summary_ops.py
Outdated
| alf.summary.record_if = _disabled_record_if | ||
| alf.summary.should_record_summaries = lambda: True |
There was a problem hiding this comment.
Why do these two need to be changed?
There was a problem hiding this comment.
record_if must be changed so that the recording interval set by the user is actually used.
should_record_summaries actually shouldn't be overridden as doing so is redundant. I've removed this check. Unit test still passes successfully.
There was a problem hiding this comment.
Hey @emailweixu, I've pushed a fix concerning our offline discussion. Now the user has to provide a set of target names, so summaries that shouldn't be messed with don't get caught by accident. It also ensures that record_if logic does not get altered for any nested summary not included in target_names.
You can take a look at the updated unit test to see the correct behavior.
Make sure non-specified summaries do not have their record_if logic overwritten
2 participants
A useful context manager for computing summary averages over an interval.
Nice to have because several alf algorithms have averaging off as default.
If using these summaries with a small mini-batch-size, we can selectively control which nested summaries get averaged without having to overwrite the source code.