chr5tphr
commented
1 year ago Hey @dkrako
thank you for the PR! I actually had thought of this feature before, so on that end I think it's definitely worth to have!
Implementation
I originally thought of either adding a new mixing Composite type (e.g. MixedComposite) which can mix arbitrary Composites, or implementing a new Composite type e.g. NameLayerMapComposite, which has the functionality you suggested for the current NameMapComposite.
For simplicity's sake, I think the latter solution may be preferable over changing NameMapComposite. I think it makes sense to keep the vanilla NameMapComposite as a way to have explicit-mapping-only-Composites.
Thinking about the actual fall-backs a user may have, I think instead of using a layer_map, a built-in Composite may be more common fallback candidate, e.g. "what happens if I only change this layer".
With this in mind, I actually tend more towards the MixedComposite solution, as complexity-wise it is actually not so different.
A MixedComposite would then be used like this:
composite = MixedComposite([
NameMapComposite([(('conv_1, 'conv_2'), Flat())]),
LayerMapComposite([((zennit.types.Convolution, ), AlphaBeta())])
])A NameLayerMapComposite could theoretically be implemented using
class NameLayerMapComposite(MixedComposite):
def __init__(self, name_map=None, layer_map=None, canonizers=None):
super().__init__([
NameMapComposite(name_map=name_map),
LayerMapComposite(layer_map=layer_map),
], canonizers=canonizers)And finally, a draft prototype for MixedComposite would be:
class MixedComposite(Composite):
def __init__(self, composites, canonizers=None):
if canonizers is None:
canonizers = []
self.composites = composites
super().__init__(
module_map=self.mapping,
canonizers=sum([composite.canonizers for composite in composites], canonizers)
)
def mapping(self, ctx, name, child):
# create a context for each of the sub-composites if ctx is empty
if not ctx:
ctx.update({composite: {} for composite in self.composites})
# evaluate all hooks so all composites can update their context
hooks = [composite.module_map(ctx[composite], name, child) for composite in self.composite]
return next((hook for hook in hooks if hook is not None), None)Requirements
I think it would be better to implement the more general MixedComposite.
The To-Do's associated with this would then be
- [ ] Implement the
MixedCompositeinsrc/zennit/composites.py(with docstrings) - [ ] Implement the
NameLayerMapCompositeinsrc/zennit/composites.py(with docstrings) - [ ] Add documentation, ideally a section in How-To's/Composites after Cooperative LayerMapComposite
- [ ] Add tests in
tests/test_composites.py - [ ] (optional) think of a better name for
MixedComposite
Would you be willing to complete the implementation and change the PR accordingly?
dkrako
This way it is possible to specify a fallback layer map if we want to apply more general rules to the remaining layers.
The parameter is optional. If None is passed, there's no change in functionality. If a layer_map list is passed with the same format as for the LayerMapComposite, then this will be used as a fallback if there's no hook for this specific layer name.
Just an example, where it's possible to set one Hook for the first two convolutional layers, and a different one for the remaining ones:
I hope you deem this to be a useful addition to this package.