New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Inhibition #26
Inhibition #26
Conversation
tnmf/TransformInvariantNMF.py
Outdated
self._inhibition_range = tuple(np.ceil(a / 2) for a in atom_shape) if inhibition_range is None else inhibition_range | ||
assert len(self._inhibition_range) == len(atom_shape) | ||
inhibition_kernels_1D = list((1 - ((np.arange(-i, i + 1) / i) ** 2) for i in self._inhibition_range)) | ||
self._inhibition_kernel = np.prod(np.array(np.meshgrid(*inhibition_kernels_1D)), axis=0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
atom.ndim separate 1D-convolutions are more efficient than this multi-D convolution
tnmf/TransformInvariantNMF.py
Outdated
# TODO: maybe also add cross-channel/cross-atom inhibition? | ||
convolve_axes = range(-len(self.atom_shape), 0) | ||
inhibition_gradient = convolvend( | ||
self.H, self._inhibition_kernel[np.newaxis, np.newaxis, :], mode='same', axes=convolve_axes) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This has to be moved into the backend.
Fixed all comments we talked about. Btw.: This fixes #25 |
Both reach the same level of sparsity with considerably different reconstruction quality
i.e. no inhibition, to fix existing unit tests
ususally, it does not make sense to set inhibition and sparsity_H > 0. Thus, setting both to default 0 is more intuitive
…convolutions unfortunately, there is no oaconvolve1D
I'd rather not destroy the tabular layout by adding line breaks here
Hi @AdrianSosic,
This PR adds inhibition essentially identical to how it was done in the old implementation (except for that this one is supporting different inhibition kernel sizes in the different directions, which is in line with support for non-square atoms).
What do you think? - Maybe, adding cross-atom inhibition might make sense, but this could also be a follow-up project as parametrization, etc. would still have to be discussed...