Skip to content
This repository has been archived by the owner on May 1, 2023. It is now read-only.

IFM sparsity collector #443

Merged
merged 10 commits into from
Dec 18, 2019
Merged

IFM sparsity collector #443

merged 10 commits into from
Dec 18, 2019

Conversation

barrh
Copy link
Contributor

@barrh barrh commented Dec 12, 2019

Patch adds support to collecting IFM sparsity, as well as multiple IFMs. If multiple IFMs are present, they are merged with torch.cat, which is a parameter to the collector.
IFM sparsity will be collected whenever activation sparsity is collected with create_activation_stats_collectors.
Renamed some of the fields/filenames.

Tested against Python3.6. Passed excluding the post-training tests. (merged @guyjacob bugfix)
Note the use of typing library for type hinting. Not sure about the state of support for this in older Python 3.5 environments. It's possible that this patch requires Python3.6 OR Python3.5+updates, but not sure how to check that.

Collect IFM sparsity by SummaryActivationStatsCollector when in_or_out
kwarg is set to `in`, versus OFM when set to 'out'.
Use `torch.cat` to merge the tuple of inputs into single tensor.
In the case where there're no inputs, it will fail.

Rename argument name from singular 'input' to plural 'inputs' in
_activation_stats_cb.

Rename the former 'sparsity' collector to 'sparsity_ofm'.
Also, omit stat_name from module_name.
e.g. sparsity_module.conv1 -> module.conv1
When True, collect OFM sparsity, otherwise, IFMs sparsity
@barrh
Copy link
Contributor Author

barrh commented Dec 12, 2019

@nzmora I would like to replace lambda t: 100 * distiller.utils.sparsity(t) with distiller.utils.sparsity.
It make sense to me because when you open it in Excel, this should be formatted in percentage, which achieves the intended result, whereas now it does not. e.g. reformatting "20.5" to percentage in Excel results in "2050%".
What do you think?

Copy link
Contributor

@nzmora nzmora left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good - I've added some comments

distiller/apputils/image_classifier.py Outdated Show resolved Hide resolved
distiller/data_loggers/collector.py Outdated Show resolved Hide resolved
distiller/data_loggers/collector.py Outdated Show resolved Hide resolved
distiller/data_loggers/collector.py Show resolved Hide resolved
Copy link
Contributor

@nzmora nzmora left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one request, but it is not mandatory for the merge. if you don't get to it in a couple of days, I'll merge

'collect_quant_stats', 'collect_histograms',
'collector_context', 'collectors_context']

class CollectorDirection(enum.Enum):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you add a documentation line explaining what IN/OUT and IFM/OFM (input/output feature maps) are?

@nzmora nzmora merged commit cc50035 into IntelLabs:master Dec 18, 2019
michaelbeale-IL pushed a commit that referenced this pull request Apr 24, 2023
Add directionality to SummaryActivationStatsCollector to allow collection of statistics on incoming and outgoing activations/feature-maps; instead of just outgoing activations.

Also includes some code refactoring.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants