Open LukasMut opened 2 years ago
This sounds reasonable. We should probably only accept None
or List[str]
. For a single module, the list contains only a single element.
For this, we want to use defaultdict(list)
from collections
and append features for every module.
I second letting users provide names. Otherwise, some users might be suprised/confused how much of their storage - depending on the network they chose - is now filled with activations for dozens of modules :)
I'm not sure if I'd go with None
for all modules though. It might be more intuitive to require the string all
. But I can't evaluate whether that makes things ugly or harder to maintain code-wise :) Just my 2 cents.
I think making extracting all modules more explicit makes sense, otherwise you could just do it by accident. Maybe the best option would be to only allow List[str]
and if the user really wants to extract everything, he could just do module_names=extractor.get_modules_names()
? I think that's an edge case anyway
Suggestion: Let user provide either
None
for all modules,str
for single module orList
for multiple modules. This should also probably come after #74.